From 56099d242809f80984e4afa37693177484aab13d Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena Date: Thu, 27 Oct 2022 11:50:43 +0200 Subject: [PATCH] [clang] Do not hide base member using-decls with different template head. 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](https://eel.is/c++draft/temp.over.link#6) and hence eligible for overload resolution, `Derived::foo` still [hides any previous using decl](https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaOverload.cpp#L1283-L1301,). Clang diverges from gcc here and can be seen more clearly in this example: ``` struct base { template int foo() { return 1; }; }; struct bar : public base { using base::foo; template 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](https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaOverload.cpp#L1148-L1152) because of [mismatching requires clause.](https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaOverload.cpp#L1390-L1405). 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. Reviewed By: ilya-biryukov, #clang-language-wg Differential Revision: https://reviews.llvm.org/D136440 --- clang/docs/ReleaseNotes.rst | 3 + clang/include/clang/Sema/Sema.h | 6 +- clang/lib/Sema/SemaOverload.cpp | 56 +++++--- clang/test/SemaTemplate/concepts-using-decl.cpp | 178 ++++++++++++++++++++++++ 4 files changed, 220 insertions(+), 23 deletions(-) create mode 100644 clang/test/SemaTemplate/concepts-using-decl.cpp diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index d7c4fbc..ebed618 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -560,6 +560,9 @@ C++20 Feature Support - Implemented `P2113R0: Proposed resolution for 2019 comment CA 112 `_ ([temp.func.order]p6.2.1 is not implemented, matching GCC). +- Do not hide templated base members introduced via using-decl in derived class + (useful specially for constrained members). Fixes `GH50886 `_. + C++2b Feature Support ^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 37ad78f..9bf1ba7 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -3674,9 +3674,9 @@ public: 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); // Calculates whether the expression Constraint depends on an enclosing diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index 4171b97..8d044c3 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -1280,26 +1280,42 @@ bool Sema::IsOverload(FunctionDecl *New, FunctionDecl *Old, !FunctionParamTypesAreEqual(OldType, NewType))) return true; - // 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()))) - 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. + 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. // diff --git a/clang/test/SemaTemplate/concepts-using-decl.cpp b/clang/test/SemaTemplate/concepts-using-decl.cpp new file mode 100644 index 0000000..fca69de --- /dev/null +++ b/clang/test/SemaTemplate/concepts-using-decl.cpp @@ -0,0 +1,178 @@ +// RUN: %clang_cc1 -std=c++20 -verify %s + +namespace static_methods { +template 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 struct Opaque {}; +template void expect(Opaque _) {} + +struct Empty{}; +constexpr int EmptySize = sizeof(Empty); + +template concept IsEmpty = sizeof(T) == EmptySize; + +namespace base_members_not_hidden { +struct base { + template + Opaque<0> foo() { return Opaque<0>(); }; +}; + +struct bar1 : public base { + using base::foo; + template requires IsEmpty + Opaque<1> foo() { return Opaque<1>(); }; +}; + +struct bar2 : public base { + using base::foo; + template + Opaque<1> foo() { return Opaque<1>(); }; +}; + +struct bar3 : public base { + using base::foo; + template + Opaque<1> foo() requires IsEmpty { return Opaque<1>(); }; +}; + +void func() { + expect<0>(base{}.foo()); + expect<0>(base{}.foo()); + expect<1>(bar1{}.foo()); + expect<0>(bar1{}.foo()); + expect<1>(bar2{}.foo()); + expect<0>(bar2{}.foo()); + expect<1>(bar3{}.foo()); + expect<0>(bar3{}.foo()); +} +} +namespace base_members_hidden { +struct base1 { + template requires IsEmpty + Opaque<0> foo() { return Opaque<0>(); }; // expected-note {{candidate function}} +}; +struct bar1 : public base1 { + using base1::foo; + template requires IsEmpty + Opaque<1> foo() { return Opaque<1>(); }; +}; +struct base2 { + template + Opaque<0> foo() { return Opaque<0>(); }; +}; +struct bar2 : public base2 { + using base2::foo; + template + Opaque<1> foo() { return Opaque<1>(); }; +}; +struct baz : public base1 { + using base1::foo; + template requires IsEmpty && IsEmpty + Opaque<1> foo() { return Opaque<1>(); }; // expected-note {{candidate function}} +}; +void func() { + expect<0>(base1{}.foo()); + expect<1>(bar1{}.foo()); + expect<0>(base2{}.foo()); + expect<1>(bar2{}.foo()); + baz{}.foo(); // expected-error {{call to member function 'foo' is ambiguous}} +} +} // namespace base_members_hidden + +namespace same_contraint_at_different_place { +struct base { + template + void foo1() {}; // expected-note 2 {{candidate function}} + template requires IsEmpty + void foo2() {}; // expected-note 2 {{candidate function}} + template + void foo3() requires IsEmpty {}; // expected-note 2 {{candidate function}} +}; +struct bar1 : public base { + using base::foo1; + using base::foo2; + using base::foo3; + template requires IsEmpty + void foo1() {}; // expected-note {{candidate function}} + template + void foo2() {}; // expected-note {{candidate function}} + template + void foo3() {}; // expected-note {{candidate function}} +}; +struct bar2 : public base { + using base::foo1; + using base::foo2; + using base::foo3; + template + void foo1() requires IsEmpty {}; // expected-note {{candidate function}} + template + void foo2() requires IsEmpty {}; // expected-note {{candidate function}} + template requires IsEmpty + void foo3() {}; // expected-note {{candidate function}} +}; +void func() { + bar1{}.foo1(); // expected-error {{call to member function 'foo1' is ambiguous}} + bar1{}.foo2(); // expected-error {{call to member function 'foo2' is ambiguous}} + bar1{}.foo3(); // expected-error {{call to member function 'foo3' is ambiguous}} + bar2{}.foo1(); // expected-error {{call to member function 'foo1' is ambiguous}} + bar2{}.foo2(); // expected-error {{call to member function 'foo2' is ambiguous}} + bar2{}.foo3(); // expected-error {{call to member function 'foo3' is ambiguous}} +} +} // namespace same_constraint_at_different_place + +namespace more_constrained { +struct base1 { + template Opaque<0> foo() { return Opaque<0>(); } +}; +struct derived1 : base1 { + using base1::foo; + template Opaque<1> foo() { return Opaque<1>(); } +}; +struct base2 { + template Opaque<0> foo() { return Opaque<0>(); } +}; +struct derived2 : base2 { + using base2::foo; + template Opaque<1> foo() { return Opaque<1>(); } +}; +void func() { + expect<0>(derived1{}.foo()); + expect<1>(derived1{}.foo()); + expect<0>(derived2{}.foo()); + expect<1>(derived2{}.foo()); +} +} // namespace more_constrained +} // namespace constrained_members + +namespace heads_without_concepts { +struct base { + template + int foo() { return 1; }; +}; + +struct bar : public base { + using base::foo; + template + 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. -- 2.7.4