[clang] Do not hide base member using-decls with different template head.

[usaxena95]

Summary

Fixes: https://github.com/llvm/llvm-project/issues/50886

Adding requires clause to template head or constraining the template parameter type is ineffective because, even though it creates a non-equivalent template head temp.over.link#6 and hence eligible for overload resolution, Derived::foo still hides any previous using decl.
Clang diverges from gcc here and can be seen more clearly in this example:

struct base {
  template <int N, int M>
  int foo() { return 1; };
};

struct bar : public base {
  using base::foo;
  template <int N> 
  int foo() { return 2; };
};

int main() {
  bar f;
  f.foo<10, 10>(); // clang previously errored while GCC does not.
}

https://godbolt.org/z/v5hnh6czq. We see that bar::foo hides base::foo because it only differs in the head.

Adding a trailing requires to the definition was a nice find. In this case, clang considers them overloads because of mismatching requires clause.. So both of them make it to the overload resolution (where constrained Derived::foo is rejected then).

---

In this patch, we do not ignore matching the template head (template parameters, type contraints and trailing requires) while considering whether the using decl of base member should be hidden. The return type of a templated function is still not considered as different return types would create ambiguous candidates.

The changed tests looks reasonable and also matches GCC behaviour: https://godbolt.org/z/8KqPEThrY

Note: We are now able to create an ambiguity in case where both base member and derived member specialisations satisfy the constraints (when the constraints are not same). Ideally using-decl should not create ambiguity. I plan to fix this later if it gathers more attention.

[usaxena95]

Added more tests.

[ilya-biryukov]

Thanks for the change. All the behavior changes look reasonable to me, but I'm struggling to understand whether we follow the standard closely here.
Left a comment to discuss this in depth.

clang/lib/Sema/SemaOverload.cpp

1142

UseMemberUsingDeclRules is used everywhere in the code and in the comments for calls to IsOverload .

Let's maybe change the name in the header instead to avoid chasing all call sites?

1202

Lambda is only used once. Could you inline it to make the code simpler?

bool IsConstructorOrAssignment = false;
if (auto *CMD = dyn_cast<CXXMethodDecl>(FD); CMD && (...)) {
  IsConsutrctorOrAssignment = true;
}

1211

How is this related to checking whether the template parameter lists check?
I seems to be missing the logical connection that goes from the standard wording to the code. Could you explain?

An example that I have in mind is:

struct A {
    template <class T, class U = int> A(T);
    template <class T, class U = int> int foo(T);
};
struct B : A {
    using A::A;
    template <class T> B(T);

    using A::foo;
    template <class T> int foo(T);
};

namespace detail {
    template <class T> int bar(T);
}
using detail::bar;
template <class T, class U = int> int bar(T);

int a = bar(10); // this is definitely ambiguous.

B b(10); // should this be ambiguous?
int c = b.foo(10); // should this be ambiguous?
// Should constructors and functions behave differently? Why?

I see that both Clang and GCC treat member declarations differently, but I don't see why they should given that they are supposed to use the same "corresponds" terminology from the standard. What am I missing?

clang/test/SemaTemplate/concepts-using-decl.cpp

111

Why should they be dropped? Could you provide pointers from the standard?

[usaxena95]

I have switched to a version where we matched template heads only when they are constraints. This is for the moment to unblock the existing bugs and not break existing code prior to C++20.
Let us continue the discussion over the possibility of introducing an ambiguity in https://github.com/llvm/llvm-project/issues/58571

clang/lib/Sema/SemaOverload.cpp

1211

I have filed https://github.com/llvm/llvm-project/issues/58571 to discuss the interpretation of the standard.

clang/test/SemaTemplate/concepts-using-decl.cpp

111

I think I misunderstood that using decls should not be introducing ambiguities.

[usaxena95]

Addressed comments.

[ilya-biryukov]

My only ask is to add a few more tests (see the relevant comment), otherwise LG.

clang/lib/Sema/SemaOverload.cpp

1197

Hopefully this does not regress performance too much now that we keep matching template even if we don't need the result.
No need to take any action (the code looks simpler this way), just something to keep in mind in case it backfires.

clang/test/SemaTemplate/concepts-using-decl.cpp

4

NIT: maybe move this to the end of the file?
I would expect concept-specific tests to go first given the name of the file.
It's still useful to capture the discovered corner-cases, obviously, just not as the first thing, maybe.

113

Could you also check that requires clauses and constraints in template parameters do not hide each other?

template <IsEmpty T> ...
// vs
template <class T> requires IsEmpty<T> ...
// vs
template <class T> void foo() requires IsEmpty<T> ...

Should not hide each other.

Another interesting case (that probably does not yet work):

struct base { template <class T> void foo(); };
struct derived : base { 
  using base::foo; 
  template <IsEmpty T> void foo();
};

^^ derived().foo<Empty>() will probably produce an ambiguity now (as we don't have an explicit requires clause here). I don't think it's worth fixing now, but keeping a test for it with a FIXME seems reasonable.

[usaxena95]

Added more tests.

clang/test/SemaTemplate/concepts-using-decl.cpp

113

The last test case would work though since we perform template head check if any template head is contrained. This is not ambiguous as overloading chooses the "most contrained" version.

[ilya-biryukov]

clang/test/SemaTemplate/concepts-using-decl.cpp

113

Ah, right, this case is not ambiguous to start with. I think I made a mistake in the description.
The call with Empty would work either way and does not check anything:

• we only see a constrained overload => it works,
• we see both overloads, but the one with IsEmpty is more specialized => it works.

I suggest also checking that derived().foo<int>() works. In that case the outcomes are different:

• we only see a constrained overload => compiler error,
• we see both overloads => constrained version does not match and the overload succeeds.

PS I was definitely under the impression that we are checking for the requires clause, but not constrained template parameters. However, I now see that I was wrong and we should also be fine with that case.

179

NIT: add newline?

[usaxena95]

Addressed comments. Added release notes.

clang/docs/ReleaseNotes.rst

 
 - Skip rebuilding lambda expressions in arguments of immediate invocations.
   This fixes `GH56183 <https://github.com/llvm/llvm-project/issues/56183>`_,
   `GH51695 <https://github.com/llvm/llvm-project/issues/51695>`_,
   `GH50455 <https://github.com/llvm/llvm-project/issues/50455>`_,
   `GH54872 <https://github.com/llvm/llvm-project/issues/54872>`_,
   `GH54587 <https://github.com/llvm/llvm-project/issues/54587>`_.
 
+- Do not hide templated base members introduced via using-decl in derived class
+  (useful specially for constrained members). Fixes `GH50886 <https://github.com/llvm/llvm-project/issues/50886>`_.
+
 C++2b Feature Support
 ^^^^^^^^^^^^^^^^^^^^^
 
 CUDA/HIP Language Changes in Clang
 ----------------------------------
 
 Objective-C Language Changes in Clang
 -------------------------------------

clang/include/clang/Sema/Sema.h

enum OverloadKind {
     /// This is not an overload because the lookup results contain a
     /// non-function.
     Ovl_NonFunction
   };
   OverloadKind CheckOverload(Scope *S,
                              FunctionDecl *New,
                              const LookupResult &OldDecls,
                              NamedDecl *&OldDecl,
-                             bool IsForUsingDecl);
-  bool IsOverload(FunctionDecl *New, FunctionDecl *Old, bool IsForUsingDecl,
-                  bool ConsiderCudaAttrs = true,
+                             bool UseMemberUsingDeclRules);
+  bool IsOverload(FunctionDecl *New, FunctionDecl *Old,
+                  bool UseMemberUsingDeclRules, bool ConsiderCudaAttrs = true,
                   bool ConsiderRequiresClauses = true);
 
   enum class AllowedExplicit {
     /// Allow no explicit functions to be used.
     None,
     /// Allow explicit conversion functions but not explicit constructors.
     Conversions,
     /// Allow both explicit conversion functions and explicit constructors.

clang/lib/Sema/SemaOverload.cpp

if (New->getFriendObjectKind() && New->getQualifier() &&
     Match = TemplateSpecResult.getAsSingle<FunctionDecl>();
     return Ovl_Match;
   }
 
   return Ovl_Overload;
 }
 
 bool Sema::IsOverload(FunctionDecl *New, FunctionDecl *Old,
                       bool UseMemberUsingDeclRules, bool ConsiderCudaAttrs,
                       bool ConsiderRequiresClauses) {
   // C++ [basic.start.main]p2: This function shall not be overloaded.
   if (New->isMain())
     return false;
 
   // MSVCRT user defined entry points cannot be overloaded.
   if (New->isMSVCRTEntryPoint())
     return false;
bool Sema::IsOverload(FunctionDecl *New, FunctionDecl *Old,
   // parameters (C++ 1.3.10), which includes the presence or absence
   // of the ellipsis; see C++ DR 357).
   if (OldQType != NewQType &&
       (OldType->getNumParams() != NewType->getNumParams() ||
        OldType->isVariadic() != NewType->isVariadic() ||
        !FunctionParamTypesAreEqual(OldType, NewType)))
     return true;
 
+  if (NewTemplate) {
     // C++ [temp.over.link]p4:
     //   The signature of a function template consists of its function
     //   signature, its return type and its template parameter list. The names
     //   of the template parameters are significant only for establishing the
     //   relationship between the template parameters and the rest of the
     //   signature.
     //
     // We check the return type and template parameter lists for function
     // templates first; the remaining checks follow.
-  //
-  // However, we don't consider either of these when deciding whether
-  // a member introduced by a shadow declaration is hidden.
-  if (!UseMemberUsingDeclRules && NewTemplate &&
-      (!TemplateParameterListsAreEqual(NewTemplate->getTemplateParameters(),
-                                       OldTemplate->getTemplateParameters(),
-                                       false, TPL_TemplateMatch) ||
-       !Context.hasSameType(Old->getDeclaredReturnType(),
-                            New->getDeclaredReturnType())))
+    bool SameTemplateParameterList = TemplateParameterListsAreEqual(
+        NewTemplate->getTemplateParameters(),
+        OldTemplate->getTemplateParameters(), false, TPL_TemplateMatch);
+    bool SameReturnType = Context.hasSameType(Old->getDeclaredReturnType(),
+                                              New->getDeclaredReturnType());
+    // FIXME(GH58571): Match template parameter list even for non-constrained
+    // template heads. This currently ensures that the code prior to C++20 is
+    // not newly broken.
+    bool ConstraintsInTemplateHead =
+        NewTemplate->getTemplateParameters()->hasAssociatedConstraints() ||
+        OldTemplate->getTemplateParameters()->hasAssociatedConstraints();
+    // C++ [namespace.udecl]p11:
+    //   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.
+    // Comparing return types is not required for the "correspond" check to
+    // decide whether a member introduced by a shadow declaration is hidden.
+    if (UseMemberUsingDeclRules && ConstraintsInTemplateHead &&
+        !SameTemplateParameterList)
       return true;
-
+    if (!UseMemberUsingDeclRules &&
+        (!SameTemplateParameterList || !SameReturnType))
+      return true;
+  }
   // If the function is a class member, its signature includes the
   // cv-qualifiers (if any) and ref-qualifier (if any) on the function itself.
   //
   // As part of this, also check whether one of the member functions
   // is static, in which case they are not overloads (C++
   // 13.1p2). While not part of the definition of the signature,
   // this check is important to determine whether these functions
   // can be overloaded.

clang/test/SemaTemplate/concepts-using-decl.cpp

+// RUN: %clang_cc1 -std=c++20 -verify %s
+
+namespace static_methods {
+template<class> concept False = false;
+
+struct Base {
+    static void foo(auto);
+};
+struct Derived : public Base {
+    using Base::foo;
+    static void foo(False auto);
+};
+void func() {
+    Derived::foo(42);
+}
+} // namespace static_methods
+
+namespace constrained_members {
+template <unsigned n> struct Opaque {};
+template <unsigned n> void expect(Opaque<n> _) {}
+
+struct Empty{};
+constexpr int EmptySize = sizeof(Empty);
+
+template<typename T> concept IsEmpty = sizeof(T) == EmptySize;
+
+namespace base_members_not_hidden {
+struct base {
+  template <typename T>
+  Opaque<0> foo() { return Opaque<0>(); };
+};
+
+struct bar1 : public base {
+  using base::foo;
+  template <typename T> requires IsEmpty<T> 
+  Opaque<1> foo() { return Opaque<1>(); };
+};
+
+struct bar2 : public base {
+  using base::foo;
+  template <IsEmpty T>
+  Opaque<1> foo() { return Opaque<1>(); };
+};
+
+struct bar3 : public base {
+  using base::foo;
+  template <typename T>
+  Opaque<1> foo() requires IsEmpty<T> { return Opaque<1>(); };
+};
+
+void func() {
+  expect<0>(base{}.foo<Empty>());
+  expect<0>(base{}.foo<int>());
+  expect<1>(bar1{}.foo<Empty>());
+  expect<0>(bar1{}.foo<int>());
+  expect<1>(bar2{}.foo<Empty>());
+  expect<0>(bar2{}.foo<int>());
+  expect<1>(bar3{}.foo<Empty>());
+  expect<0>(bar3{}.foo<int>());
+}
+}
+namespace base_members_hidden {
+struct base1 {
+  template <typename T> requires IsEmpty<T>
+  Opaque<0> foo() { return Opaque<0>(); }; // expected-note {{candidate function}}
+};
+struct bar1 : public base1 {
+  using base1::foo;
+  template <typename T> requires IsEmpty<T>
+  Opaque<1> foo() { return Opaque<1>(); };
+};
+struct base2 {
+  template <IsEmpty T>
+  Opaque<0> foo() { return Opaque<0>(); };
+};
+struct bar2 : public base2 {
+  using base2::foo;
+  template <IsEmpty T>
+  Opaque<1> foo() { return Opaque<1>(); };
+};
+struct baz : public base1 {
+  using base1::foo;
+  template <typename T> requires IsEmpty<T> && IsEmpty<T>
+  Opaque<1> foo() { return Opaque<1>(); };  // expected-note {{candidate function}}
+};
+void func() { 
+  expect<0>(base1{}.foo<Empty>());
+  expect<1>(bar1{}.foo<Empty>());
+  expect<0>(base2{}.foo<Empty>());
+  expect<1>(bar2{}.foo<Empty>());
+  baz{}.foo<Empty>(); // expected-error {{call to member function 'foo' is ambiguous}}
+}
+} // namespace base_members_hidden
+
+namespace same_contraint_at_different_place {
+struct base {
+  template <IsEmpty T>
+  void foo1() {}; // expected-note 2 {{candidate function}}
+  template <typename T> requires IsEmpty<T>
+  void foo2() {}; // expected-note 2 {{candidate function}}
+  template <typename T>
+  void foo3() requires IsEmpty<T> {}; // expected-note 2 {{candidate function}}
+};
+struct bar1 : public base {
+  using base::foo1;
+  using base::foo2;
+  using base::foo3;
+  template <typename T> requires IsEmpty<T>
+  void foo1() {}; // expected-note {{candidate function}}
+  template <IsEmpty T>
+  void foo2() {}; // expected-note {{candidate function}}
+  template <IsEmpty T>
+  void foo3() {}; // expected-note {{candidate function}}
+};
+struct bar2 : public base {
+  using base::foo1;
+  using base::foo2;
+  using base::foo3;
+  template <typename T>
+  void foo1() requires IsEmpty<T> {}; // expected-note {{candidate function}}
+  template <typename T>
+  void foo2() requires IsEmpty<T> {}; // expected-note {{candidate function}}
+  template <typename T> requires IsEmpty<T>
+  void foo3() {}; // expected-note {{candidate function}}
+};
+void func() {
+  bar1{}.foo1<Empty>(); // expected-error {{call to member function 'foo1' is ambiguous}}
+  bar1{}.foo2<Empty>(); // expected-error {{call to member function 'foo2' is ambiguous}}
+  bar1{}.foo3<Empty>(); // expected-error {{call to member function 'foo3' is ambiguous}}
+  bar2{}.foo1<Empty>(); // expected-error {{call to member function 'foo1' is ambiguous}}
+  bar2{}.foo2<Empty>(); // expected-error {{call to member function 'foo2' is ambiguous}}
+  bar2{}.foo3<Empty>(); // expected-error {{call to member function 'foo3' is ambiguous}}
+}
+} // namespace same_constraint_at_different_place
+
+namespace more_constrained {
+struct base1 { 
+  template <class T> Opaque<0> foo() { return Opaque<0>(); }
+};
+struct derived1 : base1 { 
+  using base1::foo;
+  template <IsEmpty T> Opaque<1> foo() { return Opaque<1>(); }
+};
+struct base2 { 
+  template <IsEmpty T> Opaque<0> foo() { return Opaque<0>(); }
+};
+struct derived2 : base2 { 
+  using base2::foo;
+  template <class T> Opaque<1> foo() { return Opaque<1>(); }
+};
+void func() {
+  expect<0>(derived1{}.foo<int>());
+  expect<1>(derived1{}.foo<Empty>());
+  expect<0>(derived2{}.foo<Empty>());
+  expect<1>(derived2{}.foo<int>());
+}
+} // namespace more_constrained
+} // namespace constrained_members
+
+namespace heads_without_concepts {
+struct base {
+  template <int N, int M>
+  int foo() { return 1; };
+};
+
+struct bar : public base {
+  using base::foo;
+  template <int N> 
+  int foo() { return 2; }; // expected-note {{candidate template ignored: substitution failure: too many template arguments for function template 'foo'}}
+};
+
+void func() {
+  bar f;
+  f.foo<10>();
+  // FIXME(GH58571): bar::foo should not hide base::foo.
+  f.foo<10, 10>(); // expected-error {{no matching member function for call to 'foo'}}
+}
+} // namespace heads_without_concepts.