[Clang] Implement CWG2517 Useless restriction on use of parameter in constraint-expression

[imdj]

Remove [expr.prim.req.nested] check which restrict that local parameters in constraint-expressions can only appear as unevaluated operands. This change makes the treatment of examples like requires expressions and other constant expression contexts uniform, consistent with the adoption of P2280.

References:

• https://cplusplus.github.io/CWG/issues/2517.html
• Incomplete implementation of P2280 / CWG2517 #132825

[llvmbot]

@llvm/pr-subscribers-clang

Author: Imad Aldij (imdj)

Changes

Remove [expr.prim.req.nested] check which restrict that local parameters in constraint-expressions can only appear as unevaluated operands. This change makes the treatment of examples like requires expressions and other constant expression contexts uniform, consistent with the adoption of P2280.

References:

• https://cplusplus.github.io/CWG/issues/2517.html
• #132825

---

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

2 Files Affected:

• (modified) clang/lib/Sema/SemaExpr.cpp (-11)
• (modified) clang/test/SemaCXX/constant-expression-p2280r4.cpp (+13)

diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 3af6d6c23438f..fa8eeb644c179 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -396,17 +396,6 @@ bool Sema::DiagnoseUseOfDecl(NamedDecl *D, ArrayRef<SourceLocation> Locs,
           targetDiag(*Locs.begin(), diag::err_thread_unsupported);
   }
 
-  if (isa<ParmVarDecl>(D) && isa<RequiresExprBodyDecl>(D->getDeclContext()) &&
-      !isUnevaluatedContext()) {
-    // C++ [expr.prim.req.nested] p3
-    //   A local parameter shall only appear as an unevaluated operand
-    //   (Clause 8) within the constraint-expression.
-    Diag(Loc, diag::err_requires_expr_parameter_referenced_in_evaluated_context)
-        << D;
-    Diag(D->getLocation(), diag::note_entity_declared_at) << D;
-    return true;
-  }
-
   return false;
 }
 
diff --git a/clang/test/SemaCXX/constant-expression-p2280r4.cpp b/clang/test/SemaCXX/constant-expression-p2280r4.cpp
index f22430a0e88a2..9baedd9c6c263 100644
--- a/clang/test/SemaCXX/constant-expression-p2280r4.cpp
+++ b/clang/test/SemaCXX/constant-expression-p2280r4.cpp
@@ -155,6 +155,19 @@ int g() {
 }
 }
 
+namespace GH132825 {
+template<typename ArrayType> concept LargeArray =
+    requires (ArrayType my_array) { requires my_array.size() > 5; };
+
+struct Big {
+  constexpr int size() const { return 100; }
+};
+
+void g() {
+  static_assert(LargeArray<Big>);
+}
+}
+
 namespace GH128409 {
   int &ff();
   int &x = ff(); // nointerpreter-note {{declared here}}

[zyn0217]

Thanks for the prompt patch.

[cor3ntin]

Thanks for working on this

[cor3ntin]

Do we have a test for the cases where the expression is not a constant expression?

[imdj]

Would this be covered by the tests we updated in nested-requirement.cpp? Please let me know what you think or your suggestions since I'm not very experienced here.

llvm-project/clang/test/CXX/expr/expr.prim/expr.prim.req/nested-requirement.cpp

Lines 41 to 54 in 5af23bf

	template<typename T>
	concept C2 = requires (T a) {
	requires sizeof(a) == 4; // OK
	requires a == 0; // expected-error{{substitution into constraint expression resulted in a non-constant expression}}
	// expected-note@-1{{while checking the satisfaction of nested requirement requested here}}
	// expected-note@-2{{in instantiation of requirement here}}
	// expected-note@-3{{while checking the satisfaction of nested requirement requested here}}
	// expected-note@-6{{while substituting template arguments into constraint expression here}}
	// expected-note@-5{{function parameter 'a' with unknown value cannot be used in a constant expression}}
	// expected-note@-8{{declared here}}
	};
	static_assert(C2<int>); // expected-error{{static assertion failed}}
	// expected-note@-1{{while checking the satisfaction of concept 'C2<int>' requested here}}
	// expected-note@-2{{because 'int' does not satisfy 'C2'}}

[Endilll]

If you update your branch with changes from main, unrelated changes in cxx_dr_status.html will disappear.

[shafik]

CC @mizvekov since it was suggested this is related to #126550 and it looked like based on the conversation there, there was some existing work going on.

[zyn0217]

CC @mizvekov since it was suggested this is related to #126550 and it looked like based on the conversation there, there was some existing work going on.

I think the crash I presented is independent of this DR. (This DR makes that the parameters are usable within an unevaluated requires context) while that crash has to do with the lambda context, which is what Mizvekov has been working on.

[mizvekov]

Yeah I agree with @zyn0217 here. Otherwise I haven't been able to take a look at the crash yet.