[clang] b5d9f00 - Forbid implicit conversion of constraint expression to bool
Erich Keane via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 19 10:13:22 PST 2023
Author: Erich Keane
Date: 2023-01-19T10:12:59-08:00
New Revision: b5d9f00b2096290653fcb6e8de38d5c352af63a0
URL: https://github.com/llvm/llvm-project/commit/b5d9f00b2096290653fcb6e8de38d5c352af63a0
DIFF: https://github.com/llvm/llvm-project/commit/b5d9f00b2096290653fcb6e8de38d5c352af63a0.diff
LOG: Forbid implicit conversion of constraint expression to bool
As reported in https://github.com/llvm/llvm-project/issues/54524, and
later in https://github.com/llvm/llvm-project/issues/60038, we were not
properly implmenting temp.constr.atomic P3. This patch stops implicitly
converting constraints to bool, and ensures the Rvalue conversion takes
place as needed.
Differential Revision: https://reviews.llvm.org/D141954
Added:
clang/test/CXX/temp/temp.constr/temp.constr.atomic/constrant-satisfaction-conversions.cpp
Modified:
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaConcept.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8de179cb89617..97ce8866d3ccf 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -774,6 +774,9 @@ C++20 Feature Support
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
^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index f20e4751f41a3..4d4b2482d046e 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -329,14 +329,7 @@ static ExprResult calculateConstraintSatisfaction(
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,22 @@ static ExprResult calculateConstraintSatisfaction(
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;
});
}
diff --git a/clang/test/CXX/temp/temp.constr/temp.constr.atomic/constrant-satisfaction-conversions.cpp b/clang/test/CXX/temp/temp.constr/temp.constr.atomic/constrant-satisfaction-conversions.cpp
new file mode 100644
index 0000000000000..ba8e2dc372e98
--- /dev/null
+++ b/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 at +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 at +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 at -1{{atomic constraint must be of type 'bool' (found 'S<int>')}}
+ // expected-note at -2{{while checking the satisfaction}}
+ // expected-note at -3{{in instantiation of requirement}}
+ // expected-note at -4{{while checking the satisfaction}}
+ // expected-note at -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 at -1{{atomic constraint must be of type 'bool' (found }}
+// expected-note at +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 at -1{{atomic constraint must be of type 'bool' (found 'S<int>')}}
+// expected-note at +1{{while checking the satisfaction}}
+static_assert(StructTy<int>);
More information about the cfe-commits
mailing list