http://d.puremagic.com/issues/show_bug.cgi?id=9957

           Summary: [2.061 -> 2.062] Taking pointer of enum float array
                    gives some garbage
           Product: D
           Version: D2
          Platform: All
        OS/Version: Mac OS X
            Status: NEW
          Severity: regression
          Priority: P2
         Component: DMD
        AssignedTo: nobody@puremagic.com
        ReportedBy: stdin@kraybit.com


--- Comment #0 from stdin@kraybit.com 2013-04-18 08:09:28 PDT ---
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

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=9957



--- Comment #1 from Don <clugdbug@yahoo.com.au> 2013-04-22 23:53:23 PDT ---
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.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=9957



--- Comment #2 from Kenji Hara <k.hara.pg@gmail.com> 2013-04-23 00:10:16 PDT ---
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.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=9957


Kenji Hara <k.hara.pg@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |pull, wrong-code
         OS/Version|Mac OS X                    |All


--- Comment #3 from Kenji Hara <k.hara.pg@gmail.com> 2013-04-23 00:36:26 PDT ---
https://github.com/D-Programming-Language/dmd/pull/1922

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=9957



--- Comment #4 from Don <clugdbug@yahoo.com.au> 2013-04-23 01:39:09 PDT ---
(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.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=9957



--- Comment #5 from Kenji Hara <k.hara.pg@gmail.com> 2013-04-23 01:56:43 PDT ---
(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.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=9957


Walter Bright <bugzilla@digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bugzilla@digitalmars.com


--- Comment #6 from Walter Bright <bugzilla@digitalmars.com> 2013-04-23 10:41:29 PDT ---
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.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=9957



--- Comment #7 from Kenji Hara <k.hara.pg@gmail.com> 2013-04-23 19:28:53 PDT ---
(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?

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=9957



--- Comment #8 from Don <clugdbug@yahoo.com.au> 2013-04-24 04:05:58 PDT ---
(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.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=9957


bearophile_hugs@eml.cc changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bearophile_hugs@eml.cc


--- Comment #9 from bearophile_hugs@eml.cc 2013-04-24 04:59:52 PDT ---
(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.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------