[clang] 56099d2 - [clang] Do not hide base member using-decls with different template head.
Utkarsh Saxena via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 27 02:52:55 PDT 2022
Author: Utkarsh Saxena
Date: 2022-10-27T11:52:46+02:00
New Revision: 56099d242809f80984e4afa37693177484aab13d
URL: https://github.com/llvm/llvm-project/commit/56099d242809f80984e4afa37693177484aab13d
DIFF: https://github.com/llvm/llvm-project/commit/56099d242809f80984e4afa37693177484aab13d.diff
LOG: [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 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](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
Added:
clang/test/SemaTemplate/concepts-using-decl.cpp
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaOverload.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index d7c4fbccfe90..ebed6186a1f0 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 <https://wg21.link/P2113R0>`_
([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 <https://github.com/llvm/llvm-project/issues/50886>`_.
+
C++2b Feature Support
^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 37ad78f3d796..9bf1ba7e8cc7 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3674,9 +3674,9 @@ class Sema final {
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 4171b97cf707..8d044c32bb21 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 000000000000..fca69dea5c88
--- /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<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_
diff erent_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_
diff erent_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.
More information about the cfe-commits
mailing list