[clang] Correctly(?) handle placeholder types in ExprRequirements.

Bug #52905 was originally papered over in a different way, but
I believe this is the actually proper fix, or at least closer to
it. We need to detect placeholder types as close to the front-end
as possible, and cause them to fail constraints, rather than letting
them persist into later stages.

Fixes #52905.
Fixes #52909.
Fixes #53075.

Differential Revision: https://reviews.llvm.org/D118552

diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 139ea5e..5fa2d46 100644 (file)
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -3370,8 +3370,9 @@ QualType ASTContext::getBlockPointerType(QualType T) const {
 /// lvalue reference to the specified type.
 QualType
 ASTContext::getLValueReferenceType(QualType T, bool SpelledAsLValue) const {
-  assert(getCanonicalType(T) != OverloadTy &&
-         "Unresolved overloaded function type");
+  assert((!T->isPlaceholderType() ||
+          T->isSpecificPlaceholderType(BuiltinType::UnknownAny)) &&
+         "Unresolved placeholder type");
 
   // Unique pointers, to guarantee there is only one pointer of a particular
   // structure.
@@ -3409,6 +3410,10 @@ ASTContext::getLValueReferenceType(QualType T, bool SpelledAsLValue) const {
 /// getRValueReferenceType - Return the uniqued reference to the type for an
 /// rvalue reference to the specified type.
 QualType ASTContext::getRValueReferenceType(QualType T) const {
+  assert((!T->isPlaceholderType() ||
+          T->isSpecificPlaceholderType(BuiltinType::UnknownAny)) &&
+         "Unresolved placeholder type");
+
   // Unique pointers, to guarantee there is only one pointer of a particular
   // structure.
   llvm::FoldingSetNodeID ID;

diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 483247a..3fa192c 100644 (file)
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -14320,7 +14320,8 @@ ExprResult Sema::BuildCallToMemberFunction(Scope *S, Expr *MemExprE,
     FoundDecl = MemExpr->getFoundDecl();
     Qualifier = MemExpr->getQualifier();
     UnbridgedCasts.restore();
-  } else if (auto *UnresExpr = dyn_cast<UnresolvedMemberExpr>(NakedMemExpr)) {
+  } else {
+    UnresolvedMemberExpr *UnresExpr = cast<UnresolvedMemberExpr>(NakedMemExpr);
     Qualifier = UnresExpr->getQualifier();
 
     QualType ObjectType = UnresExpr->getBaseType();
@@ -14433,9 +14434,7 @@ ExprResult Sema::BuildCallToMemberFunction(Scope *S, Expr *MemExprE,
     }
 
     MemExpr = cast<MemberExpr>(MemExprE->IgnoreParens());
-  } else
-    // Unimaged NakedMemExpr type.
-    return ExprError();
+  }
 
   QualType ResultType = Method->getReturnType();
   ExprValueKind VK = Expr::getValueKindForType(ResultType);

diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 7c6bb4c..6de486b 100644 (file)
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1943,6 +1943,9 @@ TemplateInstantiator::TransformExprRequirement(concepts::ExprRequirement *Req) {
     if (ExprInst.isInvalid())
       return nullptr;
     ExprResult TransExprRes = TransformExpr(E);
+    if (!TransExprRes.isInvalid() && !Trap.hasErrorOccurred() &&
+        TransExprRes.get()->hasPlaceholderType())
+      TransExprRes = SemaRef.CheckPlaceholderExpr(TransExprRes.get());
     if (TransExprRes.isInvalid() || Trap.hasErrorOccurred())
       TransExpr = createSubstDiag(SemaRef, Info, [&](llvm::raw_ostream &OS) {
         E->printPretty(OS, nullptr, SemaRef.getPrintingPolicy());

diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index e43b3ca..5c37fca 100644 (file)
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -12494,6 +12494,8 @@ TreeTransform<Derived>::TransformExprRequirement(concepts::ExprRequirement *Req)
     TransExpr = Req->getExprSubstitutionDiagnostic();
   else {
     ExprResult TransExprRes = getDerived().TransformExpr(Req->getExpr());
+    if (TransExprRes.isUsable() && TransExprRes.get()->hasPlaceholderType())
+      TransExprRes = SemaRef.CheckPlaceholderExpr(TransExprRes.get());
     if (TransExprRes.isInvalid())
       return nullptr;
     TransExpr = TransExprRes.get();

diff --git a/clang/test/SemaTemplate/constraints.cpp b/clang/test/SemaTemplate/constraints.cpp
index e2bb655..0bc4727 100644 (file)
--- a/clang/test/SemaTemplate/constraints.cpp
+++ b/clang/test/SemaTemplate/constraints.cpp
@@ -24,35 +24,3 @@ namespace PR45589 {
   // FIXME: These diagnostics are excessive.
   static_assert(test<false, char> == 1); // expected-note 2{{while}} expected-note 2{{during}}
 }
-
-namespace PR52905 {
-// A mock for std::convertible_to. Not complete support.
-template <typename _From, typename _To>
-concept convertible_to = __is_convertible_to(_From, _To); // expected-note {{evaluated to false}}
-
-template <typename T>
-class A {
-public:
-  using iterator = void **;
-
-  iterator begin();
-  const iterator begin() const;
-};
-
-template <class T>
-concept Beginable1 = requires(T t) {
-  { t.begin }
-  ->convertible_to<typename T::iterator>; // expected-note {{not satisfied}}
-};
-
-static_assert(Beginable1<A<int>>); // expected-error {{static_assert failed}}
-                                   // expected-note@-1 {{does not satisfy 'Beginable1'}}
-
-template <class T>
-concept Beginable2 = requires(T t) {
-  { t.begin() }
-  ->convertible_to<typename T::iterator>;
-};
-
-static_assert(Beginable2<A<int>>);
-} // namespace PR52905

diff --git a/clang/test/SemaTemplate/pr52909.cpp b/clang/test/SemaTemplate/pr52909.cpp
new file mode 100644 (file)
index 0000000..a17ede4
--- /dev/null
+++ b/clang/test/SemaTemplate/pr52909.cpp
@@ -0,0 +1,71 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+// RUN: %clang_cc1 -std=c++2b -verify %s
+
+namespace PR52905 {
+template <class> concept C = true;
+
+struct A {
+  int begin();
+  int begin() const;
+};
+
+template <class T>
+concept Beginable = requires (T t) {
+  { t.begin } -> C;
+  // expected-note@-1 {{because 't.begin' would be invalid: reference to non-static member function must be called}}
+};
+
+static_assert(Beginable<A>); // expected-error {{static_assert failed}}
+                             // expected-note@-1 {{does not satisfy 'Beginable'}}
+} // namespace PR52905
+
+namespace PR52909a {
+
+template<class> constexpr bool B = true;
+template<class T> concept True = B<T>;
+
+template <class T>
+int foo(T t) requires requires { // expected-note {{candidate template ignored: constraints not satisfied}}
+    {t.begin} -> True; // expected-note {{because 't.begin' would be invalid: reference to non-static member function must be called}}
+}
+{}
+
+struct A { int begin(); };
+auto x = foo(A()); // expected-error {{no matching function for call to 'foo'}}
+
+} // namespace PR52909a
+
+namespace PR52909b {
+
+template<class> concept True = true;
+
+template<class T> concept C = requires {
+    { T::begin } -> True; // expected-note {{because 'T::begin' would be invalid: reference to overloaded function could not be resolved}}
+};
+
+struct A {
+    static void begin(int);
+    static void begin(double);
+};
+
+static_assert(C<A>); // expected-error {{static_assert failed}}
+  // expected-note@-1 {{because 'PR52909b::A' does not satisfy 'C'}}
+
+} // namespace PR52909b
+
+namespace PR53075 {
+template<class> concept True = true;
+
+template<class T> concept C = requires {
+    { &T::f } -> True; // expected-note {{because '&T::f' would be invalid: reference to overloaded function could not be resolved}}
+};
+
+struct S {
+    int *f();
+    int *f() const;
+};
+
+static_assert(C<S>); // expected-error {{static_assert failed}}
+  // expected-note@-1 {{because 'PR53075::S' does not satisfy 'C'}}
+
+} // namespace PR53075