https://issues.dlang.org/show_bug.cgi?id=16394

--- Comment #1 from Eyal <eyal@weka.io> ---
As a result, std.algorithm:initializeAll is also buggy:

import std.stdio;
unittest {
    struct Int {
        ~this() {}
        int x = 3;
    }
    import std.algorithm : initializeAll;
    Int[2] xs = [Int(1), Int(2)];
    struct R {
        bool done;
        bool empty() { return done; }
        ref Int[2] front() { return xs; }
        void popFront() { done = true; }
    }
    writeln(xs);
    initializeAll(R());
    writeln(xs);
}

Prints out:
[Int(1, 7FE7FED92000), Int(2, 7FE7FED92000)]
[Int(3, null), Int(0, 73)]

The second field being printed for Int seems like *yet another* bug.

--

https://issues.dlang.org/show_bug.cgi?id=16394

--- Comment #2 from Steven Schveighoffer <schveiguy@yahoo.com> ---
It has been this way since the beginning (traced it back to this commit: https://github.com/dlang/druntime/commit/1532910c769ab718a528c94c9157fecebd753c66)

Looking at the code, there really isn't a way to fix it without altering the Object API or the compiler, both of which would break a lot of things.

We could have the compiler store the entire static array data into the data section. This seems wasteful to say the least.

Is there not a way to handle TypeInfo_StaticArray specially? As far as I know, it's going to be the only type that behaves this way.

Alternatively, you can check tsize() to see what the real size should be, and
then if that doesn't match initializer().length (use initializer instead of
init(), as init() is deprecated), then replicate if your plan is to initialize
to the initializer value.

--

https://issues.dlang.org/show_bug.cgi?id=16394

Steven Schveighoffer <schveiguy@yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |schveiguy@yahoo.com

--

https://issues.dlang.org/show_bug.cgi?id=16394

--- Comment #3 from Eyal <eyal@weka.io> ---
The workaround is pretty clear.

But this is nasty! Anyone who uses typeid(T).init in a generic way (not caring
what T is) is going to be broken.

It would be better to have typeid(staticArr).init throw a compile-time error than to return the wrong result.

We've spent many hours reproducing and chasing this bug in a production build.

Additionally, x = x.init  would work without blowing up the stack when x is large, and then accessing typeid() would be unnecessary.

--

https://issues.dlang.org/show_bug.cgi?id=16394

--- Comment #4 from Steven Schveighoffer <schveiguy@yahoo.com> ---
typeid(T).initializer is not meant to be used without understanding what it does in all cases. There are other subtle issues with it.

I think this is just a documentation issue. I can see where this would cause problems :(

(In reply to Eyal from comment #3)
> Additionally, x = x.init  would work without blowing up the stack when x is large, and then accessing typeid() would be unnecessary.

I'm not sure what this means.

--

https://issues.dlang.org/show_bug.cgi?id=16394

--- Comment #5 from Eyal <eyal@weka.io> ---
I disagree, a function which has subtle corner cases that cause cryptic bugs is never a "documentation issue". Maybe if it were called "unsafe_init_val" -- you'd know that documentation is crucial here. Otherwise you go on your merry way until you explode.

I've seen only 2 uses of the init value on generic types, in our production code and in std.lib.algorithm:initailizeAll.  *Both of these cases* had cryptic, terrible bugs due to this behavior of typeid(T).init!  If D's standard library can't get the use of typeid().init right, is it a legitimate function to expose?

> > Additionally, x = x.init  would work without blowing up the stack when x is large, and then accessing typeid() would be unnecessary.

> I'm not sure what this means.

The only reason we need to use typeid(x).init() is because we cannot really do:

  x = x.init;

The reason is that this kind of assignment always goes through a stack allocation. When x.init is a large value, it unnecessarily allocates a huge chunk of stack. Our stacks are of limited size (fibers). This explodes.

So we work around it using typeid(x).init() and explicit memory copies (similar
to what initializeAll does).

If the compiler had properly implemented x = x.init without superfluous huge
stack allocations, we'd have no trouble because we wouldn't need to use
typeid().init() in the first place (though perhaps this is an LDC-specific
problem)

--

https://issues.dlang.org/show_bug.cgi?id=16394

--- Comment #6 from Steven Schveighoffer <schveiguy@yahoo.com> ---
I mean it's a doc issue in that the documentation doesn't reflect what initializer() actually does (and has always done). There isn't much to say except that I don't think we can change the behavior at this point without breaking things.

It's been this way since 2009, so most people don't use it or care about it, or they would have hit this issue before. I realize we can't get your time back looking for this issue, but I think at this point, the best thing to do is fix the docs and fix any code that was done using this incorrect assumption.

(In reply to Eyal from comment #5)

> The reason is that this kind of assignment always goes through a stack allocation. When x.init is a large value, it unnecessarily allocates a huge chunk of stack. Our stacks are of limited size (fibers). This explodes.

I think this is a legitimate improvement to suggest.

--

https://issues.dlang.org/show_bug.cgi?id=16394

ag0aep6g@gmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ag0aep6g@gmail.com

--- Comment #7 from ag0aep6g@gmail.com ---
(In reply to Steven Schveighoffer from comment #6)
> I mean it's a doc issue in that the documentation doesn't reflect what
> initializer() actually does (and has always done).

I.e., it's a long-standing bug.

> There isn't much to say
> except that I don't think we can change the behavior at this point without
> breaking things.

Pretty much every bug fix can be considered a breaking change. I don't think we need to live with this, just because someone may be relying on behavior that's clearly going against documentation.

> It's been this way since 2009, so most people don't use it or care about it, or they would have hit this issue before.

If they don't use it, their code won't get broken by the fix.

> I realize we can't get your time
> back looking for this issue, but I think at this point, the best thing to do
> is fix the docs and fix any code that was done using this incorrect
> assumption.

I disagree. I think fixing the code to behave as documented is the way to go.

--

https://issues.dlang.org/show_bug.cgi?id=16394

--- Comment #8 from Steven Schveighoffer <schveiguy@yahoo.com> ---
(In reply to ag0aep6g from comment #7)
> I.e., it's a long-standing bug.

Or functioning as designed, just not properly documented.

> Pretty much every bug fix can be considered a breaking change. I don't think we need to live with this, just because someone may be relying on behavior that's clearly going against documentation.

This part of TypeInfo is meant to be used internally. I don't think this corner case warrants a compiler change, as the current state is completely usable.

> If they don't use it, their code won't get broken by the fix.

Every time we change the compiler, it may break other things. I don't think it's worth the risk for such a small improvement. Not to mention it will bloat all object files. Why break things for the vast majority of people when the fix is simple for those who have been misled by the incomplete docs?

--

https://issues.dlang.org/show_bug.cgi?id=16394

--- Comment #9 from Eyal <eyal@weka.io> ---
> Or functioning as designed, just not properly documented.

> This part of TypeInfo is meant to be used internally. I don't think this corner case warrants a compiler change, as the current state is completely usable.

Not by Weka or by Andrei Alexandrescu, apparently, as both inserted terrible bugs into production code with this.

Do you think a warning in the manual would have prevented these bugs? Do you doubt that the next usages of this function will create similar bugs?

> Every time we change the compiler, it may break other things. I don't think it's worth the risk for such a small improvement. Not to mention it will bloat all object files. Why break things for the vast majority of people when the fix is simple for those who have been misled by the incomplete docs?

Note the "fix" I am requesting does not require bloating the object files. A compile-time error about use of the "init" value of a static array's typed id is enough.  Generating a special TypeInfo object for static arrays doesn't sound like rocket science.


Claiming the interface should be terrible to simplify the implementation is classic worse-is-better, and I think D aspires to be truly better.

--