Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
232 changes: 232 additions & 0 deletions std/conv.d
Original file line number Diff line number Diff line change
Expand Up @@ -3642,6 +3642,238 @@ unittest
assert(i is k);
}


/** Sets the passed object to its `init` state.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is non-public, ddoc should not be generated for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. And where is the problem?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use a ddoc comment if it's not supposed to end up in the documentation. Use a regular comment.


Use this function instead of dealing with tricky $(D typeid(T).init()).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tricky? If a user doesn't know why, this comment doesn't help them at all. Oh, it's package anyway, so doc comment quality doesn't matter that much.


Note: be careful as it also changes memory marked as $(D const)/$(D immutable).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sounds rather dangerous. What use cases are motivating this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functions like move and emplace.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least move should not overwrite immutable fields. Consider:

struct S { immutable int a; }
void main() {
    auto obj1 = S(5);
    immutable(int)* p = &obj1.a;
    auto obj2 = move(obj1);
    assert(*p == 5);
}

The type int could be replaced with any other. The issue here is that the assert should succeed by the definition of immutable. If move took initiative in overwriting immutable fields, we'd break immutable.

The structure in this example is still moveable because it's okay to duplicate an immutable int. However, in general we should accept there are types that cannot be moved from.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least move should not overwrite immutable fields.

Disagree. move is unsafe and it's a user responsibility, that's all. Following your opinion we will finally end with emplace also not working for such types and, as a consequence, lots of derived algorithms (collections etc.) not working too. And it will be very frustrating.

As always, I'm not sure in my theoretical reasoning and suggest you to change move to reject immutable and wait for reaction from programmers.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand how the current implementation of move gives that impression, but really move is intended as a safe optimization of copying. It should work preserving safety and only overwrite mutable data in the source of the move operation. The fact that move currently overwrites immutable data is a bug in move.

Emplace has a different charter altogether as it transforms uninitialized data into actual typed data; so for emplace it's fine to overwrite immutable fields.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you failed to document move properly and it is already used as unsafe and working everywhere function.
And you can either create second safeMove or make move safe, create unsafeMove and inform people about breaking change. Personally I dislike breaking changes, so current move can be deprecated and both safeMove and unsafeMove can be created.

I suppose you do understand that you can not just go and remove moving function from phobos.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree we need to worry about breaking existing code that uses move. The only breakage would involve structs that have const or immutable fields of types with destructors. So the question is whether making move safe justifies the cost of breaking such code. I'd push for making it safe instead of adding safe vs unsafe variants. Opinions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree we need to worry about breaking existing code that uses move.

No, I'm not telling about it. I'm telling that there is a function in phobos that doing useful job X and you can't just get and remove the function from the library telling: I don't like X.

*/
package void setToInitialState(T)(ref T t)
{
alias Unqual!T U;

// If there is a compiler-generated elaborate assign, `T` isn't nested, and
// either elaborate copy constructor or elaborate destructor is absent
// `t = T.init` has no side effects. It is either an optimization
// opportunity or a compiler bug.
static if(hasElaborateAssign!T || (!isAssignable!T && !isAssignable!U))
{
import core.stdc.string;

// `typeid(T)` will also work but will cost a virtual call per each array
// dimension. We will not be here for [static arrays of] classes so
// there is no problems with `TypeInfo_Class.init` field name clash.
if(auto p = typeid(MultidimensionalStaticArrayElementType!U).init().ptr)
foreach(ref el; asFlatStaticArray((*cast(U*) &t)))
memcpy(&el, p, typeof(el).sizeof);
else
memset(cast(void*) &t, 0, T.sizeof);
}
else static if(!isAssignable!T)
{
(*cast(U*) &t) = U.init;
}
else
{
t = T.init;
}
}

unittest
{
int i = -1;
setToInitialState(i);
assert(i == 0);

static assert(!__traits(compiles, setToInitialState(5))); // doesn't accept rvalue

static bool exited = false;

static struct S(int def)
{
int i = def;
@disable this();
this(this) { assert(0); }
~this() { assert(exited); }
}

S!0 s0 = void; s0.i = -1;
setToInitialState(s0);
assert(s0.i == 0);

S!1 s1 = void; s1.i = -1;
setToInitialState(s1);
assert(s1.i == 1);

S!1[2][1] sArr = void;
foreach(ref el; sArr[0])
el.i = -1;
setToInitialState(sArr);
assert(sArr == (S!1[2][1]).init);

exited = true;
}

unittest // const
{
static struct Int1
{ int i = 1; }

static struct S
{ const Int1 i; }

int i = 0;
static assert(S.sizeof == i.sizeof);
setToInitialState(*cast(S*) &i);
assert(i == 1); i = 0;

setToInitialState(*cast(const S*) &i);
assert(i == 1); i = 0;
}


/** Calls the postblit of the given object, if any.

Faster and convenient replacement for $(D typeid(T).postblit(&t)).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is faster, could we merge it within typeid(T).postblit(&t)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you meant to add it in druntime? I'm tired to tell that currently we have no templates in druntime. And I'm not the one who thinks it should be there so you will have to move all required templates in a consistent way out from Phobos first.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a hard and fast rule to not have templates in druntime. We shouldn't use it as a restriction for how we need to approach things.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hrm, I meant "It's not a hard and fast..."

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, it's definitely the plan to add more, because things that aren't currently templated are supposed to be (e.g. the AA implementation). druntime is like any other library with regards to templates. If templatizing something is the best way to do something that it needs to do, then it's going to be templatized. If it's not, then it's not. There's nothing special about druntime that means that it will or won't contain templates, and it already does contain templates in some places.

*/
package void callPostblits(T)(ref T t)
{
static if(hasElaborateCopyConstructor!T)
{
foreach(ref el; asFlatStaticArray(t))
{
foreach(ref field; el.tupleof)
static if(hasElaborateCopyConstructor!(typeof(field)))
callPostblits(field);

static if(hasMember!(typeof(el), "__postblit"))
el.__postblit();
}
}
}

unittest
{
int i = -1;
callPostblits(i); // no-op for non-elaborate types

static assert(!__traits(compiles, callPostblits(5))); // doesn't accept rvalue

static int[] log;
static void checkLog(int[] arr...)
{ assert(log == arr); log = null; }

static bool exited = false;

static struct S
{
int i;
@disable this();
this(this) { log ~= i; }
~this() { assert(exited); }
}

S s = void; s.i = -1;
callPostblits(s);
checkLog(-1);

S[3][2][1] sArr = void;
foreach(j, ref el; *cast(S[6]*) sArr.ptr)
el.i = j;
callPostblits(sArr);
checkLog(0, 1, 2, 3, 4, 5);

static struct S2
{
S s;
S[2] sArr;

@disable this();
this(this) { log ~= -1; }
~this() { assert(exited); }
}

S2 s2 = void;
foreach(j, ref el; *cast(S[3]*) &s2)
el.i = j;
callPostblits(s2);
checkLog(0, 1, 2, -1);

exited = true;
}


/** Calls the destructor of the given object, if any.

Faster and convenient replacement for $(D typeid(T).destroy(&t)).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same question

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same answer

*/
package void callDestructors(T)(ref T t)
{
static if(hasElaborateDestructor!T)
{
foreach_reverse(ref el; asFlatStaticArray(t))
{
static if(hasMember!(typeof(el), "__dtor"))
el.__dtor();

foreach_reverse(ref field; el.tupleof)
static if(hasElaborateDestructor!(typeof(field)))
callDestructors(field);
}
}
}

unittest
{
int i = -1;
callDestructors(i); // no-op for non-elaborate types

static assert(!__traits(compiles, callDestructors(5))); // doesn't accept rvalue

static int[] log;
static void checkLog(int[] arr...)
{ assert(log == arr); log = null; }

static bool exited = false;

static struct S
{
int i;
@disable this();
this(this) { assert(exited); }
~this() { log ~= i; }
}

S s = void; s.i = -1;
callDestructors(s);
checkLog(-1);

S[3][2][1] sArr = void;
foreach(j, ref el; *cast(S[6]*) sArr.ptr)
el.i = j;
callDestructors(sArr);
checkLog(5, 4, 3, 2, 1, 0);

static struct S2
{
S s;
S[2] sArr;

@disable this();
this(this) { assert(exited); }
~this() { log ~= -1; }
}

S2 s2 = void;
foreach(j, ref el; *cast(S[3]*) &s2)
el.i = j;
callDestructors(s2);
checkLog(-1, 2, 1, 0);

exited = true;
}

// Undocumented for the time being
void toTextRange(T, W)(T value, W writer)
if (isIntegral!T && isOutputRange!(W, char))
Expand Down