Bug 11161 – Document the default struct equality comparison and operator overloading

Status
RESOLVED
Resolution
FIXED
Severity
major
Priority
P2
Component
dlang.org
Product
D
Version
D2
Platform
All
OS
All
Creation time
2013-10-03T00:59:38Z
Last change time
2019-04-09T08:07:34Z
Keywords
pull, spec
Assigned to
No Owner
Creator
Denis Shelomovskii

Comments

Comment #0 by verylonglogin.reg — 2013-10-03T00:59:38Z
--- struct S { bool opEquals(ref const S) { return false; } } struct T { S s; int value; alias value this; } void main() { T t1, t2; assert(t1.tupleof != t2.tupleof); assert(t1 != t2); // fails } --- If this behavior is expected, please write the reasons shortly and change it to documentation issue unless this feature is documented.
Comment #1 by issues.dlang — 2013-10-03T01:58:45Z
I would expect that alias this would have zero effect on == save for when comparing with a value of the type that's being aliased instead of with a value of the same type.
Comment #2 by k.hara.pg — 2013-10-03T02:45:37Z
(In reply to comment #0) > --- > struct S { > bool opEquals(ref const S) { return false; } > } > struct T { > S s; > int value; > alias value this; > } > > void main() { > T t1, t2; > assert(t1.tupleof != t2.tupleof); > assert(t1 != t2); // fails > } > --- > > If this behavior is expected, please write the reasons shortly and change it to > documentation issue unless this feature is documented. For the expression `t1 != t2`, the semantic analysis is done in the following order: 1. Find opEquals method from the type of operands 2. If 'alias this' declaration exists, compiler tries to resolve equality operation via alias this. -> `t1.value != t2.value` is tested, and succeeds to compile. 3. If any opEquals and comparable 'alias this' is not found, the struct equality comparison is rewritten to their each field comparison. The point is, #1 and #2 are the part of "operator overloading" feature, and it is precedence than the default struct equality operation (== field-wise comparison). Historically Historically: - By bug 3789, the default comparison behavior was changed from bitwise to member-wise. - By bug 10037, the operator overload resolution precedence order was fixed (default member-wise comparison does not hide explicitly defined alias this). Therefore, current behavior is expected.
Comment #3 by k.hara.pg — 2013-10-03T02:53:07Z
(In reply to comment #1) > I would expect that alias this would have zero effect on == save for when > comparing with a value of the type that's being aliased instead of with a value > of the same type. 'alias this' should be considered for operator overloading. If not, sub-typing by using 'alias this' won't work anymore.
Comment #4 by issues.dlang — 2013-10-03T10:55:48Z
> 'alias this' should be considered for operator overloading. If not, sub-typing by using 'alias this' won't work anymore. I would expect sub-typing via alias this to only come into effect when the values being compared were of different types. So, struct S { bool opEquals(ref const S) { return false; } } struct T { S s; int value; alias value this; } void main() { S s1; S s2; int i; //Uses built-in opEquals, because the types match auto result1 = s1 == s2; //Uses alias this because it's not comparing with another S auto result2 = s1 == i; } I don't understand why alias this would even come into effect until you're dealing with a type other than S. As I understand it, alias this is for when the original type doesn't work, in which case the conversion is done if that will work instead. And that doesn't apply here. I'd also expect the compiler to always define a default opEquals for structs if you don't define one. And that's what it does when there's no alias this involved.
Comment #5 by monarchdodra — 2013-10-06T01:18:48Z
(In reply to comment #2) > For the expression `t1 != t2`, the semantic analysis is done in the following > order: > > 1. Find opEquals method from the type of operands > 2. If 'alias this' declaration exists, compiler tries to resolve equality > operation via alias this. -> `t1.value != t2.value` is tested, and succeeds to > compile. I think this is completely bogus. Doing this means an alias this takes precedence over the type that holds it, and it should *never* happen. An alias this should never ever EVER take precedence over the base type. Here are some rebuttals to the above mentioned behavior. #1 alias this is designed to emulate "inheritance" for structs. EG: *extending* a struct. Today, it was mostly hijacked to instead do implicit conversion, where a struct can pass for some other type. The very "standard" scenario of: //---- struct Base { int i; } struct Derived { Base b; alias b this; int j; } void main() { Base b = {0}; Derived d1 = {b, 1}; Derived d2 = {b, 2}; assert(d1 != d2); //This fails } //---- The above assert is going to fail. This is completely unacceptable. Derived is a "subkind" of Base: It's Base plus some more. With the current behavior though, Derived is straight up treated as a Base, and everything that makes it a Derived is forgotten. #2. opAssign doesn't follow this rule. I'm not going to go into more details on this - I think it's obvious that opAssign and opEqual should follow the same set of rules. #3. If a struct has multiple "alias this", then comparison will probably fail spectacualrly. #4. Having a template called "hasElaborateEqual" would be virtually impossible to implement. Indeed, only the type that is resolved by alias this should be taken into account. Because of the above reason, I think that "alias this" most certainly should *not* be considered before checking if "T == T" is legal. Heck, I don't think it should be considered at *all* for "T == T": opAssign doesn't check.
Comment #6 by verylonglogin.reg — 2013-11-05T00:31:21Z
(In reply to comment #2) > 1. Find opEquals method from the type of operands > 2. If 'alias this' declaration exists, compiler tries to resolve equality > operation via alias this. -> `t1.value != t2.value` is tested, and succeeds to > compile. Just to be clear, currently we have this: --- struct S { int i, j; alias j this; } void main() { S s1 = {0}, s2 = {1}; assert(s1 == s2); // ok, resolved as `s1.j == s2.j` } --- And I'm quite sure it must be clearly documented. Also it should be mentioned one have to add: --- bool opEquals(in typeof(this) other) { return this.tupleof == other.tupleof; } --- if "old" comparison behavior is needed.
Comment #7 by timon.gehr — 2013-11-09T06:49:13Z
(In reply to comment #6) > (In reply to comment #2) > > 1. Find opEquals method from the type of operands > > 2. If 'alias this' declaration exists, compiler tries to resolve equality > > operation via alias this. -> `t1.value != t2.value` is tested, and succeeds to > > compile. > > Just to be clear, currently we have this: > --- > struct S > { > int i, j; > alias j this; > } > > void main() > { > S s1 = {0}, s2 = {1}; > assert(s1 == s2); // ok, resolved as `s1.j == s2.j` > } > --- Yah, broken behaviour. > And I'm quite sure it must be clearly documented. > > Also it should be mentioned one have to add: > --- > bool opEquals(in typeof(this) other) > { return this.tupleof == other.tupleof; } > --- > if "old" comparison behavior is needed. DMD bugs should never be carried over to the language documentation. I guess the most elegant way to fix this specific issue would be to just have all types that support a certain overloadable operator actually expose the necessary member function. E.g. 2.opBinary!"+"(3) should be legal and be treated like 2+3, where opBinary is a member of 'int'. (This would also solve the annoyance that generic code cannot rely on opCmp being present for all comparable types, though UFCS and some reflection would be sufficent to workaround this.)
Comment #8 by razvan.nitu1305 — 2019-01-24T10:02:27Z
Comment #9 by dlang-bot — 2019-03-01T11:16:25Z
@RazvanN7 updated dlang/dmd pull request #9289 "Fix Issue 16657 and 11161 - alias this interacts with generated opCmp and opEquals" fixing this issue: - Fix Issue 16657 and 11161 - alias this interacts with generated opCmp and opEquals https://github.com/dlang/dmd/pull/9289
Comment #10 by dlang-bot — 2019-04-09T08:07:34Z
dlang/dmd pull request #9289 "Fix Issue 16657 and 11161 - alias this interacts with generated opCmp and opEquals" was merged into master: - 6716c02dae44d6a186ac96af8b0e1f331bcb3069 by RazvanN7: Fix Issue 16657 and 11161 - alias this interacts with generated opCmp and opEquals https://github.com/dlang/dmd/pull/9289