erichkeane updated this revision to Diff 490278.
erichkeane added a comment.
Added tests suggested by Tom, also moved the cast creation after checking of
the return type, so that we end up not hitting hte assert that Tom came up
with. Added 2 tests that show the two conditions (that DID repro as a crash).
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D141954/new/
https://reviews.llvm.org/D141954
Files:
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaConcept.cpp
clang/test/CXX/temp/temp.constr/temp.constr.atomic/constrant-satisfaction-conversions.cpp
Index: clang/test/CXX/temp/temp.constr/temp.constr.atomic/constrant-satisfaction-conversions.cpp
===================================================================
--- /dev/null
+++ clang/test/CXX/temp/temp.constr/temp.constr.atomic/constrant-satisfaction-conversions.cpp
@@ -0,0 +1,67 @@
+// RUN: %clang_cc1 -std=c++20 -x c++ -Wno-constant-logical-operand -verify %s
+
+template<typename T> concept C =
+sizeof(T) == 4 && !true; // requires atomic constraints sizeof(T) == 4 and !true
+
+template<typename T> concept C2 = sizeof(T); // expected-error{{atomic constraint must be of type 'bool' (found }}
+
+template<typename T> struct S {
+ constexpr operator bool() const { return true; }
+};
+
+// expected-error@+3{{atomic constraint must be of type 'bool' (found 'S<int>')}}
+// expected-note@#FINST{{while checking constraint satisfaction}}
+// expected-note@#FINST{{in instantiation of function template specialization}}
+template<typename T> requires (S<T>{})
+void f(T);
+void f(int);
+
+// Ensure this applies to operator && as well.
+// expected-error@+3{{atomic constraint must be of type 'bool' (found 'S<int>')}}
+// expected-note@#F2INST{{while checking constraint satisfaction}}
+// expected-note@#F2INST{{in instantiation of function template specialization}}
+template<typename T> requires (S<T>{} && true)
+void f2(T);
+void f2(int);
+
+template<typename T> requires requires {
+ requires S<T>{};
+ // expected-error@-1{{atomic constraint must be of type 'bool' (found 'S<int>')}}
+ // expected-note@-2{{while checking the satisfaction}}
+ // expected-note@-3{{in instantiation of requirement}}
+ // expected-note@-4{{while checking the satisfaction}}
+ // expected-note@-6{{while substituting template arguments}}
+ // expected-note@#F3INST{{while checking constraint satisfaction}}
+ // expected-note@#F3INST{{in instantiation of function template specialization}}
+ //
+}
+void f3(T);
+void f3(int);
+
+// Doesn't diagnose, since this is no longer a compound requirement.
+template<typename T> requires (bool(1 && 2))
+void f4(T);
+void f4(int);
+
+void g() {
+ f(0); // #FINST
+ f2(0); // #F2INST
+ f3(0); // #F3INST
+ f4(0);
+}
+
+template<typename T>
+auto Nullptr = nullptr;
+
+template<typename T> concept NullTy = Nullptr<T>;
+// expected-error@-1{{atomic constraint must be of type 'bool' (found }}
+// expected-note@+1{{while checking the satisfaction}}
+static_assert(NullTy<int>);
+
+template<typename T>
+auto Struct = S<T>{};
+
+template<typename T> concept StructTy = Struct<T>;
+// expected-error@-1{{atomic constraint must be of type 'bool' (found 'S<int>')}}
+// expected-note@+1{{while checking the satisfaction}}
+static_assert(StructTy<int>);
Index: clang/lib/Sema/SemaConcept.cpp
===================================================================
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -329,14 +329,7 @@
Sema::SFINAETrap Trap(S);
SubstitutedExpression =
S.SubstConstraintExpr(const_cast<Expr *>(AtomicExpr), MLTAL);
- // Substitution might have stripped off a contextual conversion to
- // bool if this is the operand of an '&&' or '||'. For example, we
- // might lose an lvalue-to-rvalue conversion here. If so, put it back
- // before we try to evaluate.
- if (SubstitutedExpression.isUsable() &&
- !SubstitutedExpression.isInvalid())
- SubstitutedExpression =
- S.PerformContextuallyConvertToBool(SubstitutedExpression.get());
+
if (SubstitutedExpression.isInvalid() || Trap.hasErrorOccurred()) {
// C++2a [temp.constr.atomic]p1
// ...If substitution results in an invalid type or expression, the
@@ -373,6 +366,23 @@
if (!S.CheckConstraintExpression(SubstitutedExpression.get()))
return ExprError();
+ // [temp.constr.atomic]p3: To determine if an atomic constraint is
+ // satisfied, the parameter mapping and template arguments are first
+ // substituted into its expression. If substitution results in an
+ // invalid type or expression, the constraint is not satisfied.
+ // Otherwise, the lvalue-to-rvalue conversion is performed if necessary,
+ // and E shall be a constant expression of type bool.
+ //
+ // Perform the L to R Value conversion if necessary. We do so for all
+ // non-PRValue categories, else we fail to extend the lifetime of
+ // temporaries, and that fails the constant expression check.
+ if (!SubstitutedExpression.get()->isPRValue())
+ SubstitutedExpression = ImplicitCastExpr::Create(
+ S.Context, SubstitutedExpression.get()->getType(),
+ CK_LValueToRValue, SubstitutedExpression.get(),
+ /*BasePath=*/nullptr, VK_PRValue, FPOptionsOverride());
+
+
return SubstitutedExpression;
});
}
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -767,6 +767,9 @@
and `P1975R0: <https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1975r0.html>`_,
which allows parenthesized aggregate-initialization.
+- Fixed an issue with concept requirement evaluation, where we incorrectly allowed implicit
+ conversions to bool for a requirement. This fixes `GH54524 <https://github.com/llvm/llvm-project/issues/54524>`_.
+
C++2b Feature Support
^^^^^^^^^^^^^^^^^^^^^
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
