In ExprRequirement building, treat OverloadExpr as dependent

[erichkeane]

As reported in #66612, we aren't correctly treating the placeholder expression type correctly, so we ended up trying to get a reference version of it, and this resulted in an assertion, since the placeholder type cannot have a reference added.

Fixes: #66612

[llvmbot]

@llvm/pr-subscribers-clang

Changes

As reported in #66612, we aren't correctly treating the placeholder expression type correctly, so we ended up trying to get a reference version of it, and this resulted in an assertion, since the placeholder type cannot have a reference added.

Fixes: #66612

---

Full diff: https://github.com/llvm/llvm-project/pull/66683.diff

2 Files Affected:

• (modified) clang/lib/Sema/SemaExprCXX.cpp (+2-1)
• (modified) clang/test/SemaTemplate/concepts.cpp (+17)

diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index bb4ef065d5c72aa..77289595972e3cf 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -9063,7 +9063,8 @@ Sema::BuildExprRequirement(
     concepts::ExprRequirement::ReturnTypeRequirement ReturnTypeRequirement) {
   auto Status = concepts::ExprRequirement::SS_Satisfied;
   ConceptSpecializationExpr *SubstitutedConstraintExpr = nullptr;
-  if (E->isInstantiationDependent() || ReturnTypeRequirement.isDependent())
+  if (E->isInstantiationDependent() || E->getType()->isPlaceholderType() ||
+      ReturnTypeRequirement.isDependent())
     Status = concepts::ExprRequirement::SS_Dependent;
   else if (NoexceptLoc.isValid() && canThrow(E) == CanThrowResult::CT_Can)
     Status = concepts::ExprRequirement::SS_NoexceptNotMet;
diff --git a/clang/test/SemaTemplate/concepts.cpp b/clang/test/SemaTemplate/concepts.cpp
index 891b45aa5789296..68050e0f09e248a 100644
--- a/clang/test/SemaTemplate/concepts.cpp
+++ b/clang/test/SemaTemplate/concepts.cpp
@@ -1031,3 +1031,20 @@ void test() {
     fff(42UL); // expected-error {{no matching function}}
 }
 }
+
+namespace GH66612 {
+  template<typename C>
+    auto end(C c) ->int;
+
+  template <typename T>
+    concept Iterator = true;
+
+  template <typename CT>
+    concept Container = requires(CT b) {
+        { end } -> Iterator; // #66612GH_END
+    };
+
+  static_assert(Container<int>);// expected-error{{static assertion failed}}
+  // expected-note@-1{{because 'int' does not satisfy 'Container'}}
+  // expected-note@#66612GH_END{{because 'end' would be invalid: reference to overloaded function could not be resolved; did you mean to call it?}}
+}

[erichkeane]

This makes sense in my head, as what is currently an unresolved overload expr(or other overload expr) could potentially become legal after instantiation I think? So this should result in this being instantiated later/checked for correctness then.

[cor3ntin]

I've been trying to think about whether checking UnresolvedLookupExpr would make more sense but
I think what you have works in all cases.

[erichkeane]

I was concerned about other expression types as well, like OverloadExpr or placeholders that got put in elsewise, so I was hoping this was sufficiently generic here.

[cor3ntin]

Do we need a release note for this (probably!)?

[erichkeane]

Do we need a release note for this (probably!)?

Bah, yes we do...

[cor3ntin]

LGTM (but now you have conflicts in the release notes!)

[erichkeane]

Ooof... The builtin merge tool doesn't show trailing whitespace, so some leftover whitespace caused it to fail to verify!