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

Bug llvm#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 llvm#52905.
Fixes llvm#52909.
Fixes llvm#53075.

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

(cherry picked from commit f6ce456)

Co-authored-by: Arthur O'Dwyer <[email protected]>

Author: ahatanaka

clang/lib/AST/ASTContext.cpp

@@ -3378,8 +3378,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.
@@ -3417,6 +3418,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;

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());

clang/lib/Sema/TreeTransform.h

@@ -12392,6 +12392,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();

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