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

           Summary: Nullable is unusable with 2.061
           Product: D
           Version: D2
          Platform: x86_64
        OS/Version: Linux
            Status: NEW
          Severity: regression
          Priority: P2
         Component: DMD
        AssignedTo: nobody@puremagic.com
        ReportedBy: lomereiter@gmail.com


--- Comment #0 from Artem Tarasov <lomereiter@gmail.com> 2013-01-26 07:43:28 PST ---
http://dpaste.dzfl.pl/5bf80d80

import std.typecons;

struct Foo {
    Nullable!int n;
}

struct Bar {
    Foo wtf;
    this(Foo t) {
        wtf = t;
    }
}

void main() {
    Foo foo;
    Bar(foo);
}

This code throws an exception with the following backtrace:

object.Exception@/home/lomereiter/.dvm/compilers/dmd-2.061/bin/../src/phobos/std/typecons.d(1181): Enforcement failed
----------------
./wtf(bool std.exception.enforce!(bool).enforce(bool, lazy const(char)[],
immutable(char)[], ulong)+0x65) [0x416c45]
./wtf(std.typecons.Nullable!(int).Nullable.getinout(pure ref @property @safe
inout(int) function())+0x89) [0x416a45]
./wtf(ref wtf.Foo wtf.Foo.opAssign(wtf.Foo)+0x51) [0x416b05]
./wtf(wtf.Bar wtf.Bar.__ctor(wtf.Foo)+0x18) [0x416b30]

-- 
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=9404



--- Comment #1 from Artem Tarasov <lomereiter@gmail.com> 2013-01-26 07:55:13 PST ---
Further reduced case:

import std.typecons;

alias N = Nullable!int;

void foo(N a) {
    N b;
    b = a; // `N b = a;` works fine
}

void main() {
    N n;
    foo(n);
}

-- 
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=9404


monarchdodra@gmail.com changed:

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


--- Comment #2 from monarchdodra@gmail.com 2013-01-26 13:21:56 PST ---
Here is a preliminary analysis:

There is no opAssign(Nullable!T t).

Because of this doing a "nullable1 = nullable2" will actually trigger
"nullable1 = nullable2.get", at which point an exception is thrown. In the
second example:
    b = a; //This fails regardless of DMD

This can be fixed with:
//----
    void opAssign()(Nullable!T other)
    {
        _value  = other._value;
        _isNull = other._isNull;
    }
//----
That said I wonder if this is correct behavior: if "a" is null, then should "b
= a" work? I think so, but I can see reasons why it shouldn't.


----------------
The real question is: Why did the first example ever work at all? I've tracked it down to this usecase, with *at least*, a 3-level nest:

//----
import std.typecons;
import std.stdio;

struct S
{
    void opAssign(int){writeln("opAssign");}
    int get(){writeln("get"); return 1;}
    alias get this;
}

struct SS
{
    S s;
}

struct SSS
{
    SS ss;
    this(SS i)
    {
        ss = i;
    }
}

void main() {
   SS ss;
   SSS(ss);
}
//----

DMD 2.060:
//----
//----

DMD 2.061:
//----
get
opAssign
//----

AFAIK, 2.061 is the correct behavior. Something must have been fixed in 2.061, so that explains the *change* in behavior. So there should be nothing to fix there... But I'll let own of the compiler guys confirm.

In any case, I'll fix open a pull to improve Nullable to accept assignment from another nullable.

-- 
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=9404



--- Comment #3 from Artem Tarasov <lomereiter@gmail.com> 2013-01-26 13:33:21 PST ---
Thanks for your analysis!

I wonder, though, how is this correct behaviour? My understanding is, since SS doesn't have opAssign, bitwise copy should happen in SSS constructor, and that's it. Why does S.opAssign get called in this case?

> AFAIK, 2.061 is the correct behavior. Something must have been fixed in 2.061, so that explains the *change* in behavior. So there should be nothing to fix there... But I'll let own of the compiler guys confirm.
> 
> In any case, I'll fix open a pull to improve Nullable to accept assignment from another nullable.

-- 
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=9404


monarchdodra@gmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|regression                  |enhancement


--- Comment #4 from monarchdodra@gmail.com 2013-01-26 13:42:10 PST ---
(In reply to comment #3)
> Thanks for your analysis!
> 
> I wonder, though, how is this correct behaviour? My understanding is, since SS doesn't have opAssign, bitwise copy should happen in SSS constructor, and that's it. Why does S.opAssign get called in this case?
> 
> > AFAIK, 2.061 is the correct behavior. Something must have been fixed in 2.061, so that explains the *change* in behavior. So there should be nothing to fix there... But I'll let own of the compiler guys confirm.
> > 
> > In any case, I'll fix open a pull to improve Nullable to accept assignment from another nullable.

Technically, when S doesn't have an opAssign, then a *postblit* is called (which will basically be a bitwise copy when there is no postblit).

However, this only happens when there is NO opAssign at all. If there is an opAssign, then the compiler must call one of the existing opAssigns. Nullable does have an opAssign though (the one taking a T), so that is what the compiler is calling.

-- 
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=9404



--- Comment #5 from Artem Tarasov <lomereiter@gmail.com> 2013-01-26 14:32:15 PST ---
OK, apparently default opAssign behaviour has changed to call opAssign of member fields since merging this pull request: https://github.com/D-Programming-Language/dmd/pull/166

Documentation at http://dlang.org/struct.html#AssignOverload is outdated, then.

> 
> Technically, when S doesn't have an opAssign, then a *postblit* is called (which will basically be a bitwise copy when there is no postblit).
> 
> However, this only happens when there is NO opAssign at all. If there is an opAssign, then the compiler must call one of the existing opAssigns. Nullable does have an opAssign though (the one taking a T), so that is what the compiler is calling.

-- 
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=9404


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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |pull, rejects-valid
           Platform|x86_64                      |All
         OS/Version|Linux                       |All
           Severity|enhancement                 |regression


--- Comment #6 from Kenji Hara <k.hara.pg@gmail.com> 2013-01-30 20:29:48 PST ---
https://github.com/D-Programming-Language/dmd/pull/1585 https://github.com/D-Programming-Language/phobos/pull/1105

---

(In reply to comment #2)
> ----------------
> The real question is: Why did the first example ever work at all? I've tracked it down to this usecase, with *at least*, a 3-level nest:
> 
> //----
> import std.typecons;
> import std.stdio;
> 
> struct S
> {
>     void opAssign(int){writeln("opAssign");}
>     int get(){writeln("get"); return 1;}
>     alias get this;
> }
> 
> struct SS
> {
>     S s;
> }
> 
> struct SSS
> {
>     SS ss;
>     this(SS i)
>     {
>         ss = i;
>     }
> }
> 
> void main() {
>    SS ss;
>    SSS(ss);
> }
> //----
> 
> DMD 2.060:
> //----
> //----
> 
> DMD 2.061:
> //----
> get
> opAssign
> //----
> 
> AFAIK, 2.061 is the correct behavior. Something must have been fixed in 2.061, so that explains the *change* in behavior. So there should be nothing to fix there... But I'll let own of the compiler guys confirm.
> 
> In any case, I'll fix open a pull to improve Nullable to accept assignment from another nullable.

Yes, it is intended behavior in 2.061. The pull request had merged:
https://github.com/D-Programming-Language/dmd/pull/166
Was for implementing it.

But, sorry, it was not complete. In 2.061, user-defined opAssign is now _overly_ considered and used for the identity assignment. It is a compiler regression.

In this case, Nullable!T should generate bitwise copy for identity assignment.

> alias N = Nullable!int;
> void foo(N a) {
>     N b;
>     b = a; // must be bitwise, must not invoke Nullable!int.opAssign(int)
> }

If the assignment is not identity, user-defined opAssing should be called.

  void foo(N a) {
      N b;
      b = 1;  // b.opAssign(1) is called
      b = ""; // b.opAssign("") is called, and compiler will cause an error
  }

-- 
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=9404



--- Comment #7 from monarchdodra@gmail.com 2013-01-30 23:19:47 PST ---
(In reply to comment #6)
> https://github.com/D-Programming-Language/dmd/pull/1585 https://github.com/D-Programming-Language/phobos/pull/1105
> 
> ---
> 
> (In reply to comment #2)
> > ----------------
> > The real question is: Why did the first example ever work at all? I've tracked it down to this usecase, with *at least*, a 3-level nest:
> > 
> > //----
> > import std.typecons;
> > import std.stdio;
> > 
> > struct S
> > {
> >     void opAssign(int){writeln("opAssign");}
> >     int get(){writeln("get"); return 1;}
> >     alias get this;
> > }
> > 
> > struct SS
> > {
> >     S s;
> > }
> > 
> > struct SSS
> > {
> >     SS ss;
> >     this(SS i)
> >     {
> >         ss = i;
> >     }
> > }
> > 
> > void main() {
> >    SS ss;
> >    SSS(ss);
> > }
> > //----
> > 
> > DMD 2.060:
> > //----
> > //----
> > 
> > DMD 2.061:
> > //----
> > get
> > opAssign
> > //----
> > 
> > AFAIK, 2.061 is the correct behavior. Something must have been fixed in 2.061, so that explains the *change* in behavior. So there should be nothing to fix there... But I'll let own of the compiler guys confirm.
> > 
> > In any case, I'll fix open a pull to improve Nullable to accept assignment from another nullable.
> 
> Yes, it is intended behavior in 2.061. The pull request had merged:
> https://github.com/D-Programming-Language/dmd/pull/166
> Was for implementing it.
> 
> But, sorry, it was not complete. In 2.061, user-defined opAssign is now _overly_ considered and used for the identity assignment. It is a compiler regression.
> 
> In this case, Nullable!T should generate bitwise copy for identity assignment.
> 
> > alias N = Nullable!int;
> > void foo(N a) {
> >     N b;
> >     b = a; // must be bitwise, must not invoke Nullable!int.opAssign(int)
> > }
> 
> If the assignment is not identity, user-defined opAssing should be called.
> 
>   void foo(N a) {
>       N b;
>       b = 1;  // b.opAssign(1) is called
>       b = ""; // b.opAssign("") is called, and compiler will cause an error
>   }

My apologies for not linking back: https://github.com/D-Programming-Language/phobos/pull/1103

This is now a useless pull... correct?

-- 
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=9404



--- Comment #8 from Kenji Hara <k.hara.pg@gmail.com> 2013-01-30 23:35:44 PST ---
(In reply to comment #7)
> (In reply to comment #6)
> > https://github.com/D-Programming-Language/dmd/pull/1585 https://github.com/D-Programming-Language/phobos/pull/1105
> 
> My apologies for not linking back: https://github.com/D-Programming-Language/phobos/pull/1103
> 
> This is now a useless pull... correct?

Yes.

-- 
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=9404



--- Comment #9 from github-bugzilla@puremagic.com 2013-01-31 01:47:17 PST ---
Commits pushed to master at https://github.com/D-Programming-Language/phobos

https://github.com/D-Programming-Language/phobos/commit/c753619ffeffa79ef237872bae26275501525166 fix Issue 9404 - Nullable is unusable with 2.061

https://github.com/D-Programming-Language/phobos/commit/06b8d3734fbbc35c90459dd5c8ef2335b0872d2d Merge pull request #1105 from 9rnsr/fix_assign

Supplemental changes for Issue 9404

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