diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -1869,11 +1869,18 @@ /// initializer: [C99 6.7.8] /// assignment-expression /// '{' ... - ExprResult ParseInitializer() { - if (Tok.isNot(tok::l_brace)) - return ParseAssignmentExpression(); - return ParseBraceInitializer(); + ExprResult ParseInitializer(Decl *DeclForInitializer = nullptr) { + Actions.ExprEvalContexts.back().DeclForInitializer = DeclForInitializer; + ExprResult init; + if (Tok.isNot(tok::l_brace)) { + init = ParseAssignmentExpression(); + } else { + init = ParseBraceInitializer(); + } + Actions.ExprEvalContexts.back().DeclForInitializer = nullptr; + return init; } + bool MayBeDesignationStart(); ExprResult ParseBraceInitializer(); ExprResult ParseInitializerWithPotentialDesignator(); diff --git a/clang/include/clang/Sema/AnalysisBasedWarnings.h b/clang/include/clang/Sema/AnalysisBasedWarnings.h --- a/clang/include/clang/Sema/AnalysisBasedWarnings.h +++ b/clang/include/clang/Sema/AnalysisBasedWarnings.h @@ -13,7 +13,9 @@ #ifndef LLVM_CLANG_SEMA_ANALYSISBASEDWARNINGS_H #define LLVM_CLANG_SEMA_ANALYSISBASEDWARNINGS_H +#include "clang/AST/Decl.h" #include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/MapVector.h" namespace clang { @@ -23,8 +25,10 @@ class ObjCMethodDecl; class QualType; class Sema; +class AnalysisDeclContext; namespace sema { class FunctionScopeInfo; + class PossiblyUnreachableDiag; } namespace sema { @@ -49,6 +53,8 @@ enum VisitFlag { NotVisited = 0, Visited = 1, Pending = 2 }; llvm::DenseMap VisitedFD; + llvm::MapVector> + VarDeclPossiblyUnreachableDiags; /// \name Statistics /// @{ @@ -86,15 +92,25 @@ /// @} + void FlushDiagnostics(SmallVector); + public: AnalysisBasedWarnings(Sema &s); void IssueWarnings(Policy P, FunctionScopeInfo *fscope, const Decl *D, QualType BlockType); + void RegisterVarDeclWarning(VarDecl *VD, PossiblyUnreachableDiag PUD); + + void IssueWarningsForRegisteredVarDecl(VarDecl *VD); + Policy getDefaultPolicy() { return DefaultPolicy; } void PrintStats() const; + + void + EmitPossiblyUnreachableDiags(AnalysisDeclContext &AC, + SmallVector PUDs); }; }} // end namespace clang::sema diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -1039,6 +1039,11 @@ /// suffice, e.g., in a default function argument. Decl *ManglingContextDecl; + /// Declaration for initializer if one is currently being + /// parsed. Used when an expression has a possibly unreachable + /// diagnostic to reference the declaration as a whole. + Decl *DeclForInitializer = nullptr; + /// The context information used to mangle lambda expressions /// and block literals within this context. /// diff --git a/clang/lib/Analysis/AnalysisDeclContext.cpp b/clang/lib/Analysis/AnalysisDeclContext.cpp --- a/clang/lib/Analysis/AnalysisDeclContext.cpp +++ b/clang/lib/Analysis/AnalysisDeclContext.cpp @@ -119,6 +119,11 @@ return BD->getBody(); else if (const auto *FunTmpl = dyn_cast_or_null(D)) return FunTmpl->getTemplatedDecl()->getBody(); + else if (const auto *VD = dyn_cast_or_null(D)) { + if (VD->hasGlobalStorage()) { + return const_cast(dyn_cast(VD->getInit())); + } + } llvm_unreachable("unknown code decl"); } diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp --- a/clang/lib/Parse/ParseDecl.cpp +++ b/clang/lib/Parse/ParseDecl.cpp @@ -2334,7 +2334,7 @@ } PreferredType.enterVariableInit(Tok.getLocation(), ThisDecl); - ExprResult Init = ParseInitializer(); + ExprResult Init = ParseInitializer(ThisDecl); // If this is the only decl in (possibly) range based for statement, // our best guess is that the user meant ':' instead of '='. diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp --- a/clang/lib/Parse/ParseDeclCXX.cpp +++ b/clang/lib/Parse/ParseDeclCXX.cpp @@ -2984,7 +2984,7 @@ Diag(Tok, diag::err_ms_property_initializer) << PD; return ExprError(); } - return ParseInitializer(); + return ParseInitializer(D); } void Parser::SkipCXXMemberSpecification(SourceLocation RecordLoc, diff --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp --- a/clang/lib/Parse/ParseOpenMP.cpp +++ b/clang/lib/Parse/ParseOpenMP.cpp @@ -408,7 +408,7 @@ return; } - ExprResult Init(ParseInitializer()); + ExprResult Init(ParseInitializer(OmpPrivParm)); if (Init.isInvalid()) { SkipUntil(tok::r_paren, tok::annot_pragma_openmp_end, StopBeforeMatch); diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -1964,7 +1964,7 @@ // warnings on a function, method, or block. //===----------------------------------------------------------------------===// -clang::sema::AnalysisBasedWarnings::Policy::Policy() { +sema::AnalysisBasedWarnings::Policy::Policy() { enableCheckFallThrough = 1; enableCheckUnreachable = 0; enableThreadSafetyAnalysis = 0; @@ -1975,17 +1975,12 @@ return (unsigned)!D.isIgnored(diag, SourceLocation()); } -clang::sema::AnalysisBasedWarnings::AnalysisBasedWarnings(Sema &s) - : S(s), - NumFunctionsAnalyzed(0), - NumFunctionsWithBadCFGs(0), - NumCFGBlocks(0), - MaxCFGBlocksPerFunction(0), - NumUninitAnalysisFunctions(0), - NumUninitAnalysisVariables(0), - MaxUninitAnalysisVariablesPerFunction(0), - NumUninitAnalysisBlockVisits(0), - MaxUninitAnalysisBlockVisitsPerFunction(0) { +sema::AnalysisBasedWarnings::AnalysisBasedWarnings(Sema &s) + : S(s), NumFunctionsAnalyzed(0), NumFunctionsWithBadCFGs(0), + NumCFGBlocks(0), MaxCFGBlocksPerFunction(0), + NumUninitAnalysisFunctions(0), NumUninitAnalysisVariables(0), + MaxUninitAnalysisVariablesPerFunction(0), NumUninitAnalysisBlockVisits(0), + MaxUninitAnalysisBlockVisitsPerFunction(0) { using namespace diag; DiagnosticsEngine &D = S.getDiagnostics(); @@ -2003,15 +1998,62 @@ isEnabled(D, warn_use_in_invalid_state); } -static void flushDiagnostics(Sema &S, const sema::FunctionScopeInfo *fscope) { - for (const auto &D : fscope->PossiblyUnreachableDiags) +void sema::AnalysisBasedWarnings::FlushDiagnostics( + const SmallVector PUDs) { + for (const auto &D : PUDs) S.Diag(D.Loc, D.PD); } -void clang::sema:: -AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P, - sema::FunctionScopeInfo *fscope, - const Decl *D, QualType BlockType) { +void sema::AnalysisBasedWarnings::EmitPossiblyUnreachableDiags( + AnalysisDeclContext &AC, + SmallVector PUDs) { + + if (PUDs.empty()) { + return; + } + + bool Analyzed = false; + + // Register the expressions with the CFGBuilder. + for (const auto &D : PUDs) { + for (const Stmt *S : D.Stmts) + AC.registerForcedBlockExpression(S); + } + + if (AC.getCFG()) { + Analyzed = true; + for (const auto &D : PUDs) { + bool AllReachable = true; + for (const Stmt *S : D.Stmts) { + const CFGBlock *block = AC.getBlockForRegisteredExpression(S); + CFGReverseBlockReachabilityAnalysis *cra = + AC.getCFGReachablityAnalysis(); + // FIXME: We should be able to assert that block is non-null, but + // the CFG analysis can skip potentially-evaluated expressions in + // edge cases; see test/Sema/vla-2.c. + if (block && cra) { + // Can this block be reached from the entrance? + if (!cra->isReachable(&AC.getCFG()->getEntry(), block)) { + AllReachable = false; + break; + } + } + // If we cannot map to a basic block, assume the statement is + // reachable. + } + + if (AllReachable) + S.Diag(D.Loc, D.PD); + } + } + + if (!Analyzed) + FlushDiagnostics(PUDs); +} + +void sema::AnalysisBasedWarnings::IssueWarnings( + sema::AnalysisBasedWarnings::Policy P, sema::FunctionScopeInfo *fscope, + const Decl *D, QualType BlockType) { // We avoid doing analysis-based warnings when there are errors for // two reasons: @@ -2033,7 +2075,7 @@ if (Diags.hasUncompilableErrorOccurred()) { // Flush out any possibly unreachable diagnostics. - flushDiagnostics(S, fscope); + FlushDiagnostics(fscope->PossiblyUnreachableDiags); return; } @@ -2085,45 +2127,7 @@ } // Emit delayed diagnostics. - if (!fscope->PossiblyUnreachableDiags.empty()) { - bool analyzed = false; - - // Register the expressions with the CFGBuilder. - for (const auto &D : fscope->PossiblyUnreachableDiags) { - for (const Stmt *S : D.Stmts) - AC.registerForcedBlockExpression(S); - } - - if (AC.getCFG()) { - analyzed = true; - for (const auto &D : fscope->PossiblyUnreachableDiags) { - bool AllReachable = true; - for (const Stmt *S : D.Stmts) { - const CFGBlock *block = AC.getBlockForRegisteredExpression(S); - CFGReverseBlockReachabilityAnalysis *cra = - AC.getCFGReachablityAnalysis(); - // FIXME: We should be able to assert that block is non-null, but - // the CFG analysis can skip potentially-evaluated expressions in - // edge cases; see test/Sema/vla-2.c. - if (block && cra) { - // Can this block be reached from the entrance? - if (!cra->isReachable(&AC.getCFG()->getEntry(), block)) { - AllReachable = false; - break; - } - } - // If we cannot map to a basic block, assume the statement is - // reachable. - } - - if (AllReachable) - S.Diag(D.Loc, D.PD); - } - } - - if (!analyzed) - flushDiagnostics(S, fscope); - } + EmitPossiblyUnreachableDiags(AC, fscope->PossiblyUnreachableDiags); // Warning: check missing 'return' if (P.enableCheckFallThrough) { @@ -2249,7 +2253,27 @@ } } -void clang::sema::AnalysisBasedWarnings::PrintStats() const { +void sema::AnalysisBasedWarnings::RegisterVarDeclWarning( + VarDecl *VD, clang::sema::PossiblyUnreachableDiag PUD) { + VarDeclPossiblyUnreachableDiags[VD].emplace_back(PUD); +} + +void sema::AnalysisBasedWarnings::IssueWarningsForRegisteredVarDecl( + VarDecl *VD) { + AnalysisDeclContext AC(nullptr, VD); + + AC.getCFGBuildOptions().PruneTriviallyFalseEdges = true; + AC.getCFGBuildOptions().AddEHEdges = false; + AC.getCFGBuildOptions().AddInitializers = true; + AC.getCFGBuildOptions().AddImplicitDtors = true; + AC.getCFGBuildOptions().AddTemporaryDtors = true; + AC.getCFGBuildOptions().AddCXXNewAllocator = false; + AC.getCFGBuildOptions().AddCXXDefaultInitExprInCtors = true; + + EmitPossiblyUnreachableDiags(AC, VarDeclPossiblyUnreachableDiags[VD]); +} + +void sema::AnalysisBasedWarnings::PrintStats() const { llvm::errs() << "\n*** Analysis Based Warnings Stats:\n"; unsigned NumCFGsBuilt = NumFunctionsAnalyzed - NumFunctionsWithBadCFGs; diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -31,6 +31,7 @@ #include "clang/Lex/Lexer.h" // TODO: Extract static functions to fix layering. #include "clang/Lex/ModuleLoader.h" // TODO: Sema shouldn't depend on Lex #include "clang/Lex/Preprocessor.h" // Included for isCodeCompletionEnabled() +#include "clang/Sema/AnalysisBasedWarnings.h" #include "clang/Sema/CXXFieldCollector.h" #include "clang/Sema/DeclSpec.h" #include "clang/Sema/DelayedDiagnostic.h" @@ -11849,6 +11850,8 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl *var) { if (var->isInvalidDecl()) return; + AnalysisWarnings.IssueWarningsForRegisteredVarDecl(var); + if (getLangOpts().OpenCL) { // OpenCL v2.0 s6.12.5 - Every block variable declaration must have an // initialiser diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -288,6 +288,9 @@ UnparsedDefaultArgInstantiations.erase(InstPos); } + // Check for delayed warnings + AnalysisWarnings.IssueWarningsForRegisteredVarDecl(Param); + return false; } diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -4881,6 +4881,8 @@ "default argument expression has capturing blocks?"); } + AnalysisWarnings.IssueWarningsForRegisteredVarDecl(Param); + // We already type-checked the argument, so we know it works. // Just mark all of the declarations in this potentially-evaluated expression // as being "referenced". @@ -16666,23 +16668,31 @@ case ExpressionEvaluationContext::PotentiallyEvaluated: case ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed: if (!Stmts.empty() && getCurFunctionOrMethodDecl()) { - FunctionScopes.back()->PossiblyUnreachableDiags. - push_back(sema::PossiblyUnreachableDiag(PD, Loc, Stmts)); + FunctionScopes.back()->PossiblyUnreachableDiags.push_back( + PossiblyUnreachableDiag(PD, Loc, Stmts)); return true; } - // The initializer of a constexpr variable or of the first declaration of a - // static data member is not syntactically a constant evaluated constant, - // but nonetheless is always required to be a constant expression, so we - // can skip diagnosing. - // FIXME: Using the mangling context here is a hack. + // For any other kind of variable, we should build a CFG for its + // initializer and check whether the context in question is reachable. if (auto *VD = dyn_cast_or_null( - ExprEvalContexts.back().ManglingContextDecl)) { + ExprEvalContexts.back().DeclForInitializer)) { + if (VD->getDefinition()) { + VD = VD->getDefinition(); + } + // The initializer of a constexpr variable or of the first declaration of + // a static data member is not syntactically a constant evaluated + // constant, but nonetheless is always required to be a constant + // expression, so we can skip diagnosing. if (VD->isConstexpr() || (VD->isStaticDataMember() && VD->isFirstDecl() && !VD->isInline())) break; - // FIXME: For any other kind of variable, we should build a CFG for its - // initializer and check whether the context in question is reachable. + // FIXME: Some cases aren't picked up by path analysis currently + if (!Stmts.empty() && VD->isFileVarDecl()) { + AnalysisWarnings.RegisterVarDeclWarning( + VD, PossiblyUnreachableDiag(PD, Loc, Stmts)); + return true; + } } Diag(Loc, PD); diff --git a/clang/test/Sema/warn-unreachable-warning-var-decl.cpp b/clang/test/Sema/warn-unreachable-warning-var-decl.cpp new file mode 100644 --- /dev/null +++ b/clang/test/Sema/warn-unreachable-warning-var-decl.cpp @@ -0,0 +1,40 @@ +// RUN: %clang_cc1 -verify %s +int e = 1 ? 0 : 1 / 0; +int g = 1 ? 1 / 0 : 0; // expected-warning{{division by zero is undefined}} + +#define SHIFT(n) (((n) == 32) ? 0 : ((1 << (n)) - 1)) + +int x = SHIFT(32); +int y = SHIFT(0); + +// FIXME: Expressions in lambdas aren't ignored +int z = []() { + return 1 ? 0 : 1 / 0; // expected-warning{{division by zero is undefined}} +}(); + +int f(void) { + int x = 1 ? 0 : 1 / 0; + return x; +} + +template +struct X0 { + static T value; +}; + +template +struct X1 { + static T value; +}; + +template +T X0::value = 3.14; // expected-warning{{implicit conversion from 'double' to 'int' changes value from 3.14 to 3}} + +template +T X1::value = 1 ? 0 : 1 / 0; + +template struct X0; // expected-note{{in instantiation of static data member 'X0::value' requested here}} + +constexpr signed char c1 = 100 * 2; // expected-warning{{changes value}} +constexpr signed char c2 = '\x64' * '\2'; // expected-warning{{changes value}} +constexpr int shr_32 = 0 >> 32; // expected-error {{constant expression}} expected-note {{shift count 32 >= width of type}} diff --git a/clang/test/SemaCXX/constant-expression-cxx11.cpp b/clang/test/SemaCXX/constant-expression-cxx11.cpp --- a/clang/test/SemaCXX/constant-expression-cxx11.cpp +++ b/clang/test/SemaCXX/constant-expression-cxx11.cpp @@ -383,7 +383,7 @@ constexpr B b3 { { 1 }, { 2 } }; // expected-error {{constant expression}} expected-note {{reference to temporary}} expected-note {{here}} } -constexpr B &&b4 = ((1, 2), 3, 4, B { {10}, {{20}} }); +constexpr B &&b4 = ((1, 2), 3, 4, B{{10}, {{20}}}); //expected-warning {{expression result unused}} static_assert(&b4 != &b2, ""); // Proposed DR: copy-elision doesn't trigger lifetime extension. diff --git a/clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp b/clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp --- a/clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp +++ b/clang/test/SemaCXX/constexpr-builtin-bit-cast.cpp @@ -357,8 +357,8 @@ int b; }; -constexpr int ok_byte = (__builtin_bit_cast(std::byte[8], pad{1, 2}), 0); -constexpr int ok_uchar = (__builtin_bit_cast(unsigned char[8], pad{1, 2}), 0); +constexpr int ok_byte = (__builtin_bit_cast(std::byte[8], pad{1, 2}), 0); // expected-warning {{expression result unused}} +constexpr int ok_uchar = (__builtin_bit_cast(unsigned char[8], pad{1, 2}), 0); // expected-warning {{expression result unused}} #ifdef __CHAR_UNSIGNED__ // expected-note@+5 {{indeterminate value can only initialize an object of type 'unsigned char', 'char', or 'std::byte'; 'my_byte' is invalid}}}} @@ -366,12 +366,12 @@ // expected-note@+3 {{indeterminate value can only initialize an object of type 'unsigned char' or 'std::byte'; 'my_byte' is invalid}} #endif // expected-error@+1 {{constexpr variable 'bad_my_byte' must be initialized by a constant expression}} -constexpr int bad_my_byte = (__builtin_bit_cast(my_byte[8], pad{1, 2}), 0); +constexpr int bad_my_byte = (__builtin_bit_cast(my_byte[8], pad{1, 2}), 0); // expected-warning {{expression result unused}} #ifndef __CHAR_UNSIGNED__ // expected-error@+3 {{constexpr variable 'bad_char' must be initialized by a constant expression}} // expected-note@+2 {{indeterminate value can only initialize an object of type 'unsigned char' or 'std::byte'; 'char' is invalid}} #endif -constexpr int bad_char = (__builtin_bit_cast(char[8], pad{1, 2}), 0); +constexpr int bad_char = (__builtin_bit_cast(char[8], pad{1, 2}), 0); // expected-warning {{expression result unused}} struct pad_buffer { unsigned char data[sizeof(pad)]; }; constexpr bool test_pad_buffer() { diff --git a/clang/test/SemaTemplate/instantiate-static-var.cpp b/clang/test/SemaTemplate/instantiate-static-var.cpp --- a/clang/test/SemaTemplate/instantiate-static-var.cpp +++ b/clang/test/SemaTemplate/instantiate-static-var.cpp @@ -6,6 +6,7 @@ class X { public: static const T value = 10 / Divisor; // expected-error{{in-class initializer for static data member is not a constant expression}} + //expected-warning@-1 {{division by zero is undefined}} }; int array1[X::value == 5? 1 : -1];