Author: Richard Smith Date: 2020-10-15T16:58:47-07:00 New Revision: fc031d29bea856f2b91a250fd81c5f9fb79dbe07
URL: https://github.com/llvm/llvm-project/commit/fc031d29bea856f2b91a250fd81c5f9fb79dbe07 DIFF: https://github.com/llvm/llvm-project/commit/fc031d29bea856f2b91a250fd81c5f9fb79dbe07.diff LOG: 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). Added: Modified: clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaDecl.cpp clang/lib/Sema/SemaDeclAttr.cpp clang/lib/Sema/SemaDeclCXX.cpp clang/lib/Sema/SemaExceptionSpec.cpp clang/lib/Sema/SemaExpr.cpp clang/lib/Sema/SemaExprCXX.cpp clang/lib/Sema/SemaInit.cpp clang/lib/Sema/SemaOpenMP.cpp clang/lib/Sema/SemaStmt.cpp clang/lib/Sema/SemaTemplate.cpp clang/lib/Sema/SemaType.cpp clang/test/SemaCXX/enum.cpp clang/test/SemaCXX/new-delete.cpp Removed: ################################################################################ diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index b5f0c08300bf..52658715ee8c 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -11608,17 +11608,27 @@ class Sema final { 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. diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 9a6682e837dd..8542c20ec069 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -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()) { diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 0ccfb1504636..91d1e7edb489 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -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; diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 138faa161c4e..cbcaf3cc4360 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -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) { diff --git a/clang/lib/Sema/SemaExceptionSpec.cpp b/clang/lib/Sema/SemaExceptionSpec.cpp index d7695f9d7d7a..851e28741e49 100644 --- a/clang/lib/Sema/SemaExceptionSpec.cpp +++ b/clang/lib/Sema/SemaExceptionSpec.cpp @@ -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; diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 0407d5bb7f6c..7b4e9edbfbf3 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -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) diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp index b96649597274..80b1e90faa0e 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -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) diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index 751b785ce531..fc3ff1412219 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -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; diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp index d0b5a0d03e2d..961a004e95e1 100644 --- a/clang/lib/Sema/SemaOpenMP.cpp +++ b/clang/lib/Sema/SemaOpenMP.cpp @@ -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()) || diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp index 5b4aaa678974..431d592e24ba 100644 --- a/clang/lib/Sema/SemaStmt.cpp +++ b/clang/lib/Sema/SemaStmt.cpp @@ -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()) diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp index 4ecae8faad66..660964c8f92b 100644 --- a/clang/lib/Sema/SemaTemplate.cpp +++ b/clang/lib/Sema/SemaTemplate.cpp @@ -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(); } diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp index b823c962f21d..5eb05bfcaee3 100644 --- a/clang/lib/Sema/SemaType.cpp +++ b/clang/lib/Sema/SemaType.cpp @@ -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; diff --git a/clang/test/SemaCXX/enum.cpp b/clang/test/SemaCXX/enum.cpp index b193a17ebf9e..01bc3a40fc03 100644 --- a/clang/test/SemaCXX/enum.cpp +++ b/clang/test/SemaCXX/enum.cpp @@ -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 { diff --git a/clang/test/SemaCXX/new-delete.cpp b/clang/test/SemaCXX/new-delete.cpp index 64d32f235b59..2fc917ce7488 100644 --- a/clang/test/SemaCXX/new-delete.cpp +++ b/clang/test/SemaCXX/new-delete.cpp @@ -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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits