llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.13k stars 12.02k forks source link

member using declaration of function templates with different return types does not bring base class overload into derived class #49583

Open 54aefcd4-c07d-4252-8441-723563c8826f opened 3 years ago

54aefcd4-c07d-4252-8441-723563c8826f commented 3 years ago
Bugzilla Link 50239
Version trunk
OS Linux
CC @DougGregor,@zygoloid

Extended Description

Clang incorrectly accepts the following example (https://clang.godbolt.org/z/7abPjfcf6):

template <class T> struct A {
  template <class U> long f(U) const { return 1; }
};
struct B : A<int> {
  using A<int>::f;
  template <class U> int f(U) const { return 2; }
};
int main() {
  return B{}.f(3);
}

Instead, it should error at B{}.f(3), since the call is ambiguous.

A member using declaration brings into the derived class all the overloads of the specified function from the given base class, except for any overloads where the derived class has a function declaration with the same signature ([namespace.udecl]/p11).

For non-template functions, the return type is not part of the signature ([defns.signature.member]).

For function templates, the return type is part of the signature ([defns.signature.member.templ]).

In this example, A<int>::f and B::f have different return types (long vs int), and have different signatures. B::f does not hide A<int>::f, and both B::f and A<int>::f are available for overload resolution.

Because they have the same parameters types, any call is ambiguous.

llvmbot commented 7 months ago

@llvm/issue-subscribers-clang-frontend

Author: None (54aefcd4-c07d-4252-8441-723563c8826f)

| | | | --- | --- | | Bugzilla Link | [50239](https://llvm.org/bz50239) | | Version | trunk | | OS | Linux | | CC | @DougGregor,@zygoloid | ## Extended Description Clang incorrectly accepts the following example (https://clang.godbolt.org/z/7abPjfcf6): ```cpp template <class T> struct A { template <class U> long f(U) const { return 1; } }; struct B : A<int> { using A<int>::f; template <class U> int f(U) const { return 2; } }; int main() { return B{}.f(3); } ``` Instead, it should error at `B{}.f(3)`, since the call is ambiguous. A member using declaration brings into the derived class all the overloads of the specified function from the given base class, except for any overloads where the derived class has a function declaration with the same signature ([namespace.udecl]/p11). For non-template functions, the return type is not part of the signature ([defns.signature.member]). For function templates, the return type is part of the signature ([defns.signature.member.templ]). In this example, `A<int>::f` and `B::f` have different return types (`long` vs `int`), and have different signatures. `B::f` does not hide `A<int>::f`, and both `B::f` and `A<int>::f` are available for overload resolution. Because they have the same parameters types, any call is ambiguous.
shafik commented 7 months ago

It is not obvious to me what the rules are here but implementations seem to all agree: https://clang.godbolt.org/z/ooxK1xoeq

CC @zygoloid

Endilll commented 7 months ago

I think this is covered by http://eel.is/c++draft/namespace.udecl#11:

The set of declarations named by a using-declarator that inhabits a class C does not include member functions and member function templates of a base class that correspond to (and thus would conflict with) a declaration of a function or function template in C.

So using-declaration in the example doesn't circumvent the hiding done by derived class, so it's well formed in the same way that it would be without using-declaration.

@shafik feel free to close the bug if you agree with my reading of the standard.

cor3ntin commented 7 months ago

I agree with that reading @Endilll. Closing

zygoloid commented 7 months ago

These are function templates with different return types, so they don't correspond. This was changed in P1787. We have work to do.

shafik commented 7 months ago

These are function templates with different return types, so they don't correspond. This was changed in P1787. We have work to do.

Note in the godbolt link I added I made their return types the same: https://clang.godbolt.org/z/6MonvPddE

Could you point out which part of P1787 applies here.

zygoloid commented 7 months ago

The relevant change is in [namespace.udecl]/14, which used to say:

When a using-declarator brings declarations from a base class into a derived class, member functions and member function templates in the derived class override and/or hide member functions and member function templates with the same name, parameter-type-list ([dcl.fct]), trailing requires-clause (if any), cv-qualification, and ref-qualifier (if any), in a base class (rather than conflicting). Such hidden or overridden declarations are excluded from the set of declarations introduced by the using-declarator.

and after P1787 says:

The set of declarations named by a using-declarator that inhabits a class C does not include member functions and member function templates of a base class that correspond to (and thus would conflict with) a declaration of a function or function template in C.

Note that the old wording doesn't consider the return type or the template-head for a function template, and the new wording does.

Clang is getting this wrong, having not implemented the change in question. You can see that in an example such as:

template <class T> struct A {
  template <class U> 
  decltype(U()) f(U) const { return 1; }
};
struct B : A<int> {
  using A<int>::f;
  template <class U>
  decltype(U(1, 2)) f(U) const { return 2; }
};
int main() {
    B().f(0);
}

... which GCC accepts but Clang rejects: A::f is not in the overload set for Clang, but is for GCC.

I think the reason we're getting the right answer for the original example and for the example with matching return types is because the conversion of the object parameter is better for the derived-class function, but the original report is correct: we should have both functions visible to name lookup inside B, and we incorrectly do not.

Here's another variant of the original example we're getting wrong:

template <class T> struct A {
  template <class U> long f(U) const { return 1; }
};
struct B : A<int> {
  using A<int>::f;
  template <class U> int f(U) const { return 2; }
};
auto p = &B::f<int>;

GCC correctly reports an ambiguity here, Clang does not.

zygoloid commented 7 months ago

Interesting, if we take your example with the same return types and tweak it so the base-class version is not hidden even by the old rule, then Clang does report an ambiguity, and GCC still does not. In particular:

I think the reason we're getting the right answer for the original example and for the example with matching return types is because the conversion of the object parameter is better for the derived-class function, but the original report is correct: we should have both functions visible to name lookup inside B, and we incorrectly do not.

I think that's not actually right: we're supposed to consider the type of the object parameter to be the derived-class type in this case, see [over.match.funcs.general]/4:

For non-conversion functions that are implicit object member functions nominated by a using-declaration in a derived class, the function is considered to be a member of the derived class for the purpose of defining the type of the implicit object parameter.

So I think the original report is simply correct: the example is ambiguous, and every compiler gets it wrong. Clang, EDG, and MSVC are getting it wrong because they don't implement this part of P1787 yet, it seems. GCC is getting it wrong despite implementing this part of P1787; my guess is they have a non-standard tie-breaker of some kind, though I'm not sure why. Looks like this changed in GCC 7, and they reported an ambiguity like everyone else before then.

zygoloid commented 7 months ago

Looks like this changed in GCC 7, and they reported an ambiguity like everyone else before then.

I wonder... GCC 7 added support for CTAD and the revamped inheriting constructor rules, meaning these two tie-breaks would have been added:

  • [over.match.best]/2.7: F1 is a constructor for a class D, F2 is a constructor for a base class B of D, and for all arguments the corresponding parameters of F1 and F2 have the same type
  • [over.match.best]/2.10: F1 and F2 are generated from class template argument deduction ([over.match.class.deduct]) for a class D, and F2 is generated from inheriting constructors from a base class of D while F1 is not, and for each explicit function argument, the corresponding parameters of F1 and F2 are either both ellipses or have the same type, or, if not that,

Neither of these apply here, but they both come close and would prefer the derived-class function over the base-class function in the cases we're looking at if they were applied too broadly. (Eg, if you ignore the "constructor" restriction.)

That's my best guess: GCC is applying this rule to non-constructor cases too. That's probably reasonable if implementing this rule leads to any regressions.

Endilll commented 7 months ago

These are function templates with different return types, so they don't correspond.

I'm not sure about this. Different return types disqualify those function templates as corresponding overloads, because they don't satisfy http://eel.is/c++draft/basic.scope.scope#4.5. This also makes them correspond per http://eel.is/c++draft/basic.scope.scope#4.3.