[clang] 7ff4f48 - Relax assert in ExprConstant to a return None.
Jon Chesterfield via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 21 05:10:12 PDT 2021
Author: Jon Chesterfield
Date: 2021-10-21T13:09:56+01:00
New Revision: 7ff4f48adb26429086d6fd85a14336e57cd340dc
URL: https://github.com/llvm/llvm-project/commit/7ff4f48adb26429086d6fd85a14336e57cd340dc
DIFF: https://github.com/llvm/llvm-project/commit/7ff4f48adb26429086d6fd85a14336e57cd340dc.diff
LOG: Relax assert in ExprConstant to a return None.
Fixes a compiler assert on passing a compile time integer to atomic builtins.
Assert introduced in D61522
Function changed from ->bool to ->Optional in D76646
Simplifies call sites to getIntegerConstantExpr to elide the now-redundant
isValueDependent checks.
Reviewed By: aaron.ballman
Differential Revision: https://reviews.llvm.org/D112159
Added:
Modified:
clang/lib/AST/ExprConstant.cpp
clang/lib/Sema/SemaAttr.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/lib/Sema/SemaTemplateDeduction.cpp
clang/lib/Sema/SemaType.cpp
clang/test/SemaTemplate/atomics.cpp
Removed:
################################################################################
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 59943d19a3e42..c74e4c96f7b0f 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -15501,8 +15501,10 @@ bool Expr::isIntegerConstantExpr(const ASTContext &Ctx,
Optional<llvm::APSInt> Expr::getIntegerConstantExpr(const ASTContext &Ctx,
SourceLocation *Loc,
bool isEvaluated) const {
- assert(!isValueDependent() &&
- "Expression evaluator can't be called on a dependent expression.");
+ if (isValueDependent()) {
+ // Expression evaluator can't succeed on a dependent expression.
+ return None;
+ }
APSInt Value;
diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp
index 40d8beab76057..100f8e36a9b86 100644
--- a/clang/lib/Sema/SemaAttr.cpp
+++ b/clang/lib/Sema/SemaAttr.cpp
@@ -340,7 +340,7 @@ void Sema::ActOnPragmaPack(SourceLocation PragmaLoc, PragmaMsStackAction Action,
// pack(0) is like pack(), which just works out since that is what
// we use 0 for in PackAttr.
- if (Alignment->isTypeDependent() || Alignment->isValueDependent() || !Val ||
+ if (Alignment->isTypeDependent() || !Val ||
!(*Val == 0 || Val->isPowerOf2()) || Val->getZExtValue() > 16) {
Diag(PragmaLoc, diag::warn_pragma_pack_invalid_alignment);
return; // Ignore
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 5b9568c1ed95f..9c4e3ca103624 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -216,7 +216,7 @@ static bool checkUInt32Argument(Sema &S, const AttrInfo &AI, const Expr *Expr,
uint32_t &Val, unsigned Idx = UINT_MAX,
bool StrictlyUnsigned = false) {
Optional<llvm::APSInt> I = llvm::APSInt(32);
- if (Expr->isTypeDependent() || Expr->isValueDependent() ||
+ if (Expr->isTypeDependent() ||
!(I = Expr->getIntegerConstantExpr(S.Context))) {
if (Idx != UINT_MAX)
S.Diag(getAttrLoc(AI), diag::err_attribute_argument_n_type)
@@ -308,7 +308,7 @@ static bool checkFunctionOrMethodParameterIndex(
(HP ? getFunctionOrMethodNumParams(D) : 0) + HasImplicitThisParam;
Optional<llvm::APSInt> IdxInt;
- if (IdxExpr->isTypeDependent() || IdxExpr->isValueDependent() ||
+ if (IdxExpr->isTypeDependent() ||
!(IdxInt = IdxExpr->getIntegerConstantExpr(S.Context))) {
S.Diag(getAttrLoc(AI), diag::err_attribute_argument_n_type)
<< &AI << AttrArgNum << AANT_ArgumentIntegerConstant
@@ -2860,8 +2860,7 @@ static void handleSentinelAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
if (AL.getNumArgs() > 0) {
Expr *E = AL.getArgAsExpr(0);
Optional<llvm::APSInt> Idx = llvm::APSInt(32);
- if (E->isTypeDependent() || E->isValueDependent() ||
- !(Idx = E->getIntegerConstantExpr(S.Context))) {
+ if (E->isTypeDependent() || !(Idx = E->getIntegerConstantExpr(S.Context))) {
S.Diag(AL.getLoc(), diag::err_attribute_argument_n_type)
<< AL << 1 << AANT_ArgumentIntegerConstant << E->getSourceRange();
return;
@@ -2880,8 +2879,7 @@ static void handleSentinelAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
if (AL.getNumArgs() > 1) {
Expr *E = AL.getArgAsExpr(1);
Optional<llvm::APSInt> Idx = llvm::APSInt(32);
- if (E->isTypeDependent() || E->isValueDependent() ||
- !(Idx = E->getIntegerConstantExpr(S.Context))) {
+ if (E->isTypeDependent() || !(Idx = E->getIntegerConstantExpr(S.Context))) {
S.Diag(AL.getLoc(), diag::err_attribute_argument_n_type)
<< AL << 2 << AANT_ArgumentIntegerConstant << E->getSourceRange();
return;
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 0f3f50c8f6c6f..571b790b31eaa 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -2137,39 +2137,38 @@ Sema::BuildCXXNew(SourceRange Range, bool UseGlobal,
// Let's see if this is a constant < 0. If so, we reject it out of hand,
// per CWG1464. Otherwise, if it's not a constant, we must have an
// unparenthesized array type.
- if (!(*ArraySize)->isValueDependent()) {
- // We've already performed any required implicit conversion to integer or
- // unscoped enumeration type.
- // FIXME: Per CWG1464, we are required to check the value prior to
- // converting to size_t. This will never find a negative array size in
- // C++14 onwards, because Value is always unsigned here!
- if (Optional<llvm::APSInt> Value =
- (*ArraySize)->getIntegerConstantExpr(Context)) {
- if (Value->isSigned() && Value->isNegative()) {
- return ExprError(Diag((*ArraySize)->getBeginLoc(),
- diag::err_typecheck_negative_array_size)
- << (*ArraySize)->getSourceRange());
- }
-
- if (!AllocType->isDependentType()) {
- unsigned ActiveSizeBits = ConstantArrayType::getNumAddressingBits(
- Context, AllocType, *Value);
- if (ActiveSizeBits > ConstantArrayType::getMaxSizeBits(Context))
- return ExprError(
- Diag((*ArraySize)->getBeginLoc(), diag::err_array_too_large)
- << toString(*Value, 10) << (*ArraySize)->getSourceRange());
- }
- KnownArraySize = Value->getZExtValue();
- } else if (TypeIdParens.isValid()) {
- // Can't have dynamic array size when the type-id is in parentheses.
- Diag((*ArraySize)->getBeginLoc(), diag::ext_new_paren_array_nonconst)
- << (*ArraySize)->getSourceRange()
- << FixItHint::CreateRemoval(TypeIdParens.getBegin())
- << FixItHint::CreateRemoval(TypeIdParens.getEnd());
+ // We've already performed any required implicit conversion to integer or
+ // unscoped enumeration type.
+ // FIXME: Per CWG1464, we are required to check the value prior to
+ // converting to size_t. This will never find a negative array size in
+ // C++14 onwards, because Value is always unsigned here!
+ if (Optional<llvm::APSInt> Value =
+ (*ArraySize)->getIntegerConstantExpr(Context)) {
+ if (Value->isSigned() && Value->isNegative()) {
+ return ExprError(Diag((*ArraySize)->getBeginLoc(),
+ diag::err_typecheck_negative_array_size)
+ << (*ArraySize)->getSourceRange());
+ }
- TypeIdParens = SourceRange();
+ if (!AllocType->isDependentType()) {
+ unsigned ActiveSizeBits =
+ ConstantArrayType::getNumAddressingBits(Context, AllocType, *Value);
+ if (ActiveSizeBits > ConstantArrayType::getMaxSizeBits(Context))
+ return ExprError(
+ Diag((*ArraySize)->getBeginLoc(), diag::err_array_too_large)
+ << toString(*Value, 10) << (*ArraySize)->getSourceRange());
}
+
+ KnownArraySize = Value->getZExtValue();
+ } else if (TypeIdParens.isValid()) {
+ // Can't have dynamic array size when the type-id is in parentheses.
+ Diag((*ArraySize)->getBeginLoc(), diag::ext_new_paren_array_nonconst)
+ << (*ArraySize)->getSourceRange()
+ << FixItHint::CreateRemoval(TypeIdParens.getBegin())
+ << FixItHint::CreateRemoval(TypeIdParens.getEnd());
+
+ TypeIdParens = SourceRange();
}
// Note that we do *not* convert the argument in any way. It can
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp
index 424342df7725b..facd3e0998e8f 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -2169,11 +2169,10 @@ DeduceTemplateArgumentsByTypeMatch(Sema &S,
}
Expr *ArgExpr = (ArgDepMatrix->*GetArgDimensionExpr)();
- if (!ArgExpr->isValueDependent())
- if (Optional<llvm::APSInt> ArgConst =
- ArgExpr->getIntegerConstantExpr(S.Context))
- if (*ArgConst == *ParamConst)
- return Sema::TDK_Success;
+ if (Optional<llvm::APSInt> ArgConst =
+ ArgExpr->getIntegerConstantExpr(S.Context))
+ if (*ArgConst == *ParamConst)
+ return Sema::TDK_Success;
return Sema::TDK_NonDeducedMismatch;
}
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index d2d54281c63e0..444c73360edf4 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -7814,7 +7814,7 @@ static bool isPermittedNeonBaseType(QualType &Ty,
static bool verifyValidIntegerConstantExpr(Sema &S, const ParsedAttr &Attr,
llvm::APSInt &Result) {
const auto *AttrExpr = Attr.getArgAsExpr(0);
- if (!AttrExpr->isTypeDependent() && !AttrExpr->isValueDependent()) {
+ if (!AttrExpr->isTypeDependent()) {
if (Optional<llvm::APSInt> Res =
AttrExpr->getIntegerConstantExpr(S.Context)) {
Result = *Res;
diff --git a/clang/test/SemaTemplate/atomics.cpp b/clang/test/SemaTemplate/atomics.cpp
index 19b607f738a81..609c9595263a4 100644
--- a/clang/test/SemaTemplate/atomics.cpp
+++ b/clang/test/SemaTemplate/atomics.cpp
@@ -15,3 +15,13 @@ namespace PR11320 {
}
void h() { g<int>(0); }
}
+
+// Can pass value dependent integer to atomic builtin
+template <int Order>
+void fetchAdd(int *A, int V) {
+ __atomic_fetch_add(A, V, Order);
+}
+
+void fetchAddUse(int *A, int V) {
+ fetchAdd<__ATOMIC_ACQ_REL>(A, V);
+}
More information about the cfe-commits
mailing list