www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 9404] New: Nullable is unusable with 2.061

reply d-bugmail puremagic.com writes:
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: -------
Jan 26 2013
next sibling parent d-bugmail puremagic.com writes:
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: -------
Jan 26 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Jan 26 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Jan 26 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Jan 26 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Jan 26 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Jan 30 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Jan 30 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Jan 30 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Jan 31 2013
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=9404


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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED


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