replica
/
hanchenye-llvm-project
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Switch the default of VerifyIntegerConstantExpression from constant 

folding to not constant folding.

Constant folding of ICEs is done as a GCC compatibility measure, but new
code was picking it up, presumably by accident, due to the bad default.

While here, also switch the flag from a bool to an enum to make it more
obvious what it means at call sites. This highlighted a couple of places
where our behavior is different between C++11 and C++14 due to switching
from checking for an ICE to checking for a converted constant
expression (where there is no 'fold' codepath).
This commit is contained in:
Richard Smith 2020-10-15 16:50:13 -07:00
parent 2bf423b021
commit fc031d29be
14 changed files with 83 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
clang/include/clang/Sema/Sema.h
View File

@ -11608,17 +11608,27 @@ public:
virtual ~VerifyICEDiagnoser() {}
};
enum AllowFoldKind {
NoFold,
AllowFold,
};
/// VerifyIntegerConstantExpression - Verifies that an expression is an ICE,
/// and reports the appropriate diagnostics. Returns false on success.
/// Can optionally return the value of the expression.
ExprResult VerifyIntegerConstantExpression(Expr *E, llvm::APSInt *Result,
VerifyICEDiagnoser &Diagnoser,
bool AllowFold = true);
AllowFoldKind CanFold = NoFold);
ExprResult VerifyIntegerConstantExpression(Expr *E, llvm::APSInt *Result,
unsigned DiagID,
bool AllowFold = true);
AllowFoldKind CanFold = NoFold);
ExprResult VerifyIntegerConstantExpression(Expr *E,
llvm::APSInt *Result = nullptr);
llvm::APSInt *Result = nullptr,
AllowFoldKind CanFold = NoFold);
ExprResult VerifyIntegerConstantExpression(Expr *E,
AllowFoldKind CanFold = NoFold) {
return VerifyIntegerConstantExpression(E, nullptr, CanFold);
}
/// VerifyBitField - verifies that a bit field expression is an ICE and has
/// the correct width, and that the field type is valid.

9
clang/lib/Sema/SemaDecl.cpp
View File

@ -16418,7 +16418,7 @@ ExprResult Sema::VerifyBitField(SourceLocation FieldLoc,
return BitWidth;
llvm::APSInt Value;
ExprResult ICE = VerifyIntegerConstantExpression(BitWidth, &Value);
ExprResult ICE = VerifyIntegerConstantExpression(BitWidth, &Value, AllowFold);
if (ICE.isInvalid())
return ICE;
BitWidth = ICE.get();
@ -17536,6 +17536,8 @@ EnumConstantDecl *Sema::CheckEnumConstant(EnumDecl *Enum,
if (Enum->isDependentType() || Val->isTypeDependent())
EltTy = Context.DependentTy;
else {
// FIXME: We don't allow folding in C++11 mode for an enum with a fixed
// underlying type, but do allow it in all other contexts.
if (getLangOpts().CPlusPlus11 && Enum->isFixed()) {
// C++11 [dcl.enum]p5: If the underlying type is fixed, [...] the
// constant-expression in the enumerator-definition shall be a converted
@ -17549,8 +17551,9 @@ EnumConstantDecl *Sema::CheckEnumConstant(EnumDecl *Enum,
else
Val = Converted.get();
} else if (!Val->isValueDependent() &&
!(Val = VerifyIntegerConstantExpression(Val,
&EnumVal).get())) {
!(Val =
VerifyIntegerConstantExpression(Val, &EnumVal, AllowFold)
.get())) {
// C99 6.7.2.2p2: Make sure we have an integer constant expression.
} else {
if (Enum->isComplete()) {

12
clang/lib/Sema/SemaDeclAttr.cpp
View File

@ -3723,10 +3723,8 @@ void Sema::AddAlignValueAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E) {
if (!E->isValueDependent()) {
llvm::APSInt Alignment;
ExprResult ICE
= VerifyIntegerConstantExpression(E, &Alignment,
diag::err_align_value_attribute_argument_not_int,
/*AllowFold*/ false);
ExprResult ICE = VerifyIntegerConstantExpression(
E, &Alignment, diag::err_align_value_attribute_argument_not_int);
if (ICE.isInvalid())
return;
@ -3832,10 +3830,8 @@ void Sema::AddAlignedAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E,
// FIXME: Cache the number on the AL object?
llvm::APSInt Alignment;
ExprResult ICE
= VerifyIntegerConstantExpression(E, &Alignment,
diag::err_aligned_attribute_argument_not_int,
/*AllowFold*/ false);
ExprResult ICE = VerifyIntegerConstantExpression(
E, &Alignment, diag::err_aligned_attribute_argument_not_int);
if (ICE.isInvalid())
return;

9
clang/lib/Sema/SemaDeclCXX.cpp
View File

@ -1078,7 +1078,7 @@ static IsTupleLike isTupleLike(Sema &S, SourceLocation Loc, QualType T,
if (E.isInvalid())
return IsTupleLike::Error;
E = S.VerifyIntegerConstantExpression(E.get(), &Size, Diagnoser, false);
E = S.VerifyIntegerConstantExpression(E.get(), &Size, Diagnoser);
if (E.isInvalid())
return IsTupleLike::Error;
@ -15996,9 +15996,10 @@ Decl *Sema::BuildStaticAssertDeclaration(SourceLocation StaticAssertLoc,
AssertExpr = FullAssertExpr.get();
llvm::APSInt Cond;
if (!Failed && VerifyIntegerConstantExpression(AssertExpr, &Cond,
diag::err_static_assert_expression_is_not_constant,
/*AllowFold=*/false).isInvalid())
if (!Failed && VerifyIntegerConstantExpression(
AssertExpr, &Cond,
diag::err_static_assert_expression_is_not_constant)
.isInvalid())
Failed = true;
if (!Failed && !Cond) {

4
clang/lib/Sema/SemaExceptionSpec.cpp
View File

@ -99,9 +99,7 @@ ExprResult Sema::ActOnNoexceptSpec(SourceLocation NoexceptLoc,
llvm::APSInt Result;
Converted = VerifyIntegerConstantExpression(
Converted.get(), &Result,
diag::err_noexcept_needs_constant_expression,
/*AllowFold*/ false);
Converted.get(), &Result, diag::err_noexcept_needs_constant_expression);
if (!Converted.isInvalid())
EST = !Result ? EST_NoexceptFalse : EST_NoexceptTrue;
return Converted;

18
clang/lib/Sema/SemaExpr.cpp
View File

@ -14989,9 +14989,8 @@ ExprResult Sema::ActOnChooseExpr(SourceLocation BuiltinLoc,
} else {
// The conditional expression is required to be a constant expression.
llvm::APSInt condEval(32);
ExprResult CondICE
= VerifyIntegerConstantExpression(CondExpr, &condEval,
diag::err_typecheck_choose_expr_requires_constant, false);
ExprResult CondICE = VerifyIntegerConstantExpression(
CondExpr, &condEval, diag::err_typecheck_choose_expr_requires_constant);
if (CondICE.isInvalid())
return ExprError();
CondExpr = CondICE.get();
@ -15853,7 +15852,8 @@ bool Sema::DiagnoseAssignmentResult(AssignConvertType ConvTy,
}
ExprResult Sema::VerifyIntegerConstantExpression(Expr *E,
llvm::APSInt *Result) {
llvm::APSInt *Result,
AllowFoldKind CanFold) {
class SimpleICEDiagnoser : public VerifyICEDiagnoser {
public:
SemaDiagnosticBuilder diagnoseNotICEType(Sema &S, SourceLocation Loc,
@ -15866,13 +15866,13 @@ ExprResult Sema::VerifyIntegerConstantExpression(Expr *E,
}
} Diagnoser;
return VerifyIntegerConstantExpression(E, Result, Diagnoser);
return VerifyIntegerConstantExpression(E, Result, Diagnoser, CanFold);
}
ExprResult Sema::VerifyIntegerConstantExpression(Expr *E,
llvm::APSInt *Result,
unsigned DiagID,
bool AllowFold) {
AllowFoldKind CanFold) {
class IDDiagnoser : public VerifyICEDiagnoser {
unsigned DiagID;
@ -15885,7 +15885,7 @@ ExprResult Sema::VerifyIntegerConstantExpression(Expr *E,
}
} Diagnoser(DiagID);
return VerifyIntegerConstantExpression(E, Result, Diagnoser, AllowFold);
return VerifyIntegerConstantExpression(E, Result, Diagnoser, CanFold);
}
Sema::SemaDiagnosticBuilder
@ -15902,7 +15902,7 @@ Sema::VerifyICEDiagnoser::diagnoseFold(Sema &S, SourceLocation Loc) {
ExprResult
Sema::VerifyIntegerConstantExpression(Expr *E, llvm::APSInt *Result,
VerifyICEDiagnoser &Diagnoser,
bool AllowFold) {
AllowFoldKind CanFold) {
SourceLocation DiagLoc = E->getBeginLoc();
if (getLangOpts().CPlusPlus11) {
@ -16020,7 +16020,7 @@ Sema::VerifyIntegerConstantExpression(Expr *E, llvm::APSInt *Result,
Notes.clear();
}
if (!Folded || !AllowFold) {
if (!Folded || !CanFold) {
if (!Diagnoser.Suppress) {
Diagnoser.diagnoseNotICE(*this, DiagLoc) << E->getSourceRange();
for (const PartialDiagnosticAt &Note : Notes)

16
clang/lib/Sema/SemaExprCXX.cpp
View File

@ -1767,6 +1767,8 @@ Sema::ActOnCXXNew(SourceLocation StartLoc, bool UseGlobal,
DeclaratorChunk::ArrayTypeInfo &Array = D.getTypeObject(I).Arr;
if (Expr *NumElts = (Expr *)Array.NumElts) {
if (!NumElts->isTypeDependent() && !NumElts->isValueDependent()) {
// FIXME: GCC permits constant folding here. We should either do so consistently
// or not do so at all, rather than changing behavior in C++14 onwards.
if (getLangOpts().CPlusPlus14) {
// C++1y [expr.new]p6: Every constant-expression in a noptr-new-declarator
// shall be a converted constant expression (5.19) of type std::size_t
@ -1777,10 +1779,10 @@ Sema::ActOnCXXNew(SourceLocation StartLoc, bool UseGlobal,
CCEK_ArrayBound)
.get();
} else {
Array.NumElts
= VerifyIntegerConstantExpression(NumElts, nullptr,
diag::err_new_array_nonconst)
.get();
Array.NumElts =
VerifyIntegerConstantExpression(
NumElts, nullptr, diag::err_new_array_nonconst, AllowFold)
.get();
}
if (!Array.NumElts)
return ExprError();
@ -5486,9 +5488,9 @@ static uint64_t EvaluateArrayTypeTrait(Sema &Self, ArrayTypeTrait ATT,
case ATT_ArrayExtent: {
llvm::APSInt Value;
uint64_t Dim;
if (Self.VerifyIntegerConstantExpression(DimExpr, &Value,
diag::err_dimension_expr_not_constant_integer,
false).isInvalid())
if (Self.VerifyIntegerConstantExpression(
DimExpr, &Value, diag::err_dimension_expr_not_constant_integer)
.isInvalid())
return 0;
if (Value.isSigned() && Value.isNegative()) {
Self.Diag(KeyLoc, diag::err_dimension_expr_not_constant_integer)

3
clang/lib/Sema/SemaInit.cpp
View File

@ -3130,7 +3130,8 @@ CheckArrayDesignatorExpr(Sema &S, Expr *Index, llvm::APSInt &Value) {
SourceLocation Loc = Index->getBeginLoc();
// Make sure this is an integer constant expression.
ExprResult Result = S.VerifyIntegerConstantExpression(Index, &Value);
ExprResult Result =
S.VerifyIntegerConstantExpression(Index, &Value, Sema::AllowFold);
if (Result.isInvalid())
return Result;

6
clang/lib/Sema/SemaOpenMP.cpp
View File

@ -5836,7 +5836,8 @@ Sema::DeclGroupPtrTy Sema::ActOnOpenMPDeclareSimdDirective(
NewStep = PerformOpenMPImplicitIntegerConversion(Step->getExprLoc(), Step)
.get();
if (NewStep)
NewStep = VerifyIntegerConstantExpression(NewStep).get();
NewStep =
VerifyIntegerConstantExpression(NewStep, /*FIXME*/ AllowFold).get();
}
NewSteps.push_back(NewStep);
}
@ -12812,7 +12813,8 @@ ExprResult Sema::VerifyPositiveIntegerConstantInClause(Expr *E,
E->isInstantiationDependent() || E->containsUnexpandedParameterPack())
return E;
llvm::APSInt Result;
ExprResult ICE = VerifyIntegerConstantExpression(E, &Result);
ExprResult ICE =
VerifyIntegerConstantExpression(E, &Result, /*FIXME*/ AllowFold);
if (ICE.isInvalid())
return ExprError();
if ((StrictlyPositive && !Result.isStrictlyPositive()) ||

2
clang/lib/Sema/SemaStmt.cpp
View File

@ -467,7 +467,7 @@ Sema::ActOnCaseExpr(SourceLocation CaseLoc, ExprResult Val) {
ExprResult ER = E;
if (!E->isValueDependent())
ER = VerifyIntegerConstantExpression(E);
ER = VerifyIntegerConstantExpression(E, AllowFold);
if (!ER.isInvalid())
ER = DefaultLvalueConversion(ER.get());
if (!ER.isInvalid())

3
clang/lib/Sema/SemaTemplate.cpp
View File

@ -7105,8 +7105,7 @@ ExprResult Sema::CheckTemplateArgument(NonTypeTemplateParmDecl *Param,
}
} Diagnoser(ArgType);
Arg = VerifyIntegerConstantExpression(Arg, &Value, Diagnoser,
false).get();
Arg = VerifyIntegerConstantExpression(Arg, &Value, Diagnoser).get();
if (!Arg)
return ExprError();
}

9
clang/lib/Sema/SemaType.cpp
View File

@ -2196,7 +2196,8 @@ QualType Sema::BuildExtIntType(bool IsUnsigned, Expr *BitWidth,
return Context.getDependentExtIntType(IsUnsigned, BitWidth);
llvm::APSInt Bits(32);
ExprResult ICE = VerifyIntegerConstantExpression(BitWidth, &Bits);
ExprResult ICE =
VerifyIntegerConstantExpression(BitWidth, &Bits, /*FIXME*/ AllowFold);
if (ICE.isInvalid())
return QualType();
@ -2272,9 +2273,13 @@ static ExprResult checkArraySize(Sema &S, Expr *&ArraySize,
}
} Diagnoser(VLADiag, VLAIsError);
// FIXME: GCC does *not* allow folding here in general; see PR44406.
// For GCC compatibility, we should remove this folding and leave it to
// TryFixVariablyModifiedType to convert VLAs to constant array types.
ExprResult R = S.VerifyIntegerConstantExpression(
ArraySize, &SizeVal, Diagnoser,
(S.LangOpts.GNUMode || S.LangOpts.OpenCL));
(S.LangOpts.GNUMode || S.LangOpts.OpenCL) ? Sema::AllowFold
: Sema::NoFold);
if (Diagnoser.IsVLA)
return ExprResult();
return R;

10
clang/test/SemaCXX/enum.cpp
View File

@ -107,6 +107,16 @@ enum { overflow = 123456 * 234567 };
// expected-warning@-5 {{overflow in expression; result is -1106067520 with type 'int'}}
#endif
// FIXME: This is not consistent with the above case.
enum NoFold : int { overflow2 = 123456 * 234567 };
#if __cplusplus >= 201103L
// expected-error@-2 {{enumerator value is not a constant expression}}
// expected-note@-3 {{value 28958703552 is outside the range of representable values}}
#else
// expected-warning@-5 {{overflow in expression; result is -1106067520 with type 'int'}}
// expected-warning@-6 {{extension}}
#endif
// PR28903
struct PR28903 {
enum {

11
clang/test/SemaCXX/new-delete.cpp
View File

@ -1,6 +1,7 @@
// RUN: %clang_cc1 -fsyntax-only -verify %s -triple=i686-pc-linux-gnu -Wno-new-returns-null
// RUN: %clang_cc1 -fsyntax-only -verify %s -triple=i686-pc-linux-gnu -Wno-new-returns-null -std=c++98
// RUN: %clang_cc1 -fsyntax-only -verify %s -triple=i686-pc-linux-gnu -Wno-new-returns-null -std=c++11
// RUN: %clang_cc1 -fsyntax-only -verify %s -triple=i686-pc-linux-gnu -Wno-new-returns-null -std=c++14
#include <stddef.h>
@ -621,3 +622,13 @@ int *p0 = dependent_array_size();
int *p3 = dependent_array_size(1, 2, 3);
int *fail = dependent_array_size("hello"); // expected-note {{instantiation of}}
#endif
// FIXME: Our behavior here is incredibly inconsistent. GCC allows
// constant-folding in array bounds in new-expressions.
int (*const_fold)[12] = new int[3][&const_fold + 12 - &const_fold];
#if __cplusplus >= 201402L
// expected-error@-2 {{array size is not a constant expression}}
// expected-note@-3 {{cannot refer to element 12 of non-array}}
#elif __cplusplus < 201103L
// expected-error@-5 {{cannot allocate object of variably modified type}}
#endif