Bug 19569 – overload resolution not right?

Status
RESOLVED
Resolution
FIXED
Severity
normal
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2019-01-09T23:33:09Z
Last change time
2019-03-13T09:16:31Z
Keywords
pull
Assigned to
No Owner
Creator
Manu

Comments

Comment #0 by turkeyman — 2019-01-09T23:33:09Z
So, I understand we can overload on `nothrow` For instance, this works: void foo(); void foo() nothrow; void test() { foo(); } void test() nothrow { foo(); } But if `foo` take a template argument, it breaks down: void foo(T)(); void foo(T)() nothrow; void test() { foo!int(); } void test() nothrow { foo!int(); } > Error: called with argument types `()` matches both: > `foo!int.foo()` > and: > `foo!int.foo()` I feel like this is a bug...?
Comment #1 by doob — 2019-01-10T06:51:47Z
Attributes for templates are inferred. "void foo(T)();" will be inferred to be "nothrow" if the arguments are "nothrow". Attributes are not inferred for regular functions.
Comment #2 by turkeyman — 2019-01-10T07:01:38Z
I'm not sure what you just said.
Comment #3 by turkeyman — 2019-01-10T07:04:20Z
I mean, if there's not a really good reason those 2 pieces of code don't work the same, then they should work the same...
Comment #4 by slavo5150 — 2019-01-10T07:58:47Z
I think what Jacob is saying is that `nothrow` will be inferred for templates at the time of instantiation based on the context of the instatiation. That means you should not create both `void foo(T)()` and `void foo(T)() nothrow`. When the template is instantiated the compiler will automatically add `nothrow` to the instantiation if the context is `nothrow`. The bug here may be that the compiler allowed you to declare both a `nothrow` and non-`nothrow` template without error. I suppose it should be an error to add any inferred attribute to templates if they're going to inferred by the caller. Either that, or, if the user explicitly added an attribute to a template, then inference for that attribute should be disabled at the time of instantiation. I can't say I approve of this attribute inference behavior -- I'd prefer to have some mataprogramming facilities to control it -- but that's the way D currently seems to work, so changing it would probably require a DIP.
Comment #5 by simen.kjaras — 2019-01-10T08:19:06Z
Still wrong. Consider: void foo()() { throw new Exception(""); } // Definitely not nothrow void foo()() nothrow { } // definitely is nothrow void test() { foo(); } void test() nothrow { foo(); } This gives the exact same error as before, demonstrating that this is indeed a problem exactly the way Manu describes it. However, if both variants of foo are indeed inferred to be nothrow, the criticisms presented by Jacob are correct. As for 'if the user explicitly added an attribute to a template, then inference for that attribute should be disabled at the time of instantiation.', consider this: void bar()() nothrow { throw new Exception(""); } unittest { bar(); } That gives the error message 'nothrow function bar!().bar may throw', so that already is disabled - if a templated function is marked nothrow, it has to abide by that promise.
Comment #6 by turkeyman — 2019-01-10T08:38:24Z
> so changing it would probably require a DIP. Look at my example; any reasonable observer would say those cases should behave the same. I think it's objectively a bug, that shouldn't need a DIP...
Comment #7 by simen.kjaras — 2019-01-10T08:43:38Z
> any reasonable observer would say those cases should behave the same. Here I have to disagree - both templates will be nothrow, so the compiler is correct to be confused. Look at my example above for a case where it's actually not ambiguous and still shows the same error message.
Comment #8 by turkeyman — 2019-01-10T08:55:16Z
> both templates will be nothrow, so the compiler is correct to be confused. How can they both be nothrow? One is nothrow, the other is not... they're extern, so they can't infer anything. The instantiated templates should have identical signatures to the functions above. > Look at my example above for a case where it's actually not ambiguous and still shows the same error message. Right... but that's a bug, right? The 2 functions definitely have different (and correct) signatures, so the overload resolution should work just like the simple case?
Comment #9 by simen.kjaras — 2019-01-10T09:13:36Z
> Right... but that's a bug, right? The 2 functions definitely have different (and correct) signatures, so the overload resolution should work just like the simple case? Absotively. I back you 100% in that this is a bug, and should absolutely not require a DIP in any form. > How can they both be nothrow? One is nothrow, the other is not... they're extern, so they can't infer anything. The instantiated templates should have identical signatures to the functions above. Ah, you're right - I forgot extern templates are a thing. For completion then, an example of how attributes are indeed not inferred for extern templates: void foo()(); void test() nothrow { foo(); // Error: function `foo!().foo` is not `nothrow` }
Comment #10 by doob — 2019-01-10T10:18:04Z
(In reply to Manu from comment #8) > How can they both be nothrow? One is nothrow, the other is not... they're > extern, so they can't infer anything. Oh, they don't have a body, I didn't think of that. Then I don't see how it could infer anything.
Comment #11 by r.sagitario — 2019-01-11T07:21:45Z
> So, I understand we can overload on `nothrow` I'm not aware that overloading on function attributes is possible. Do you have a reference in the specigfication?
Comment #12 by turkeyman — 2019-01-11T07:39:54Z
I don't have a reference... I've just been doing it for years and assumed it was intended functionality. I mean, it's certainly necessary that we overload on const/immutable/shared/etc... so I figured the other attributes come naturally too.
Comment #13 by r.sagitario — 2019-01-11T07:44:21Z
> that we overload on const/immutable/shared These are type modifiers for 'this', so they are different.
Comment #14 by turkeyman — 2019-01-11T07:49:57Z
Well either way, it seems to work and I've been using it for years.
Comment #15 by r.sagitario — 2019-01-11T07:58:19Z
Can you show an example? The one above does not: https://run.dlang.io/is/JCgWPp
Comment #16 by turkeyman — 2019-01-12T08:03:23Z
Oh wow... interesting. I think I have broken code floating around :/ So, is the issue here that it's possible to emit 2 functions that have ambiguous distinction, but no compile error? If a nothrow function can call either one, then it probably shouldn't just pick one, but rather emit an ambiguous call error instead?
Comment #17 by dlang-bot — 2019-03-03T04:38:13Z
@look-at-me created dlang/dmd pull request #9406 "[Reg 2.074.1] Fix Issue 19569 Fixed overload resolution giving priority to covariants when should be ambiguous." fixing this issue: - Fix Issue 19569 Fixed overload resolution giving priority to covariants when should be ambiguous. https://github.com/dlang/dmd/pull/9406
Comment #18 by razvan.nitu1305 — 2019-03-12T20:15:28Z
(In reply to Manu from comment #16) > Oh wow... interesting. > I think I have broken code floating around :/ > > So, is the issue here that it's possible to emit 2 functions that have > ambiguous distinction, but no compile error? > If a nothrow function can call either one, then it probably shouldn't just > pick one, but rather emit an ambiguous call error instead? But it doesn't pick one randomly, it picks the "better one" which is the one that is pure/@safe/nothrow/@nogc
Comment #19 by dlang-bot — 2019-03-13T09:16:31Z
dlang/dmd pull request #9406 "[Reg 2.074.1] Fix Issue 19569 Fixed overload resolution giving priority to covariants when should be ambiguous." was merged into master: - 3e29f1141b76aa087eab2dc917bd250174560abd by look-at-me: Fix Issue 19569 Fixed overload resolution giving priority to covariants when should be ambiguous. https://github.com/dlang/dmd/pull/9406