Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow aliasing templated functions from mixin template to add them to overload set #19504

Open
dlangBugzillaToGithub opened this issue Nov 5, 2018 · 5 comments

Comments

@dlangBugzillaToGithub
Copy link

Dennis (@dkorpel) reported this on 2018-11-05T11:28:33Z

Transferred from https://issues.dlang.org/show_bug.cgi?id=19365

CC List

  • Adam D. Ruppe
  • Bolpat

Description

I'm trying to mixin generic operator overloads for + and -, and then add specialized operators for * and /. Minimized code:

```
mixin template Operators() {
    int opBinary(string op: "+")(S rhs) {return 1;}
}

struct S {
    mixin Operators;
    int opBinary(string op: "*")(S rhs) {return 2;}
}

void main() {
    import std.stdio;
    S s;
    writeln(s * s); // 2
    writeln(s + s); // incompatible types for (s) + (s)
    writeln(s.opBinary!"+"(s)); //`opBinary!"+"` does not match template declaration opBinary(string op : "*")(S rhs)
}
```

While opBinary!"+" and opBinary!"*" work in isolation, the prescence of opBinary!"*" completely shadows the opBinary!"+" template in the mixin. I expect both the normal and mixed in opBinary templates to be operator overloading candidates.
@dlangBugzillaToGithub
Copy link
Author

destructionator commented on 2018-12-06T13:56:40Z

That's not a bug, that's by design. It allows you to override an overload set from a mixin template while keeping the other things.

To merge the sets, you use alias:

struct S {
    mixin Operators ops;
    int opBinary(string op: "*")(S rhs) {return 2;}
    alias opBinary = ops.opBinary;
}


However, while that works with every other case, with this one.... it is giving me

Error: alias `ooooo.S.opBinary` conflicts with template ooooo.S.opBinary(string op : "*")(S rhs) at ooooo.d(7)


So that *might* be a bug, it isn't merging specialized templates... I was going to close this immediately, but maybe this specific thing is legit.

To work around that, offer a dispatcher function instead of an alias:


struct S {
    mixin Operators ops;
    int opBinary(string op: "*")(S rhs) {return 2;}
    int opBinary(string op)(S rhs) {
        // forward all others to the mixin
        return ops.opBinary!op(rhs);
    }
}


see for design:
https://dlang.org/spec/template-mixin.html#mixin_scope

@dlangBugzillaToGithub
Copy link
Author

dkorpel commented on 2018-12-07T18:14:10Z

(In reply to Adam D. Ruppe from comment #1)
> So that *might* be a bug, it isn't merging specialized templates... I was
> going to close this immediately, but maybe this specific thing is legit.

Thanks for the workaround.

@dlangBugzillaToGithub
Copy link
Author

dkorpel commented on 2018-12-07T18:17:59Z

(In reply to Adam D. Ruppe from comment #1)
> So that *might* be a bug, it isn't merging specialized templates... I was
> going to close this immediately, but maybe this specific thing is legit.

I also noticed that it matters whether you put the alias before or after the new opBinary method. 

Anyway, I'll change this to an enhancement, thanks for the workaround. (I happily replace the ugly string mixin with a dispatcher function.)

@dlangBugzillaToGithub
Copy link
Author

qs.il.paperinik commented on 2020-12-17T20:56:37Z

(In reply to Adam D. Ruppe from comment #1)
> That's not a bug, that's by design. It allows you to override an overload
> set from a mixin template while keeping the other things.

This hasn't to do with overload sets. Overload sets work are separated overloads to search for matches where an ambiguity error occurs when there are any matches in two or more sets, regardless whether one match is clearly better.

The symbols inserted by mixin templates are always completely shadowed by eponymous symbols in the same scope. ALWAYS. The case Dennis describes is where the mixed-in symbol is the only one that could match. The error isn't an ambiguity error, so this is not primarily about overload sets, but symbol shadowing. This is intentional.

The enhancement therefore boils down to proposing against the current way of shadowing and for overloading sets. Putting the mixed-in symbols in a named scope like you're suggesting actually creates overloading sets in the first place.

There is a good reason for the shadowing, so this enhancement probably isn't going anywhere.

@dlangBugzillaToGithub
Copy link
Author

dkorpel commented on 2020-12-28T10:56:19Z

(In reply to Bolpat from comment #4)
> Overload sets work are separated overloads to search for matches where an
> ambiguity error occurs when there are any matches in two or more sets,
> regardless whether one match is clearly better.

I don't understand this sentence.

> Putting the mixed-in symbols in a named
> scope like you're suggesting actually creates overloading sets in the first
> place.
> 
> There is a good reason for the shadowing, so this enhancement probably isn't
> going anywhere.

The suggestion is not to change mixin template behavior, it is to allow using `alias` to explicitly combine mixin methods with existing member functions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant