Bug 9957 – [2.061 -> 2.062] Taking pointer of enum float array gives some garbage

import std.stdio; void main() { enum float[3][1] A = [[1.0, 2.0, 3.0]]; auto a = A[0].ptr; writeln(a[0], " should be ", A[0][0]); writeln(a[1], " should be ", A[0][1]); writeln(a[2], " should be ", A[0][2]); } ———— > rdmd -m32 test.d 1 should be 1 1.76941e-40 should be 2 -1.98565 should be 3 > rdmd -m64 test.d 1 should be 1 1.4013e-45 should be 2 4.33594e+15 should be 3 > _ ———— Example output above. Changes every run. Worked in 2.061. Don't know if it's supposed to work, but if not, should at least throw an error? MacOS 10.8.3 DMD 2.062

I don't think this should compile. It's like writing auto p = &5; -- taking the address of a manifest constant doesn't make sense.

This is a regression caused by fixing bug 8913. (In reply to comment #1) > I don't think this should compile. It's like writing auto p = &5; > -- taking the address of a manifest constant doesn't make sense. I think it should work. Manifest constant with array value will create array literal, so: auto a = A[0].ptr; will be optimized to: auto a = [1.0, 2.0, 3.0].ptr; By fixing bug 8913, accessing sarr.ptr is now lowered to &sarr[0]. With this, above code is also lowered to: auto a = &[1.0, 2.0, 3.0][0]; Then currently it is optimized to: auto a = &1.0; I think that the root cause of this bug is here. Essentially optimizer should keep lvalue-ness of expressions, but it is accidentally disappeared in the last optimization. It is incorrect.

(In reply to comment #2) > This is a regression caused by fixing bug 8913. > > (In reply to comment #1) > > I don't think this should compile. It's like writing auto p = &5; > > -- taking the address of a manifest constant doesn't make sense. > > I think it should work. Manifest constant with array value will create array > literal, so: > > auto a = A[0].ptr; > > will be optimized to: > > auto a = [1.0, 2.0, 3.0].ptr; I think that's not valid. That isn't A[0].ptr It's A[0].dup.ptr.

(In reply to comment #4) > (In reply to comment #2) > > I think it should work. Manifest constant with array value will create array > > literal, so: > > > > auto a = A[0].ptr; > > > > will be optimized to: > > > > auto a = [1.0, 2.0, 3.0].ptr; > > I think that's not valid. That isn't A[0].ptr It's A[0].dup.ptr. Hmm... Sure, in this case the array literal `[1.0, 2.0, 3.0]` has a _static array type_ float[3]. So getting its memory address is not well defined in current D language spec. I'd like to hear Walter's opinion.

I agree with Don. Enums do not have an address (even if under the hood they do). Users should use 'const' or 'immutable' if they need it to have an address.

(In reply to comment #6) > I agree with Don. Enums do not have an address (even if under the hood they > do). Users should use 'const' or 'immutable' if they need it to have an > address. OK. It's not bad design. But implementing it is not so easy. Because 1. Current dmd does not have such information internally. CTFE engine now uses ownedByCtfe flag, but using it in semantic analysis is not intended. 2. It is not well designed enough. For example: enum float[] A = [1.0, 2.0, 3.0]; auto a = A.ptr; //? In this case, the elements of A would be allocated on the runtime heap, or "not addressable compile time memory"? Currently it operates as the former, but it should be prohibited? Anyway, doing all in 2.063 beta phase is difficult. Therefore I'd like to propose that: 1. We once fix this "regression" and make the OP code workable in 2.063, by merging my patch. 2. Later define the concept well and implement it. How about?

(In reply to comment #7) > (In reply to comment #6) > > I agree with Don. Enums do not have an address (even if under the hood they > > do). Users should use 'const' or 'immutable' if they need it to have an > > address. > > OK. It's not bad design. But implementing it is not so easy. Because > > 1. Current dmd does not have such information internally. CTFE engine now uses > ownedByCtfe flag, but using it in semantic analysis is not intended. > > 2. It is not well designed enough. For example: > > enum float[] A = [1.0, 2.0, 3.0]; > auto a = A.ptr; //? > > In this case, the elements of A would be allocated on the runtime heap, or "not > addressable compile time memory"? Currently it operates as the former, but it > should be prohibited? I think it should be prohibited. In fact, the problem starts here: > enum float[] A = [1.0, 2.0, 3.0]; I think, a bit controversially, that this line shouldn't compile. See bug 9953. An enum of a reference type doesn't make sense, since it implicitly requires an address. > Anyway, doing all in 2.063 beta phase is difficult. I agree. > > Therefore I'd like to propose that: > 1. We once fix this "regression" and make the OP code workable in 2.063, by > merging my patch. That's probably the easiest thing to do.

(In reply to comment #8) > > enum float[] A = [1.0, 2.0, 3.0]; > > I think, a bit controversially, that this line shouldn't compile. > See bug 9953. > An enum of a reference type doesn't make sense, since it implicitly requires an > address. I kind of agree with this change, but it's a large change, so I think it should be presented and discussed in the main D newsgroup.

Commits pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/dbc85a5d1f56cb74bc5bc16ac4fac3b133c1c07b fix Issue 9957 - Taking pointer of enum float array gives some garbage https://github.com/D-Programming-Language/dmd/commit/6c95d9b80eb1e597dda2d406baecafb90734f8c5 Merge pull request #1922 from 9rnsr/fix9957 [REG2.062] Issue 9957 - Taking pointer of enum float array gives some garbage

Commit pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/7518f83d0082e5cd339ef5d7c215f13a0287ad4b Do not include test case for issue 9957 It's not behavior that we guarantee.

In git head (and maybe in 2.063 release) the codegen bug has been fixed. So I'll mark this as "resolved fixed". But, taking address of the part of enum value is not guaranteed behavior in the language spec. It would be disallowed in the future release. Do not rely on the current behavior.

Commit pushed to 2.063 at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/95032fff948cbdbb63c31c7d9f19b39fe5117763 Merge pull request #1922 from 9rnsr/fix9957 [REG2.062] Issue 9957 - Taking pointer of enum float array gives some garbage