www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 12024] New: [REG DMD2.065-b2] template instantiation for swap(SysTime, SysTime) fails

reply d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12024

           Summary: [REG DMD2.065-b2] template instantiation for
                    swap(SysTime, SysTime) fails
           Product: D
           Version: D2
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: regression
          Priority: P2
         Component: Phobos
        AssignedTo: nobody puremagic.com
        ReportedBy: sludwig outerproduct.org



PST ---
Still works on 2.064. This bug in particular makes it impossible to sort an
array of SysTimes or an array of structs containing SysTime. Not sure if the
root cause lies in a Phobos change or in a DMD change.

---
import std.algorithm;
void main() {
  SysTime a, b;
  swap(a, b);
}
---

bug_sort.d(4): Error: template std.algorithm.swap cannot deduce function from
argument types !()(SysTime, SysTime), candidates are:
C:\D\dmd2\windows\bin\..\..\src\phobos\std\algorithm.d(1997):       
std.algorithm.swap(T)(ref T lhs, ref T rhs) if (allMutableFields!T &&
!is(typeof(T.init.proxySwap(T.init))))
C:\D\dmd2\windows\bin\..\..\src\phobos\std\algorithm.d(2042):       
std.algorithm.swap(T)(T lhs, T rhs) if (is(typeof(T.init.proxySwap(T.init))))

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jan 29 2014
next sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12024


Vladimir Panteleev <thecybershadow gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |thecybershadow gmail.com



13:21:30 EET ---
Caused by https://github.com/D-Programming-Language/phobos/pull/1603

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jan 29 2014
prev sibling next sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12024


monarchdodra gmail.com changed:

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




 Caused by https://github.com/D-Programming-Language/phobos/pull/1603


Indeed. However, as I explained there, I think it is legit behavior:

SysTime contains a rebindable, and Rebindable contains an immutable, and swap
can't make the choice to mutate immutable.

That said, the issue could be in Rebindable to being with: It stores an
immutable member, but obviously mutates it all the time. In that case, why
bother storing an immutable at all? I think the fix is there.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jan 29 2014
prev sibling next sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12024




14:14:37 EET ---

 SysTime contains a rebindable, and Rebindable contains an immutable, and swap


can't make the choice to mutate immutable.

Actually, it contains a union between mutable and immutable. And unions imply
that all guarantees are off.

So, maybe the solution would be to make swap allow an immutable field if it's
in an union with a mutable field?

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jan 29 2014
prev sibling next sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12024






 Actually, it contains a union between mutable and immutable. And unions imply
 that all guarantees are off.
 
 So, maybe the solution would be to make swap allow an immutable field if it's
 in an union with a mutable field?


There is currently (AFAIK) no way to introspect that a member is part of an
anonymous union. It *could* make for a partial solution though, yes.

Also, aren't there cases where an union contains *all* const members?

In any case, yes, there is room for improvement.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jan 29 2014
prev sibling next sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12024




14:59:47 EET ---
Well, the regression needs to be solved one way or another. Not being able to
sort an array of timestamps is pretty bad.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jan 29 2014
prev sibling next sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12024






 Well, the regression needs to be solved one way or another.


Absolutly.


 Not being able to sort an array of timestamps is pretty bad.


If your original issue is that of sorting timestamps, then you may also be
interested in:

https://d.puremagic.com/issues/show_bug.cgi?id=11129

Because timestamps *are* assignable, so the fact that the elements aren't
swappable should not be preventing you from sorting them.

I think it was when I was originally looking into that issue that I introduced
that check. Small incremental improvements I guess.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jan 29 2014
prev sibling next sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12024


yebblies <yebblies gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |yebblies gmail.com





 
 There is currently (AFAIK) no way to introspect that a member is part of an
 anonymous union. It *could* make for a partial solution though, yes.
 


It should be possibly by comparing .offsetof with other members.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jan 29 2014
prev sibling next sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12024






 Caused by https://github.com/D-Programming-Language/phobos/pull/1603


 
 Indeed. However, as I explained there, I think it is legit behavior:
 
 SysTime contains a rebindable, and Rebindable contains an immutable, and swap
 can't make the choice to mutate immutable.
 
 That said, the issue could be in Rebindable to being with: It stores an
 immutable member, but obviously mutates it all the time. In that case, why
 bother storing an immutable at all? I think the fix is there.


If a non-mutable field has one or more overlapped union _mutable_ fields, the
whole struct is treated as modifiable.

import std.traits : Unqual;
struct Rebindable(T)
{
    union
    {
        T origin;
        Unqual!T stripped;  // overlapped union mutable field of 'origin'
    }
}
void main()
{
    Rebindable!int r1 = {origin:10};
    Rebindable!int r2 = {origin:20};
    r1 = r2;   // Rebindable!int is modifiable (assignable)
               // even if non-mutable field `T origin;` exists.
    assert(r1.origin == 20);
}

If all of overlapped union fields are non-mutable, the whole struct is not also
modifiable.

struct S
{
    union
    {
        immutable int x;
        immutable int y;
    }
}
void main()
{
    S s1 = {x:10};
    S s2 = {x:20};
    s1 = s2;  // cannot modify struct s1 S with immutable members
}

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jan 29 2014
prev sibling next sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12024




16:05:44 EET ---
Interesting. So this should "just work":

diff --git a/std/algorithm.d b/std/algorithm.d
index 036b918..0ed5606 100644
--- a/std/algorithm.d
+++ b/std/algorithm.d
   -2054,6 +2054,9    void swap(T)(ref T lhs, ref T rhs) if
(is(typeof(lhs.proxySwap(rhs))))
 private template allMutableFields(T)
 {
     alias OT = OriginalType!T;
+    static if (is(typeof({ T t = void; t = t; })))
+        enum allMutableFields = true;
+    else
     static if (is(OT == struct) || is(OT == union))
         enum allMutableFields = isMutable!OT && allSatisfy!(.allMutableFields,
FieldTypeTuple!OT);
     else
   -2072,6 +2075,9    unittest
     struct S2{const int i;}
     static assert( allMutableFields!S1);
     static assert(!allMutableFields!S2);
+
+    struct S3{union X{int m;const int c;}X x;}
+    static assert( allMutableFields!S3);
 }

 unittest

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jan 29 2014
prev sibling next sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12024




16:07:02 EET ---
Derp, unit test line should be:

    struct S3{union{int m;const int c;}}

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jan 29 2014
prev sibling next sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12024






 It should be possibly by comparing .offsetof with other members.


Smart. Unfortunatly, for a "recursive" implementation, it is a bit difficult to
exploit: More often than not, you want to know before hand that you are about
to process an aggregate in an union.



 If a non-mutable field has one or more overlapped union _mutable_ fields, the
 whole struct is treated as modifiable.
 
 import std.traits : Unqual;
 struct Rebindable(T)
 {
     union
     {
         T origin;
         Unqual!T stripped;  // overlapped union mutable field of 'origin'
     }
 }


Yes, absolutely. That's what I had in mind. But as I said, even detecting that
you are in an (anonymous) union is a bit difficult. Do-able with your
suggestion, just... difficult.



 Interesting. So this should "just work":
 
 diff --git a/std/algorithm.d b/std/algorithm.d
 index 036b918..0ed5606 100644
 --- a/std/algorithm.d
 +++ b/std/algorithm.d
    -2054,6 +2054,9    void swap(T)(ref T lhs, ref T rhs) if
 (is(typeof(lhs.proxySwap(rhs))))
  private template allMutableFields(T)
  {
      alias OT = OriginalType!T;
 +    static if (is(typeof({ T t = void; t = t; })))
 +        enum allMutableFields = true;
 +    else
      static if (is(OT == struct) || is(OT == union))
          enum allMutableFields = isMutable!OT && allSatisfy!(.allMutableFields,
 FieldTypeTuple!OT);
      else
    -2072,6 +2075,9    unittest
      struct S2{const int i;}
      static assert( allMutableFields!S1);
      static assert(!allMutableFields!S2);
 +
 +    struct S3{union X{int m;const int c;}X x;}
 +    static assert( allMutableFields!S3);
  }


I think that's wrong, because "static if (is(typeof({ T t = void; t = t; })))"
can work in the pressence of an opAssign: Complex structs with immutable
members but with a valid opAssign can't be swapped. The only case where it
would work is if T didn't have an opAssign to begin with. However, that test
would be mostly useless, since we'd have to deploy code for structs that do
have opAssign regardless.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jan 29 2014
prev sibling next sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12024




---




 It should be possibly by comparing .offsetof with other members.


 
 Smart. Unfortunatly, for a "recursive" implementation, it is a bit difficult to
 exploit: More often than not, you want to know before hand that you are about
 to process an aggregate in an union.
 


 If a non-mutable field has one or more overlapped union _mutable_ fields, the
 whole struct is treated as modifiable.
 
 import std.traits : Unqual;
 struct Rebindable(T)
 {
     union
     {
         T origin;
         Unqual!T stripped;  // overlapped union mutable field of 'origin'
     }
 }


 
 Yes, absolutely. That's what I had in mind. But as I said, even detecting that
 you are in an (anonymous) union is a bit difficult. Do-able with your
 suggestion, just... difficult.


I think adding std.traits.isBlitAssignable(T) for the purpose would be good.
And you can refer the code in the compiler.

https://github.com/D-Programming-Language/dmd/blob/master/src/mtype.c#L8588
int TypeStruct::isAssignable()

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jan 29 2014
prev sibling next sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12024


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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |pull, rejects-valid



---
https://github.com/D-Programming-Language/phobos/pull/1891

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jan 30 2014
prev sibling next sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12024




Commits pushed to master at https://github.com/D-Programming-Language/phobos

https://github.com/D-Programming-Language/phobos/commit/2d32c78af43a3d0a1b9015a8c971cefed9a1035f
fix Issue 12024 - template instantiation for swap(SysTime, SysTime) fails

https://github.com/D-Programming-Language/phobos/commit/b8f242e78f587f41aa344173b8961bf613e20c0d


[REG2.065a] Issue 12024 - template instantiation for swap(SysTime, SysTime)
fails

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jan 31 2014
prev sibling next sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12024


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

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


-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jan 31 2014
prev sibling next sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12024




Commit pushed to 2.065 at https://github.com/D-Programming-Language/phobos

https://github.com/D-Programming-Language/phobos/commit/84422029eb26ccea2cd97422c2e731792ae1e114


[REG2.065a] Issue 12024 - template instantiation for swap(SysTime, SysTime)
fails
Conflicts:
    std/algorithm.d

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Feb 01 2014
prev sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12024




Commit pushed to master at https://github.com/D-Programming-Language/phobos

https://github.com/D-Programming-Language/phobos/commit/84422029eb26ccea2cd97422c2e731792ae1e114


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