diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index e6e33e7a9a280..de0055bf243d9 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -319,6 +319,9 @@ Improvements to Clang's diagnostics ----------------------------------- - Diagnostics messages now refer to ``structured binding`` instead of ``decomposition``, to align with `P0615R0 `_ changing the term. (#GH157880) +- Clang now suppresses runtime behavior warnings for unreachable code in file-scope + variable initializers, matching the behavior for functions. This prevents false + positives for operations in unreachable branches of constant expressions. - Added a separate diagnostic group ``-Wfunction-effect-redeclarations``, for the more pedantic diagnostics for function effects (``[[clang::nonblocking]]`` and ``[[clang::nonallocating]]``). Moved the warning for a missing (though implied) attribute on a redeclaration into this group. diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index 0d2316f73fb62..b6d5d92321154 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -5223,11 +5223,7 @@ class Parser : public CodeCompletionHandler { /// assignment-expression /// '{' ... /// \endverbatim - ExprResult ParseInitializer() { - if (Tok.isNot(tok::l_brace)) - return ParseAssignmentExpression(); - return ParseBraceInitializer(); - } + ExprResult ParseInitializer(Decl *DeclForInitializer = nullptr); /// MayBeDesignationStart - Return true if the current token might be the /// start of a designator. If we can tell it is impossible that it is a diff --git a/clang/include/clang/Sema/AnalysisBasedWarnings.h b/clang/include/clang/Sema/AnalysisBasedWarnings.h index 4103c3f006a8f..20a2030f56034 100644 --- a/clang/include/clang/Sema/AnalysisBasedWarnings.h +++ b/clang/include/clang/Sema/AnalysisBasedWarnings.h @@ -14,15 +14,19 @@ #define LLVM_CLANG_SEMA_ANALYSISBASEDWARNINGS_H #include "clang/AST/Decl.h" +#include "clang/Sema/ScopeInfo.h" #include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/MapVector.h" #include namespace clang { +class AnalysisDeclContext; class Decl; class FunctionDecl; class QualType; class Sema; +class VarDecl; namespace sema { class FunctionScopeInfo; class SemaPPCallbacks; @@ -57,6 +61,8 @@ class AnalysisBasedWarnings { enum VisitFlag { NotVisited = 0, Visited = 1, Pending = 2 }; llvm::DenseMap VisitedFD; + std::multimap + VarDeclPossiblyUnreachableDiags; Policy PolicyOverrides; void clearOverrides(); @@ -107,6 +113,10 @@ class AnalysisBasedWarnings { // Issue warnings that require whole-translation-unit analysis. void IssueWarnings(TranslationUnitDecl *D); + void registerVarDeclWarning(VarDecl *VD, PossiblyUnreachableDiag PUD); + + void issueWarningsForRegisteredVarDecl(VarDecl *VD); + // Gets the default policy which is in effect at the given source location. Policy getPolicyInEffectAt(SourceLocation Loc); diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 87b96c2d5ad09..1ee0a499d66ad 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -6765,6 +6765,11 @@ class Sema final : public SemaBase { /// 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. + VarDecl *DeclForInitializer = nullptr; + /// If we are processing a decltype type, a set of call expressions /// for which we have deferred checking the completeness of the return type. SmallVector DelayedDecltypeCalls; diff --git a/clang/lib/Analysis/AnalysisDeclContext.cpp b/clang/lib/Analysis/AnalysisDeclContext.cpp index 5a52056f3e6a5..f188fc6921ed1 100644 --- a/clang/lib/Analysis/AnalysisDeclContext.cpp +++ b/clang/lib/Analysis/AnalysisDeclContext.cpp @@ -117,6 +117,11 @@ Stmt *AnalysisDeclContext::getBody(bool &IsAutosynthesized) const { 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->isFileVarDecl()) { + return const_cast(dyn_cast_or_null(VD->getInit())); + } + } llvm_unreachable("unknown code decl"); } diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp index e4b158e4a6248..15fbf44d5de01 100644 --- a/clang/lib/Parse/ParseDecl.cpp +++ b/clang/lib/Parse/ParseDecl.cpp @@ -2613,7 +2613,7 @@ Decl *Parser::ParseDeclarationAfterDeclaratorAndAttributes( } 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 index b96968d4592f5..d8ed7e3ff96bd 100644 --- a/clang/lib/Parse/ParseDeclCXX.cpp +++ b/clang/lib/Parse/ParseDeclCXX.cpp @@ -3359,7 +3359,7 @@ ExprResult Parser::ParseCXXMemberInitializer(Decl *D, bool IsFunction, 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/ParseInit.cpp b/clang/lib/Parse/ParseInit.cpp index a3be3744a9327..0e86c4c48d5e4 100644 --- a/clang/lib/Parse/ParseInit.cpp +++ b/clang/lib/Parse/ParseInit.cpp @@ -581,3 +581,26 @@ bool Parser::ParseMicrosoftIfExistsBraceInitializer(ExprVector &InitExprs, return !trailingComma; } + +ExprResult Parser::ParseInitializer(Decl *DeclForInitializer) { + // Set DeclForInitializer for file-scope variables. + // For constexpr references, set it to suppress runtime warnings. + // For non-constexpr references, don't set it to avoid evaluation issues + // with self-referencing initializers. Local variables (including local + // constexpr) should emit runtime warnings. + if (DeclForInitializer && !Actions.ExprEvalContexts.empty()) { + if (auto *VD = dyn_cast(DeclForInitializer); + VD && VD->isFileVarDecl() && + (!VD->getType()->isReferenceType() || VD->isConstexpr())) + Actions.ExprEvalContexts.back().DeclForInitializer = VD; + } + + ExprResult init; + if (Tok.isNot(tok::l_brace)) { + init = ParseAssignmentExpression(); + } else { + init = ParseBraceInitializer(); + } + + return init; +} diff --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp index 25199c739ace9..16d32a8b38d03 100644 --- a/clang/lib/Parse/ParseOpenMP.cpp +++ b/clang/lib/Parse/ParseOpenMP.cpp @@ -339,7 +339,7 @@ void Parser::ParseOpenMPReductionInitializerForDecl(VarDecl *OmpPrivParm) { } PreferredType.enterVariableInit(Tok.getLocation(), OmpPrivParm); - 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 index 140b709dbb651..41a98323450e4 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -2734,6 +2734,70 @@ static void flushDiagnostics(Sema &S, const sema::FunctionScopeInfo *fscope) { S.Diag(D.Loc, D.PD); } +template +static void emitPossiblyUnreachableDiags(Sema &S, AnalysisDeclContext &AC, + std::pair PUDs) { + + if (PUDs.first == PUDs.second) + return; + + for (auto I = PUDs.first; I != PUDs.second; ++I) { + for (const Stmt *S : I->Stmts) + AC.registerForcedBlockExpression(S); + } + + if (AC.getCFG()) { + CFGReverseBlockReachabilityAnalysis *Analysis = + AC.getCFGReachablityAnalysis(); + + for (auto I = PUDs.first; I != PUDs.second; ++I) { + const auto &D = *I; + if (llvm::all_of(D.Stmts, [&](const Stmt *St) { + const CFGBlock *Block = AC.getBlockForRegisteredExpression(St); + // 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 && Analysis) + if (!Analysis->isReachable(&AC.getCFG()->getEntry(), Block)) + return false; + return true; + })) { + S.Diag(D.Loc, D.PD); + } + } + } else { + for (auto I = PUDs.first; I != PUDs.second; ++I) + S.Diag(I->Loc, I->PD); + } +} + +void sema::AnalysisBasedWarnings::registerVarDeclWarning( + VarDecl *VD, clang::sema::PossiblyUnreachableDiag PUD) { + VarDeclPossiblyUnreachableDiags.emplace(VD, PUD); +} + +void sema::AnalysisBasedWarnings::issueWarningsForRegisteredVarDecl( + VarDecl *VD) { + if (!llvm::is_contained(VarDeclPossiblyUnreachableDiags, VD)) + return; + + AnalysisDeclContext AC(/*Mgr=*/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; + + auto Range = VarDeclPossiblyUnreachableDiags.equal_range(VD); + auto SecondRange = + llvm::make_second_range(llvm::make_range(Range.first, Range.second)); + emitPossiblyUnreachableDiags( + S, AC, std::make_pair(SecondRange.begin(), SecondRange.end())); +} + // An AST Visitor that calls a callback function on each callable DEFINITION // that is NOT in a dependent context: class CallableVisitor : public DynamicRecursiveASTVisitor { @@ -2945,45 +3009,8 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings( } // 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); - } + auto &PUDs = fscope->PossiblyUnreachableDiags; + emitPossiblyUnreachableDiags(S, AC, std::make_pair(PUDs.begin(), PUDs.end())); // Warning: check missing 'return' if (P.enableCheckFallThrough) { diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index fc3aabf5741ca..7402a8d9e26b0 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -59,6 +59,7 @@ #include "clang/Sema/SemaWasm.h" #include "clang/Sema/Template.h" #include "llvm/ADT/STLForwardCompat.h" +#include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/SmallPtrSet.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/StringExtras.h" @@ -13118,6 +13119,13 @@ namespace { if (isa(OrigDecl)) return; + // Skip checking for file-scope constexpr variables - constant evaluation + // will produce appropriate errors without needing runtime diagnostics. + // Local constexpr should still emit runtime warnings. + if (auto *VD = dyn_cast(OrigDecl); + VD && VD->isConstexpr() && VD->isFileVarDecl()) + return; + E = E->IgnoreParens(); // Skip checking T a = a where T is not a record or reference type. @@ -13745,6 +13753,11 @@ void Sema::DiagnoseUniqueObjectDuplication(const VarDecl *VD) { } void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) { + auto ResetDeclForInitializer = llvm::make_scope_exit([this]() { + if (this->ExprEvalContexts.empty()) + this->ExprEvalContexts.back().DeclForInitializer = nullptr; + }); + // If there is no declaration, there was an error parsing it. Just ignore // the initializer. if (!RealDecl) { @@ -15070,6 +15083,10 @@ void Sema::FinalizeDeclaration(Decl *ThisDecl) { if (!VD) return; + // Emit any deferred warnings for the variable's initializer, even if the + // variable is invalid + AnalysisWarnings.issueWarningsForRegisteredVarDecl(VD); + // Apply an implicit SectionAttr if '#pragma clang section bss|data|rodata' is active if (VD->hasGlobalStorage() && VD->isThisDeclarationADefinition() && !inTemplateInstantiation() && !VD->hasAttr()) { diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index a50c27610dc96..6d828b03fbdbc 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -20567,31 +20567,36 @@ void Sema::MarkDeclarationsReferencedInExpr(Expr *E, } /// Emit a diagnostic when statements are reachable. -/// FIXME: check for reachability even in expressions for which we don't build a -/// CFG (eg, in the initializer of a global or in a constant expression). -/// For example, -/// namespace { auto *p = new double[3][false ? (1, 2) : 3]; } bool Sema::DiagIfReachable(SourceLocation Loc, ArrayRef Stmts, const PartialDiagnostic &PD) { - if (!Stmts.empty() && getCurFunctionOrMethodDecl()) { - if (!FunctionScopes.empty()) - FunctionScopes.back()->PossiblyUnreachableDiags.push_back( - sema::PossiblyUnreachableDiag(PD, Loc, Stmts)); - return true; - } - + VarDecl *Decl = ExprEvalContexts.back().DeclForInitializer; // 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. - if (auto *VD = dyn_cast_or_null( - ExprEvalContexts.back().ManglingContextDecl)) { - if (VD->isConstexpr() || - (VD->isStaticDataMember() && VD->isFirstDecl() && !VD->isInline())) - return false; - // FIXME: For any other kind of variable, we should build a CFG for its - // initializer and check whether the context in question is reachable. + if (Decl && + (Decl->isConstexpr() || (Decl->isStaticDataMember() && + Decl->isFirstDecl() && !Decl->isInline()))) + return false; + + if (Stmts.empty()) { + Diag(Loc, PD); + return true; + } + + if (getCurFunction()) { + FunctionScopes.back()->PossiblyUnreachableDiags.push_back( + sema::PossiblyUnreachableDiag(PD, Loc, Stmts)); + return true; + } + + // For non-constexpr file-scope variables with reachability context (non-empty + // Stmts), build a CFG for the initializer and check whether the context in + // question is reachable. + if (Decl && Decl->isFileVarDecl()) { + AnalysisWarnings.registerVarDeclWarning( + Decl, sema::PossiblyUnreachableDiag(PD, Loc, Stmts)); + return true; } Diag(Loc, PD); diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp index 28925cca8f956..74381e0cb182e 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -6201,6 +6201,10 @@ void Sema::InstantiateVariableInitializer( currentEvaluationContext().RebuildDefaultArgOrDefaultInit = parentEvaluationContext().RebuildDefaultArgOrDefaultInit; + // Set DeclForInitializer for this variable so DiagIfReachable can properly + // suppress runtime diagnostics for constexpr/static member variables + currentEvaluationContext().DeclForInitializer = Var; + if (OldVar->getInit()) { // Instantiate the initializer. ExprResult Init = @@ -6468,6 +6472,8 @@ void Sema::InstantiateVariableDefinition(SourceLocation PointOfInstantiation, PassToConsumerRAII.Var = Var; Var->setTemplateSpecializationKind(OldVar->getTemplateSpecializationKind(), OldVar->getPointOfInstantiation()); + // Emit any deferred warnings for the variable's initializer + AnalysisWarnings.issueWarningsForRegisteredVarDecl(Var); } // This variable may have local implicit instantiations that need to be diff --git a/clang/test/Sema/warn-unreachable-file-scope.c b/clang/test/Sema/warn-unreachable-file-scope.c new file mode 100644 index 0000000000000..64a6918cbcf77 --- /dev/null +++ b/clang/test/Sema/warn-unreachable-file-scope.c @@ -0,0 +1,37 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s + +typedef unsigned char u8; + +u8 a1 = (0 ? 0xffff : 0xff); +u8 a2 = (1 ? 0xffff : 0xff); // expected-warning {{implicit conversion from 'int' to 'u8' (aka 'unsigned char') changes value from 65535 to 255}} +u8 a3 = (1 ? 0xff : 0xffff); +u8 a4 = (0 ? 0xff : 0xffff); // expected-warning {{implicit conversion from 'int' to 'u8' (aka 'unsigned char') changes value from 65535 to 255}} + +unsigned long long b1 = 1 ? 0 : 1ULL << 64; +unsigned long long b2 = 0 ? 0 : 1ULL << 64; // expected-warning {{shift count >= width of type}} +unsigned long long b3 = 1 ? 1ULL << 64 : 0; // expected-warning {{shift count >= width of type}} + +#define M(n) (((n) == 64) ? ~0ULL : ((1ULL << (n)) - 1)) +unsigned long long c1 = M(64); +unsigned long long c2 = M(32); + +static u8 d1 = (0 ? 0xffff : 0xff); +static u8 d2 = (1 ? 0xffff : 0xff); // expected-warning {{implicit conversion from 'int' to 'u8' (aka 'unsigned char') changes value from 65535 to 255}} + +int a = 1 ? 6 : (1,2); +int b = 0 ? 6 : (1,2); // expected-warning {{left operand of comma operator has no effect}} + +void f(void) { + u8 e1 = (0 ? 0xffff : 0xff); + u8 e2 = (1 ? 0xffff : 0xff); // expected-warning {{implicit conversion from 'int' to 'u8' (aka 'unsigned char') changes value from 65535 to 255}} + + unsigned long long e3 = 1 ? 0 : 1ULL << 64; + unsigned long long e4 = 0 ? 0 : 1ULL << 64; // expected-warning {{shift count >= width of type}} +} + +void statics(void) { + static u8 f1 = (0 ? 0xffff : 0xff); + static u8 f2 = (1 ? 0xffff : 0xff); // expected-warning {{implicit conversion from 'int' to 'u8' (aka 'unsigned char') changes value from 65535 to 255}} + static u8 f3 = (1 ? 0xff : 0xffff); + static u8 f4 = (0 ? 0xff : 0xffff); // expected-warning {{implicit conversion from 'int' to 'u8' (aka 'unsigned char') changes value from 65535 to 255}} +}