www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 10567] New: Typeinfo.compare has unreasonable signature requirements on opCmp

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

           Summary: Typeinfo.compare has unreasonable signature
                    requirements on opCmp
           Product: D
           Version: D2
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: DMD
        AssignedTo: nobody puremagic.com
        ReportedBy: hsteoh quickfur.ath.cx


--- Comment #0 from hsteoh quickfur.ath.cx 2013-07-07 21:25:25 PDT ---
CODE:

------------snip-----------
import std.stdio;

struct S {
        int[] data;

        int opCmp(const S s) const {
                return (data < s.data) ? -1 : (data == s.data) ? 0 : 1;
        }
}

void main() {
        auto s = S([1,2,3]);
        auto t = S([1,2,3]);

        writeln(s==t);
        writeln(typeid(s).compare(&s, &t)); // prints 16
}
------------snip-----------

Here, we defined opCmp to compare the array wrapped in S, and == correctly
calls the custom opCmp to return true.

However, typeid(S) fails to call the custom opCmp; it appears to fall back to
the default implementation of opCmp, which does a bitwise compare of S. This is
a bug, because if the signature of opCmp is changed to:

int opCmp(ref const S s) const { ... }

then typeid(S) correctly calls the custom opCmp instead.

However, requiring ref in the argument is unnecessarily restrictive. If ==
works correctly without requiring a ref const argument, then why should
typeid(S).compare require a ref const argument?

This bug is blocking issue #8435 and issue #10118.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jul 07 2013
next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10567



--- Comment #1 from hsteoh quickfur.ath.cx 2013-07-07 21:31:24 PDT ---
Furthermore, if opCmp is a template function, it is never picked up in the
typeinfo. This makes it impossible to make typeinfo.compare behave correctly
when you need to overload opCmp on templated argument types, because an IFTI
bug makes it impossible to define both a template and non-template opCmp
simultaneously.

Why the big deal with typeinfo.compare? If == works, isn't that good enough?
It's not good enough because the AA implementation uses typeinfo.compare for
key comparisons. Thus you have the situation where two AA keys compare equal on
==, and toHash is correctly defined so that the keys have equal hash values,
but aa[key] does not work because typeinfo.compare uses the wrong key
comparison function. This is one of the underlying issues in issue #8435 and
issue #10567.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jul 07 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10567



--- Comment #2 from hsteoh quickfur.ath.cx 2013-07-07 21:34:09 PDT ---
Gah, I meant issue #10118.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jul 07 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10567



--- Comment #3 from hsteoh quickfur.ath.cx 2013-07-08 08:33:16 PDT ---
Temporary workaround: define int opCmp(ref const T t) const *before* any of the
other opCmp overloads, and have it redirect to one of them.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jul 08 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10567


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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |pull


--- Comment #4 from Kenji Hara <k.hara.pg gmail.com> 2013-07-08 19:44:29 PDT ---
Incomplete compiler fix, and supplemental druntime change.
https://github.com/D-Programming-Language/dmd/pull/2321
https://github.com/D-Programming-Language/druntime/pull/543

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jul 08 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10567



--- Comment #5 from github-bugzilla puremagic.com 2013-07-09 02:47:53 PDT ---
Commits pushed to master at https://github.com/D-Programming-Language/druntime

https://github.com/D-Programming-Language/druntime/commit/607c25a22d8d72f2d7fb5f81c861c7e54534101e
Supplemental fix for issue 10567

Add helper function in druntime, same as opEquals case.

https://github.com/D-Programming-Language/druntime/commit/d46b4eb096c2246c59a161a1d4e5494b38d784d0
Merge pull request #543 from 9rnsr/fix10567

Supplemental fix for issue 10567

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jul 09 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10567



--- Comment #6 from github-bugzilla puremagic.com 2013-07-21 14:02:34 PDT ---
Commits pushed to master at https://github.com/D-Programming-Language/dmd

https://github.com/D-Programming-Language/dmd/commit/acd073afcb89d639c8c99cd7f8233788db6036d6
fix Issue 10567 - Typeinfo.compare has unreasonable signature requirements on
opCmp

If needed, generate `__xopEquals` method in order to adapt `opCmp` member
function (even if it's template) to the function pointer
`TypeInfo_Struct.xopCmp`.

https://github.com/D-Programming-Language/dmd/commit/19988b6d9d854f68e3984f871f0e8977a7be1f09
Merge pull request #2321 from 9rnsr/fix10567

Partial fix for Issue 10567 - Typeinfo.compare has unreasonable signature
requirements on opCmp

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jul 21 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10567



--- Comment #7 from Kenji Hara <k.hara.pg gmail.com> 2013-07-22 23:59:56 PDT ---
https://github.com/D-Programming-Language/dmd/pull/2374

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jul 22 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10567



--- Comment #8 from github-bugzilla puremagic.com 2013-07-25 02:24:27 PDT ---
Commits pushed to master at https://github.com/D-Programming-Language/dmd

https://github.com/D-Programming-Language/dmd/commit/f7d34abe745751326f26dc3ac203af01e151bdea
Additional fix for issue 10567 in order to generate correct TypeInfo

https://github.com/D-Programming-Language/dmd/commit/e25184bce39f87438e8170af624ea1bd35bbc7cc
Merge pull request #2374 from 9rnsr/fix10567

Additional fix for issue 10567 in order to generate correct TypeInfo

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jul 25 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10567



--- Comment #9 from hsteoh quickfur.ath.cx 2013-07-25 11:05:34 PDT ---
Hi Kenji,

It seems that the latest pull has fixed this bug. Is there anything else that
must be fixed? (You mentioned that this was only a "partial" fix?)

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jul 25 2013
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10567



--- Comment #10 from Kenji Hara <k.hara.pg gmail.com> 2013-08-05 02:54:04 PDT
---
(In reply to comment #9)
 Hi Kenji,
 
 It seems that the latest pull has fixed this bug. Is there anything else that
 must be fixed? (You mentioned that this was only a "partial" fix?)
The remain issue is that does not detect alias this opCmp correctly. struct X { int opCmp(X) { return 0; } } struct S { int val; X x; alias x this; } void main() { S s1 = S(1); S s2 = S(2); assert(!(s1 < s2) && !(s1 > s2)); // OK assert(s1.opCmp(s2) == 0); // OK assert(typeid(S).compare(&s1, &s2) == 0); // doesn't work } -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Aug 05 2013