Bug 15957 – Disabled postblit + template mixin break opAssign with confusing error message

Status
RESOLVED
Resolution
FIXED
Severity
major
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2016-04-26T00:21:00Z
Last change time
2017-08-02T08:07:08Z
Keywords
industry
Assigned to
nobody
Creator
public

Comments

Comment #0 by public — 2016-04-26T00:21:16Z
``` struct S { @disable this(this); mixin Impl; } template Impl ( ) { void opAssign ( int[] x ) { } } unittest { S s; s = [ 1, 2, 3 ]; } // function wat.S.opAssign (S p) is not callable using argument types (int[]) ``` Either moving opAssign from template to struct directly or removing `@disable this(this)` makes the snippet compile.
Comment #1 by public — 2016-04-26T12:48:46Z
(checked that issue is present at least on 2.070 and 2.071)
Comment #2 by bugzilla — 2016-04-27T05:27:48Z
Comment #3 by bugzilla — 2016-04-27T05:58:42Z
(In reply to Walter Bright from comment #2) > https://github.com/dlang/dmd/pull/5713 That's the wrong answer. What's happening is, when the compiler sees the this(this), is it says "I need to create an identity assignment operator S.opAssign(S)." It proceeds to do so, and inserts it into the symbol table of S as a member function. Then, when: s = [1,2,3]; is encountered, it does a lookup of opAssign. It finds S.opAssign(S), and of course that fails to match. Mixed in templates are considered imports as far as looking things up goes. The member functions are not overloaded against them, the member functions override them. This entirely explains the behavior you are reporting. Trying to change the lookup rules will have all kinds of ripple effects, and it's hard to see what they might all be in advance. It would be a high risk change. But there's an easy fix. Add opAssign(ref S) to the mixed in template: template Impl(S) { void opAssign(int[] x) { } @disable void opAssign(ref S); } Now, a default opAssign(S) will not be generated, the lookup will find Impl.opAssign, the two entries will overload against each other, and the first one is the better match.
Comment #4 by public — 2016-04-27T20:20:34Z
Interesting. Why does it need to create opAssign when post-blit is disabled? Is it documented anywhere? It sounds very confusing that disabling any symbol result in hidden injection of another one in main scope.
Comment #5 by bugzilla — 2016-04-28T10:42:53Z
(In reply to Dicebot from comment #4) > Interesting. Why does it need to create opAssign when post-blit is disabled? The @disable has no effect on the logic. An opAssign is always generated when there's a postblit, disabled or not, because opAssign is what the compiler generates a call to when assigning. The opAssign is constructed out of the postblit. > Is it documented anywhere? It sounds very confusing that disabling any > symbol result in hidden injection of another one in main scope. The logic for when to create an opAssign() is in the needOpAssign() function in clone.d. Unfortunately, changing the logic of this will break existing code, likely a lot of it, in maddening ways. But that isn't the real root of the problem. The real root isn't the @disable, it's the auto generation of various member functions and how they overload (or not) with other functions inside mixin templates. To overrule the auto generation, provide the symbol, which is the solution I illustrated.
Comment #6 by public — 2016-04-28T12:39:56Z
This is rather far from what I'd call "documented somewhere" :) Sorry if this semantics may seem intuitive to you, but they are in fact highly confusing, at least with existing error messages. I have spent ~2 hours to simply reduce the test case (it isn't very dustmite friendly) and wasn't even close to guessing underlying reason after that. Your proposed fix works fine for specific code I have but I can't consider it resolved with existing state of things in general. There is a dire need of documentation upgrade here. Some suggestions: 1) "An opAssign is always generated when there's a postblit, disabled or not, because opAssign is what the compiler generates a call to when assigning" This needs to be mentioned at https://dlang.org/spec/struct.html#struct-postblit (ideally, together with rationale). The fact that compiler auto-generated assignment when postblit is present is less surprising, but the fact it is still generated when one explicitly says to disable it is extremely confusing. What would be the body of generated opAssign when postblit is disabled? 2) "... function wat.S.opAssign (S p) is not callable using argument types (int[])" It would be quite helpful for such error messages to mention that relevant function is not user-defined: "... auto-generated function wat.S.opAssign (S p) is not callable using argument types (int[])" 3) "Mixed in templates are considered imports as far as looking things up goes." This is contradicts dlang.org documentation: https://dlang.org/spec/template-mixin.html "... It is analogous to cutting and pasting the body of the template into the location of the mixin." It looks like documentation doesn't match actual intention behind the mixin semantics and needs to be updated.
Comment #7 by bugzilla — 2016-04-28T17:42:57Z
Those are good suggestions.
Comment #8 by bugzilla — 2016-04-28T18:55:51Z
Comment #9 by bugzilla — 2016-04-28T20:56:17Z
Comment #10 by github-bugzilla — 2016-04-29T18:05:43Z
Commits pushed to master at https://github.com/dlang/dmd https://github.com/dlang/dmd/commit/4a340ec542b86a28275614fbb977a2bcad68e79a fix Issue 15957 - Disabled postblit + template mixin break opAssign with confusing error message https://github.com/dlang/dmd/commit/7829d3f3cdd42a2f7aecbf14bcdf265dabc56677 Merge pull request #5723 from WalterBright/fix15957 fix Issue 15957 - Disabled postblit + template mixin break opAssign w…
Comment #11 by github-bugzilla — 2016-10-01T11:46:41Z
Commits pushed to stable at https://github.com/dlang/dmd https://github.com/dlang/dmd/commit/4a340ec542b86a28275614fbb977a2bcad68e79a fix Issue 15957 - Disabled postblit + template mixin break opAssign with confusing error message https://github.com/dlang/dmd/commit/7829d3f3cdd42a2f7aecbf14bcdf265dabc56677 Merge pull request #5723 from WalterBright/fix15957
Comment #12 by github-bugzilla — 2017-08-02T08:07:08Z
Commit pushed to dmd-cxx at https://github.com/dlang/dmd https://github.com/dlang/dmd/commit/7e602e31da3eae7a46171b0b2fb38b701319e28d Issue 15957 - Disabled postblit + template mixin break opAssign with confusing error message