https://github.com/JustinStitt updated https://github.com/llvm/llvm-project/pull/163885
>From f49f6b0210e2fd5368516f733679520704995330 Mon Sep 17 00:00:00 2001 From: Justin Stitt <[email protected]> Date: Mon, 13 Oct 2025 14:11:00 -0700 Subject: [PATCH 1/7] [Clang] Consider reachability for file-scope warnings on initializers Suppress runtime warnings for unreachable code in global variable initializers while maintaining warnings for the same code in functions. For context, some global variable declarations using ternaries can emit warnings during initialization even when a particular expression is never used. This behavior differs from GCC since they properly emit warnings based on reachability -- even at file scope. The simplest example is: ```c $ cat test.c unsigned long long b1 = 1 ? 0 : 1ULL << 64; $ clang-21 -fsyntax-only test.c test.c:1:39: warning: shift count >= width of type [-Wshift-count-overflow] 1 | unsigned long long foo = 1 ? 0 : 1ULL << 64; | ^ ~~ $ gcc-13 -fsyntax-only test.c <empty> ``` Clang previously emitted a ``-Wshift-count-overflow`` warning regardless of branch taken. This PR constructs a CFG and defers runtime diagnostic emission until we can analyze the CFG for reachability. To be clear, the intention is only to modify initializers in global scope and only remove warnings if unreachable, no new warnings should pop up anywhere. Link: https://reviews.llvm.org/D63889 (original PR by Nathan Huckleberry) Link: https://github.com/clangBuiltLinux/linux/issues/92 Co-Authored-By: Nathan Huckleberry <[email protected]> Signed-off-by: Justin Stitt <[email protected]> --- clang/include/clang/Parse/Parser.h | 29 ++++- .../clang/Sema/AnalysisBasedWarnings.h | 16 +++ clang/include/clang/Sema/Sema.h | 5 + clang/lib/Analysis/AnalysisDeclContext.cpp | 5 + clang/lib/Parse/ParseDecl.cpp | 2 +- clang/lib/Parse/ParseDeclCXX.cpp | 2 +- clang/lib/Parse/ParseOpenMP.cpp | 2 +- clang/lib/Sema/AnalysisBasedWarnings.cpp | 114 ++++++++++++------ clang/lib/Sema/SemaDecl.cpp | 21 ++++ clang/lib/Sema/SemaExpr.cpp | 35 ++++-- .../lib/Sema/SemaTemplateInstantiateDecl.cpp | 6 + clang/test/Sema/warn-unreachable-file-scope.c | 30 +++++ 12 files changed, 210 insertions(+), 57 deletions(-) create mode 100644 clang/test/Sema/warn-unreachable-file-scope.c diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index 0d2316f73fb62..b2e06287b32ef 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -5223,10 +5223,31 @@ class Parser : public CodeCompletionHandler { /// assignment-expression /// '{' ... /// \endverbatim - ExprResult ParseInitializer() { - if (Tok.isNot(tok::l_brace)) - return ParseAssignmentExpression(); - return ParseBraceInitializer(); + ExprResult ParseInitializer(Decl *DeclForInitializer = nullptr) { + // 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<VarDecl>(DeclForInitializer)) { + if (VD->isFileVarDecl()) { + if (!VD->getType()->isReferenceType() || VD->isConstexpr()) { + Actions.ExprEvalContexts.back().DeclForInitializer = + DeclForInitializer; + } + } + } + } + + ExprResult init; + if (Tok.isNot(tok::l_brace)) { + init = ParseAssignmentExpression(); + } else { + init = ParseBraceInitializer(); + } + + return init; } /// MayBeDesignationStart - Return true if the current token might be the diff --git a/clang/include/clang/Sema/AnalysisBasedWarnings.h b/clang/include/clang/Sema/AnalysisBasedWarnings.h index 4103c3f006a8f..1f82c764e31be 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 <memory> namespace clang { +class AnalysisDeclContext; class Decl; class FunctionDecl; class QualType; class Sema; +class VarDecl; namespace sema { class FunctionScopeInfo; class SemaPPCallbacks; @@ -57,10 +61,14 @@ class AnalysisBasedWarnings { enum VisitFlag { NotVisited = 0, Visited = 1, Pending = 2 }; llvm::DenseMap<const FunctionDecl*, VisitFlag> VisitedFD; + llvm::MapVector<VarDecl *, SmallVector<PossiblyUnreachableDiag, 4>> + VarDeclPossiblyUnreachableDiags; Policy PolicyOverrides; void clearOverrides(); + void FlushDiagnostics(SmallVector<clang::sema::PossiblyUnreachableDiag, 4>); + /// \name Statistics /// @{ @@ -107,6 +115,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); @@ -116,6 +128,10 @@ class AnalysisBasedWarnings { Policy &getPolicyOverrides() { return PolicyOverrides; } void PrintStats() const; + + void + EmitPossiblyUnreachableDiags(AnalysisDeclContext &AC, + SmallVector<PossiblyUnreachableDiag, 4> PUDs); }; } // namespace sema diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 87b96c2d5ad09..8952fd8de4c5f 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. + Decl *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<CallExpr *, 8> DelayedDecltypeCalls; diff --git a/clang/lib/Analysis/AnalysisDeclContext.cpp b/clang/lib/Analysis/AnalysisDeclContext.cpp index 5a52056f3e6a5..d118d81b923bf 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<FunctionTemplateDecl>(D)) return FunTmpl->getTemplatedDecl()->getBody(); + else if (const auto *VD = dyn_cast_or_null<VarDecl>(D)) { + if (VD->hasGlobalStorage()) { + return const_cast<Stmt *>(dyn_cast_or_null<Stmt>(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/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..6df92d44f47fe 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -2734,6 +2734,80 @@ static void flushDiagnostics(Sema &S, const sema::FunctionScopeInfo *fscope) { S.Diag(D.Loc, D.PD); } +void sema::AnalysisBasedWarnings::FlushDiagnostics( + const SmallVector<clang::sema::PossiblyUnreachableDiag, 4> PUDs) { + for (const auto &D : PUDs) + S.Diag(D.Loc, D.PD); +} + +void sema::AnalysisBasedWarnings::EmitPossiblyUnreachableDiags( + AnalysisDeclContext &AC, + SmallVector<clang::sema::PossiblyUnreachableDiag, 4> PUDs) { + + if (PUDs.empty()) { + return; + } + + bool Analyzed = false; + + 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 *St : D.Stmts) { + const CFGBlock *block = AC.getBlockForRegisteredExpression(St); + CFGReverseBlockReachabilityAnalysis *cra = + AC.getCFGReachablityAnalysis(); + 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::RegisterVarDeclWarning( + VarDecl *VD, clang::sema::PossiblyUnreachableDiag PUD) { + VarDeclPossiblyUnreachableDiags[VD].emplace_back(PUD); +} + +void sema::AnalysisBasedWarnings::IssueWarningsForRegisteredVarDecl( + VarDecl *VD) { + if (VarDeclPossiblyUnreachableDiags.find(VD) == + VarDeclPossiblyUnreachableDiags.end()) { + return; + } + + 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]); +} + // 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 +3019,7 @@ 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); - } + EmitPossiblyUnreachableDiags(AC, fscope->PossiblyUnreachableDiags); // Warning: check missing 'return' if (P.enableCheckFallThrough) { diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index fc3aabf5741ca..43ad2dcecb565 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -13118,6 +13118,13 @@ namespace { if (isa<ParmVarDecl>(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<VarDecl>(OrigDecl)) + if (VD->isConstexpr() && VD->isFileVarDecl()) + return; + E = E->IgnoreParens(); // Skip checking T a = a where T is not a record or reference type. @@ -13745,6 +13752,16 @@ void Sema::DiagnoseUniqueObjectDuplication(const VarDecl *VD) { } void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) { + // RAII helper to ensure DeclForInitializer is cleared on all exit paths + struct ClearDeclForInitializer { + Sema &S; + ClearDeclForInitializer(Sema &S) : S(S) {} + ~ClearDeclForInitializer() { + if (!S.ExprEvalContexts.empty()) + S.ExprEvalContexts.back().DeclForInitializer = nullptr; + } + } Clearer(*this); + // If there is no declaration, there was an error parsing it. Just ignore // the initializer. if (!RealDecl) { @@ -15070,6 +15087,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<SectionAttr>()) { diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index a50c27610dc96..68413287be756 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -20573,25 +20573,38 @@ void Sema::MarkDeclarationsReferencedInExpr(Expr *E, /// namespace { auto *p = new double[3][false ? (1, 2) : 3]; } bool Sema::DiagIfReachable(SourceLocation Loc, ArrayRef<const Stmt *> Stmts, const PartialDiagnostic &PD) { - if (!Stmts.empty() && getCurFunctionOrMethodDecl()) { - if (!FunctionScopes.empty()) - FunctionScopes.back()->PossiblyUnreachableDiags.push_back( - sema::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. if (auto *VD = dyn_cast_or_null<VarDecl>( - ExprEvalContexts.back().ManglingContextDecl)) { + ExprEvalContexts.back().DeclForInitializer)) { 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 (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 (auto *VD = dyn_cast_or_null<VarDecl>( + ExprEvalContexts.back().DeclForInitializer)) { + if (VD->isFileVarDecl()) { + AnalysisWarnings.RegisterVarDeclWarning( + VD, 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..af630b82bde60 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..e6b20bb592f10 --- /dev/null +++ b/clang/test/Sema/warn-unreachable-file-scope.c @@ -0,0 +1,30 @@ +// 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}} +} >From 729aa96ab45a6bb32ee977fc2ca3d454ecd2d3bc Mon Sep 17 00:00:00 2001 From: Justin Stitt <[email protected]> Date: Thu, 16 Oct 2025 19:19:27 -0700 Subject: [PATCH 2/7] remove fixme Signed-off-by: Justin Stitt <[email protected]> --- clang/lib/Sema/SemaExpr.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 68413287be756..14e692f4449aa 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -20567,10 +20567,6 @@ 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<const Stmt *> Stmts, const PartialDiagnostic &PD) { // The initializer of a constexpr variable or of the first declaration of a >From dc22756032b092dc7462db315cafbb08e52887c0 Mon Sep 17 00:00:00 2001 From: Justin Stitt <[email protected]> Date: Fri, 17 Oct 2025 11:07:31 -0700 Subject: [PATCH 3/7] various fixes and refactors based on review from gh@zwuis - add release note - move Parser::ParseInitializer impl to a source file - remove FlushDiagnostics, favoring open-coded version - DeclForInitializer is now a VarDecl in Sema.h - use certain llvm helpers like all_of and is_contained Signed-off-by: Justin Stitt <[email protected]> --- clang/docs/ReleaseNotes.rst | 3 ++ clang/include/clang/Parse/Parser.h | 27 +--------- .../clang/Sema/AnalysisBasedWarnings.h | 2 - clang/include/clang/Sema/Sema.h | 2 +- clang/lib/Analysis/AnalysisDeclContext.cpp | 2 +- clang/lib/Parse/ParseInit.cpp | 25 +++++++++ clang/lib/Sema/AnalysisBasedWarnings.cpp | 51 ++++++------------- clang/lib/Sema/SemaDecl.cpp | 6 +-- clang/test/Sema/warn-unreachable-file-scope.c | 7 +++ 9 files changed, 57 insertions(+), 68 deletions(-) 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 <https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0615r0.html>`_ 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 b2e06287b32ef..b6d5d92321154 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -5223,32 +5223,7 @@ class Parser : public CodeCompletionHandler { /// assignment-expression /// '{' ... /// \endverbatim - ExprResult ParseInitializer(Decl *DeclForInitializer = nullptr) { - // 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<VarDecl>(DeclForInitializer)) { - if (VD->isFileVarDecl()) { - if (!VD->getType()->isReferenceType() || VD->isConstexpr()) { - Actions.ExprEvalContexts.back().DeclForInitializer = - DeclForInitializer; - } - } - } - } - - ExprResult init; - if (Tok.isNot(tok::l_brace)) { - init = ParseAssignmentExpression(); - } else { - init = ParseBraceInitializer(); - } - - return init; - } + 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 1f82c764e31be..af61afba357f8 100644 --- a/clang/include/clang/Sema/AnalysisBasedWarnings.h +++ b/clang/include/clang/Sema/AnalysisBasedWarnings.h @@ -67,8 +67,6 @@ class AnalysisBasedWarnings { Policy PolicyOverrides; void clearOverrides(); - void FlushDiagnostics(SmallVector<clang::sema::PossiblyUnreachableDiag, 4>); - /// \name Statistics /// @{ diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 8952fd8de4c5f..1ee0a499d66ad 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -6768,7 +6768,7 @@ class Sema final : public SemaBase { /// 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; + 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. diff --git a/clang/lib/Analysis/AnalysisDeclContext.cpp b/clang/lib/Analysis/AnalysisDeclContext.cpp index d118d81b923bf..f188fc6921ed1 100644 --- a/clang/lib/Analysis/AnalysisDeclContext.cpp +++ b/clang/lib/Analysis/AnalysisDeclContext.cpp @@ -118,7 +118,7 @@ Stmt *AnalysisDeclContext::getBody(bool &IsAutosynthesized) const { else if (const auto *FunTmpl = dyn_cast_or_null<FunctionTemplateDecl>(D)) return FunTmpl->getTemplatedDecl()->getBody(); else if (const auto *VD = dyn_cast_or_null<VarDecl>(D)) { - if (VD->hasGlobalStorage()) { + if (VD->isFileVarDecl()) { return const_cast<Stmt *>(dyn_cast_or_null<Stmt>(VD->getInit())); } } diff --git a/clang/lib/Parse/ParseInit.cpp b/clang/lib/Parse/ParseInit.cpp index a3be3744a9327..3a17d622b2550 100644 --- a/clang/lib/Parse/ParseInit.cpp +++ b/clang/lib/Parse/ParseInit.cpp @@ -581,3 +581,28 @@ 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<VarDecl>(DeclForInitializer); + VD && VD->isFileVarDecl()) { + if (!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/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 6df92d44f47fe..0f68a483be1ff 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -2734,21 +2734,12 @@ static void flushDiagnostics(Sema &S, const sema::FunctionScopeInfo *fscope) { S.Diag(D.Loc, D.PD); } -void sema::AnalysisBasedWarnings::FlushDiagnostics( - const SmallVector<clang::sema::PossiblyUnreachableDiag, 4> PUDs) { - for (const auto &D : PUDs) - S.Diag(D.Loc, D.PD); -} - void sema::AnalysisBasedWarnings::EmitPossiblyUnreachableDiags( AnalysisDeclContext &AC, SmallVector<clang::sema::PossiblyUnreachableDiag, 4> PUDs) { - if (PUDs.empty()) { + if (PUDs.empty()) return; - } - - bool Analyzed = false; for (const auto &D : PUDs) { for (const Stmt *S : D.Stmts) @@ -2756,31 +2747,23 @@ void sema::AnalysisBasedWarnings::EmitPossiblyUnreachableDiags( } if (AC.getCFG()) { - Analyzed = true; for (const auto &D : PUDs) { - bool AllReachable = true; - for (const Stmt *St : D.Stmts) { - const CFGBlock *block = AC.getBlockForRegisteredExpression(St); - CFGReverseBlockReachabilityAnalysis *cra = - AC.getCFGReachablityAnalysis(); - 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) + if (llvm::all_of(D.Stmts, [&](const Stmt *St) { + const CFGBlock *Block = AC.getBlockForRegisteredExpression(St); + CFGReverseBlockReachabilityAnalysis *Analysis = + AC.getCFGReachablityAnalysis(); + if (Block && Analysis) + if (!Analysis->isReachable(&AC.getCFG()->getEntry(), Block)) + return false; + return true; + })) { S.Diag(D.Loc, D.PD); + } } + } else { + for (const auto &D : PUDs) + S.Diag(D.Loc, D.PD); } - - if (!Analyzed) - FlushDiagnostics(PUDs); } void sema::AnalysisBasedWarnings::RegisterVarDeclWarning( @@ -2790,12 +2773,10 @@ void sema::AnalysisBasedWarnings::RegisterVarDeclWarning( void sema::AnalysisBasedWarnings::IssueWarningsForRegisteredVarDecl( VarDecl *VD) { - if (VarDeclPossiblyUnreachableDiags.find(VD) == - VarDeclPossiblyUnreachableDiags.end()) { + if (!llvm::is_contained(VarDeclPossiblyUnreachableDiags, VD)) return; - } - AnalysisDeclContext AC(nullptr, VD); + AnalysisDeclContext AC(/*Mgr=*/nullptr, VD); AC.getCFGBuildOptions().PruneTriviallyFalseEdges = true; AC.getCFGBuildOptions().AddEHEdges = false; diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 43ad2dcecb565..9f8a44a9b125f 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -13121,9 +13121,9 @@ namespace { // 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<VarDecl>(OrigDecl)) - if (VD->isConstexpr() && VD->isFileVarDecl()) - return; + if (auto *VD = dyn_cast<VarDecl>(OrigDecl); + VD && VD->isConstexpr() && VD->isFileVarDecl()) + return; E = E->IgnoreParens(); diff --git a/clang/test/Sema/warn-unreachable-file-scope.c b/clang/test/Sema/warn-unreachable-file-scope.c index e6b20bb592f10..64a6918cbcf77 100644 --- a/clang/test/Sema/warn-unreachable-file-scope.c +++ b/clang/test/Sema/warn-unreachable-file-scope.c @@ -28,3 +28,10 @@ void f(void) { 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}} +} >From 62161771a561f4935f238e6e2cc7b81bb88bcde1 Mon Sep 17 00:00:00 2001 From: Justin Stitt <[email protected]> Date: Fri, 17 Oct 2025 14:22:02 -0700 Subject: [PATCH 4/7] dont recompute Analysis in llvm::all_of Signed-off-by: Justin Stitt <[email protected]> --- clang/lib/Sema/AnalysisBasedWarnings.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 0f68a483be1ff..b30ee43d2a801 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -2747,11 +2747,12 @@ void sema::AnalysisBasedWarnings::EmitPossiblyUnreachableDiags( } if (AC.getCFG()) { + CFGReverseBlockReachabilityAnalysis *Analysis = + AC.getCFGReachablityAnalysis(); + for (const auto &D : PUDs) { if (llvm::all_of(D.Stmts, [&](const Stmt *St) { const CFGBlock *Block = AC.getBlockForRegisteredExpression(St); - CFGReverseBlockReachabilityAnalysis *Analysis = - AC.getCFGReachablityAnalysis(); if (Block && Analysis) if (!Analysis->isReachable(&AC.getCFG()->getEntry(), Block)) return false; >From 6cb036236ead1582d9bb046faf3e5a35d70a5b5e Mon Sep 17 00:00:00 2001 From: Justin Stitt <[email protected]> Date: Mon, 20 Oct 2025 10:44:30 -0700 Subject: [PATCH 5/7] use multimap over MapVector Signed-off-by: Justin Stitt <[email protected]> --- .../clang/Sema/AnalysisBasedWarnings.h | 8 +++--- clang/lib/Sema/AnalysisBasedWarnings.cpp | 28 +++++++++++-------- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/clang/include/clang/Sema/AnalysisBasedWarnings.h b/clang/include/clang/Sema/AnalysisBasedWarnings.h index af61afba357f8..415d82aab04f4 100644 --- a/clang/include/clang/Sema/AnalysisBasedWarnings.h +++ b/clang/include/clang/Sema/AnalysisBasedWarnings.h @@ -61,7 +61,7 @@ class AnalysisBasedWarnings { enum VisitFlag { NotVisited = 0, Visited = 1, Pending = 2 }; llvm::DenseMap<const FunctionDecl*, VisitFlag> VisitedFD; - llvm::MapVector<VarDecl *, SmallVector<PossiblyUnreachableDiag, 4>> + std::multimap<VarDecl *, PossiblyUnreachableDiag> VarDeclPossiblyUnreachableDiags; Policy PolicyOverrides; @@ -127,9 +127,9 @@ class AnalysisBasedWarnings { void PrintStats() const; - void - EmitPossiblyUnreachableDiags(AnalysisDeclContext &AC, - SmallVector<PossiblyUnreachableDiag, 4> PUDs); + template <typename Iterator> + void EmitPossiblyUnreachableDiags(AnalysisDeclContext &AC, + std::pair<Iterator, Iterator> PUDs); }; } // namespace sema diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index b30ee43d2a801..98b9b241442f2 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -2734,15 +2734,15 @@ static void flushDiagnostics(Sema &S, const sema::FunctionScopeInfo *fscope) { S.Diag(D.Loc, D.PD); } +template <typename Iterator> void sema::AnalysisBasedWarnings::EmitPossiblyUnreachableDiags( - AnalysisDeclContext &AC, - SmallVector<clang::sema::PossiblyUnreachableDiag, 4> PUDs) { + AnalysisDeclContext &AC, std::pair<Iterator, Iterator> PUDs) { - if (PUDs.empty()) + if (PUDs.first == PUDs.second) return; - for (const auto &D : PUDs) { - for (const Stmt *S : D.Stmts) + for (auto I = PUDs.first; I != PUDs.second; ++I) { + for (const Stmt *S : I->Stmts) AC.registerForcedBlockExpression(S); } @@ -2750,7 +2750,8 @@ void sema::AnalysisBasedWarnings::EmitPossiblyUnreachableDiags( CFGReverseBlockReachabilityAnalysis *Analysis = AC.getCFGReachablityAnalysis(); - for (const auto &D : PUDs) { + 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); if (Block && Analysis) @@ -2762,14 +2763,14 @@ void sema::AnalysisBasedWarnings::EmitPossiblyUnreachableDiags( } } } else { - for (const auto &D : PUDs) - S.Diag(D.Loc, D.PD); + 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[VD].emplace_back(PUD); + VarDeclPossiblyUnreachableDiags.emplace(VD, PUD); } void sema::AnalysisBasedWarnings::IssueWarningsForRegisteredVarDecl( @@ -2787,7 +2788,11 @@ void sema::AnalysisBasedWarnings::IssueWarningsForRegisteredVarDecl( AC.getCFGBuildOptions().AddCXXNewAllocator = false; AC.getCFGBuildOptions().AddCXXDefaultInitExprInCtors = true; - EmitPossiblyUnreachableDiags(AC, VarDeclPossiblyUnreachableDiags[VD]); + auto Range = VarDeclPossiblyUnreachableDiags.equal_range(VD); + auto SecondRange = + llvm::make_second_range(llvm::make_range(Range.first, Range.second)); + EmitPossiblyUnreachableDiags( + AC, std::make_pair(SecondRange.begin(), SecondRange.end())); } // An AST Visitor that calls a callback function on each callable DEFINITION @@ -3001,7 +3006,8 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings( } // Emit delayed diagnostics. - EmitPossiblyUnreachableDiags(AC, fscope->PossiblyUnreachableDiags); + auto &PUDs = fscope->PossiblyUnreachableDiags; + EmitPossiblyUnreachableDiags(AC, std::make_pair(PUDs.begin(), PUDs.end())); // Warning: check missing 'return' if (P.enableCheckFallThrough) { >From 037fc5811efda0d5617a558302d62061426479e7 Mon Sep 17 00:00:00 2001 From: Justin Stitt <[email protected]> Date: Mon, 20 Oct 2025 11:54:31 -0700 Subject: [PATCH 6/7] various changes from review based on review from gh@zwuis and gh@cor3ntin - collapse if into parent if - use make_scope_exit over hand-rolled RAII obj - avoid casting a DeclForInitializer back to VarDecl since it already is one - make a few functions correctly formatted with lowercase - make EmitPossiblyUnreachableDiags static --- .../clang/Sema/AnalysisBasedWarnings.h | 8 ++----- clang/lib/Parse/ParseInit.cpp | 8 +++---- clang/lib/Sema/AnalysisBasedWarnings.cpp | 14 ++++++------ clang/lib/Sema/SemaDecl.cpp | 16 +++++--------- clang/lib/Sema/SemaExpr.cpp | 22 ++++++++----------- .../lib/Sema/SemaTemplateInstantiateDecl.cpp | 2 +- 6 files changed, 28 insertions(+), 42 deletions(-) diff --git a/clang/include/clang/Sema/AnalysisBasedWarnings.h b/clang/include/clang/Sema/AnalysisBasedWarnings.h index 415d82aab04f4..20a2030f56034 100644 --- a/clang/include/clang/Sema/AnalysisBasedWarnings.h +++ b/clang/include/clang/Sema/AnalysisBasedWarnings.h @@ -113,9 +113,9 @@ class AnalysisBasedWarnings { // Issue warnings that require whole-translation-unit analysis. void IssueWarnings(TranslationUnitDecl *D); - void RegisterVarDeclWarning(VarDecl *VD, PossiblyUnreachableDiag PUD); + void registerVarDeclWarning(VarDecl *VD, PossiblyUnreachableDiag PUD); - void IssueWarningsForRegisteredVarDecl(VarDecl *VD); + void issueWarningsForRegisteredVarDecl(VarDecl *VD); // Gets the default policy which is in effect at the given source location. Policy getPolicyInEffectAt(SourceLocation Loc); @@ -126,10 +126,6 @@ class AnalysisBasedWarnings { Policy &getPolicyOverrides() { return PolicyOverrides; } void PrintStats() const; - - template <typename Iterator> - void EmitPossiblyUnreachableDiags(AnalysisDeclContext &AC, - std::pair<Iterator, Iterator> PUDs); }; } // namespace sema diff --git a/clang/lib/Parse/ParseInit.cpp b/clang/lib/Parse/ParseInit.cpp index 3a17d622b2550..0e86c4c48d5e4 100644 --- a/clang/lib/Parse/ParseInit.cpp +++ b/clang/lib/Parse/ParseInit.cpp @@ -590,11 +590,9 @@ ExprResult Parser::ParseInitializer(Decl *DeclForInitializer) { // constexpr) should emit runtime warnings. if (DeclForInitializer && !Actions.ExprEvalContexts.empty()) { if (auto *VD = dyn_cast<VarDecl>(DeclForInitializer); - VD && VD->isFileVarDecl()) { - if (!VD->getType()->isReferenceType() || VD->isConstexpr()) { - Actions.ExprEvalContexts.back().DeclForInitializer = VD; - } - } + VD && VD->isFileVarDecl() && + (!VD->getType()->isReferenceType() || VD->isConstexpr())) + Actions.ExprEvalContexts.back().DeclForInitializer = VD; } ExprResult init; diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 98b9b241442f2..ca4ca6d75c475 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -2735,8 +2735,8 @@ static void flushDiagnostics(Sema &S, const sema::FunctionScopeInfo *fscope) { } template <typename Iterator> -void sema::AnalysisBasedWarnings::EmitPossiblyUnreachableDiags( - AnalysisDeclContext &AC, std::pair<Iterator, Iterator> PUDs) { +static void emitPossiblyUnreachableDiags(Sema &S, AnalysisDeclContext &AC, + std::pair<Iterator, Iterator> PUDs) { if (PUDs.first == PUDs.second) return; @@ -2768,12 +2768,12 @@ void sema::AnalysisBasedWarnings::EmitPossiblyUnreachableDiags( } } -void sema::AnalysisBasedWarnings::RegisterVarDeclWarning( +void sema::AnalysisBasedWarnings::registerVarDeclWarning( VarDecl *VD, clang::sema::PossiblyUnreachableDiag PUD) { VarDeclPossiblyUnreachableDiags.emplace(VD, PUD); } -void sema::AnalysisBasedWarnings::IssueWarningsForRegisteredVarDecl( +void sema::AnalysisBasedWarnings::issueWarningsForRegisteredVarDecl( VarDecl *VD) { if (!llvm::is_contained(VarDeclPossiblyUnreachableDiags, VD)) return; @@ -2791,8 +2791,8 @@ void sema::AnalysisBasedWarnings::IssueWarningsForRegisteredVarDecl( auto Range = VarDeclPossiblyUnreachableDiags.equal_range(VD); auto SecondRange = llvm::make_second_range(llvm::make_range(Range.first, Range.second)); - EmitPossiblyUnreachableDiags( - AC, std::make_pair(SecondRange.begin(), SecondRange.end())); + emitPossiblyUnreachableDiags( + S, AC, std::make_pair(SecondRange.begin(), SecondRange.end())); } // An AST Visitor that calls a callback function on each callable DEFINITION @@ -3007,7 +3007,7 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings( // Emit delayed diagnostics. auto &PUDs = fscope->PossiblyUnreachableDiags; - EmitPossiblyUnreachableDiags(AC, std::make_pair(PUDs.begin(), PUDs.end())); + 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 9f8a44a9b125f..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" @@ -13752,15 +13753,10 @@ void Sema::DiagnoseUniqueObjectDuplication(const VarDecl *VD) { } void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) { - // RAII helper to ensure DeclForInitializer is cleared on all exit paths - struct ClearDeclForInitializer { - Sema &S; - ClearDeclForInitializer(Sema &S) : S(S) {} - ~ClearDeclForInitializer() { - if (!S.ExprEvalContexts.empty()) - S.ExprEvalContexts.back().DeclForInitializer = nullptr; - } - } Clearer(*this); + 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. @@ -15089,7 +15085,7 @@ void Sema::FinalizeDeclaration(Decl *ThisDecl) { // Emit any deferred warnings for the variable's initializer, even if the // variable is invalid - AnalysisWarnings.IssueWarningsForRegisteredVarDecl(VD); + AnalysisWarnings.issueWarningsForRegisteredVarDecl(VD); // Apply an implicit SectionAttr if '#pragma clang section bss|data|rodata' is active if (VD->hasGlobalStorage() && VD->isThisDeclarationADefinition() && diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 14e692f4449aa..6d828b03fbdbc 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -20569,16 +20569,15 @@ void Sema::MarkDeclarationsReferencedInExpr(Expr *E, /// Emit a diagnostic when statements are reachable. bool Sema::DiagIfReachable(SourceLocation Loc, ArrayRef<const Stmt *> Stmts, const PartialDiagnostic &PD) { + 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. - if (auto *VD = dyn_cast_or_null<VarDecl>( - ExprEvalContexts.back().DeclForInitializer)) { - if (VD->isConstexpr() || - (VD->isStaticDataMember() && VD->isFirstDecl() && !VD->isInline())) - return false; - } + if (Decl && + (Decl->isConstexpr() || (Decl->isStaticDataMember() && + Decl->isFirstDecl() && !Decl->isInline()))) + return false; if (Stmts.empty()) { Diag(Loc, PD); @@ -20594,13 +20593,10 @@ bool Sema::DiagIfReachable(SourceLocation Loc, ArrayRef<const Stmt *> Stmts, // 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 (auto *VD = dyn_cast_or_null<VarDecl>( - ExprEvalContexts.back().DeclForInitializer)) { - if (VD->isFileVarDecl()) { - AnalysisWarnings.RegisterVarDeclWarning( - VD, sema::PossiblyUnreachableDiag(PD, Loc, Stmts)); - return true; - } + 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 af630b82bde60..74381e0cb182e 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -6473,7 +6473,7 @@ void Sema::InstantiateVariableDefinition(SourceLocation PointOfInstantiation, Var->setTemplateSpecializationKind(OldVar->getTemplateSpecializationKind(), OldVar->getPointOfInstantiation()); // Emit any deferred warnings for the variable's initializer - AnalysisWarnings.IssueWarningsForRegisteredVarDecl(Var); + AnalysisWarnings.issueWarningsForRegisteredVarDecl(Var); } // This variable may have local implicit instantiations that need to be >From 082c9769209e80504a2205581a4387bcf39ca5df Mon Sep 17 00:00:00 2001 From: Justin Stitt <[email protected]> Date: Tue, 21 Oct 2025 11:07:25 -0700 Subject: [PATCH 7/7] reinstate FIXME as it was not fixed Signed-off-by: Justin Stitt <[email protected]> --- clang/lib/Sema/AnalysisBasedWarnings.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index ca4ca6d75c475..41a98323450e4 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -2754,6 +2754,9 @@ static void emitPossiblyUnreachableDiags(Sema &S, AnalysisDeclContext &AC, 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; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
