Bug 20581 – DIP1000 wrongly flags hidden ref temporary

Compiling phobos unittests with -checkaction=context causes DIP1000 to raise an error about a hidden temporary outliving the referenced content. Reduced example: void main() @safe { int[5] a = [ 1, 2, 3, 4, 5 ]; assert(retro(a[]).source is a[]); // => Hidden temporary __assertOp4 } auto retro(return scope int[] s) @safe { static struct R { int[] source; } return R(s); } safe.d(6): Error: address of variable a assigned to __assertOp4 with longer lifetime

The cause of this is that assert() may throw an exception and so the stack reference (used by -checkaction=context) will outlive main() as the stack is unwound. The error is correctly generated, suppressing it will lead to stack corruption. I don't think much can be done about it. For context to work, the assert() expression needs to not rely on pointers into the stack frame.

(In reply to Walter Bright from comment #1) > The cause of this is that assert() may throw an exception and so the stack > reference (used by -checkaction=context) will outlive main() as the stack is > unwound. > > The error is correctly generated, suppressing it will lead to stack > corruption. I don't think much can be done about it. For context to work, > the assert() expression needs to not rely on pointers into the stack frame. No, the stack reference is not part of the exception because __assertOp4 is turned into a string before throwing an exception.

This seems to be a 2.091 regression. Using an explicit non-temporary still works, with an equivalent lifetime of the temporary: void main() @safe { int[5] a = [ 1, 2, 3, 4, 5 ]; { auto blub = retro(a[]).source; assert(blub is a[]); } } The original expression with temporary is lowered to: assert(((int[] __assertOp4 = retro(a[]).source;) , __assertOp4) is a[], _d_assert_fail(__assertOp4, a[])); // Generates the message for -checkaction=context and takes both // operands by `auto ref const`. string _d_assert_fail(ref const(int[]) a, const(int[]) b) { ... } _d_assert_msg might throw later, using the generated msg, as Florian mentioned.

(In reply to kinke from comment #3) > This seems to be a 2.091 regression. That a side effect of https://github.com/dlang/dmd/pull/10550 which made dmd use ref whenever possible. This avoids behaviour changes due to additional constructor/destructor calls when -checkaction=context is used. > Using an explicit non-temporary still works, with an equivalent lifetime of the temporary: DIP1000 handles variables from different statements just fine but struggles if multiple temporaries are used within a single statement. Note that this problem is not tied to -checkaction=context, see e.g. https://github.com/dlang/dmd/pull/10550#issuecomment-559996626

@MoonlightSentinel created dlang/dmd pull request #12258 "Fix Issue 20581 - DIP1000 wrongly flags hidden ref temporary" fixing this issue: - Fix Issue 20581 - DIP1000 wrongly flags hidden ref temporary Restrict errors for escaping by ref to `VarDeclaration`'s that actually outlive the referenced `VarDeclaration`. The only exceptions to this rule are references to other `ref` parameters https://github.com/dlang/dmd/pull/12258

dlang/dmd pull request #12258 "Fix Issue 20581 - DIP1000 wrongly flags hidden ref temporary" was merged into master: - 7fa99471be7ecaba5c9472a1a5d7f2f78c3c1fde by MoonlightSentinel: Fix Issue 20581 - DIP1000 wrongly flags hidden ref temporary Restrict errors for escaping by ref to `VarDeclaration`'s that actually outlive the referenced `VarDeclaration`. The only exceptions to this rule are references to other `ref` parameters https://github.com/dlang/dmd/pull/12258