Fix Issue 11591 (std.typecons.Tuple regression) by workarounding Issue 11557.
#1707
Conversation
| return field[i] < rhs.field[i] ? -1 : 1; | ||
| // Workaround dmd @@@BUG11557@@@ | ||
| static if(is(Unused == class)) | ||
| return cast() field[i] < cast() rhs.field[i] ? -1 : 1; |
There was a problem hiding this comment.
Semi on topic, but is this kind of cast documented? I couldn't find it any? I think it basically casts away the qualifiers? It's the same as cast(Unqual!(typeof(arg))(arg); ? Or are there more subtleties?
There was a problem hiding this comment.
It is documented. Just search for cast() at cast documentation.
There was a problem hiding this comment.
Ah. I couldn't find it. I get it now. I was actually wondering how to make a qualified cast to "no qualification", I have my answer. Thanks.
There was a problem hiding this comment.
I think the "correct" cast should be cast(Type[i]): cast() strips all qualifiers, regardless of what the original qualifiers on the type were. Currently, Tuples don't support const fields, but they could. Furthermore, they currently do support shared fields, so that shouldn't be stripped away when calling opCmp.
|
This fixes a nasty regression. Pull as soon as possible please. |
What regression? This hasn't been working for as far back as 2.060 (haven't checked any further). Could you show which you are thinking about exactly?
I'm concerned about the const casting you are doing here. If the types in the type tuple can't be ordered when const, then I don't see why it would be expected for the tuple itself to be order-able. There is really nothing that justifies that the cast is safe. |
At least year ago
It is needed because of druntime Issue 1824. And dmd already calls |
Right... Array keys :/ I was testing them as values. My bad. Indeed, that is a regression.
I get what you are trying to bypass, I'm pointing out it is also breaking the type system.
Right, but inconsistency/bugs isn't an excuse to introduce more inconsistency, bugs elsewhere. That said, this bug seems to be pretty nasty indeed, so I think it there is a net gain to doing this. |
|
Today, const class types cannot become key of AAs. Therefore Tuple of const class should also be impossible to become key of AAs. It is correct behavior from type system. We should be careful to add specific workaround in Phobos or druntime, because they are core parts of D language. From the long-term view, I think that keeping the current broken behavior would have certain amount benefit, that will make easy to fix language in near the future. |
|
I filed Issue 11591 for
Actually they can, filed Issue 11589. Also any uncomparable type can, filed Issue 11590. And finally associative arrays work with unoverriden
Nop. Runtime failures because of type system has no excuse. Make it compile-time at least. As for current situation with |
Issue URL: https://d.puremagic.com/issues/show_bug.cgi?id=11591 Workarounding issue URL: https://d.puremagic.com/issues/show_bug.cgi?id=11557
Is it possible to add more inconsistency in D associative arrays? It's really obscure what the hell it is doing in the language and runtime complicating supporter lives instead of just being a regular library collection. |
|
_AA_s fixed now. |
Issue URL: https://d.puremagic.com/issues/show_bug.cgi?id=11591
Workarounding issue URL: https://d.puremagic.com/issues/show_bug.cgi?id=11557