[Clang] Consider reachability for file-scope warnings on initializers #163885
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-analysis Author: Justin Stitt (JustinStitt) ChangesEvery once in awhile, we get a new Linux kernel report about false positive clang warnings being emitted for file-scope variables when initialized. See [[0]](https://lore.kernel.org/all/8c252429-5000-0649-c49f-8225d911241b@gmail.com/) and [[1]](https://lore.kernel.org/all/202112181827.o3X7GmHz-lkp@intel.com/). To help fix this, back in 2019 Nathan Huckleberry sent a phab PR D63889. Nathan was @nickdesaulniers' intern and most likely had to return to school before that PR landed. I was able to use a good amount of code from that PR with a few workarounds to fix bitrot and clang regression suite failures. So, consider this a spiritual revival of that patchset. cc @nickdesaulniers @kees @nathanchance (the other Nathan) 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: $ 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 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) Full diff: https://github.com/llvm/llvm-project/pull/163885.diff 12 Files Affected:
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index e301cf1080977..ec37cf2b0640d 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 265462a86e405..b376950fea916 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -6750,6 +6750,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 d6cd7eb8c2c3d..3ac629463a68c 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 19f94122f151e..a546688e81981 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 04f29c83dd457..8e60b3043c8ad 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 8606227152a84..8d73ea308b38c 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2724,6 +2724,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 {
@@ -2935,45 +3009,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 6eaf7b9435491..5547ef316ecb5 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -13100,6 +13100,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.
@@ -13727,6 +13734,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) {
@@ -15045,6 +15062,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 4d3c7d611f370..a09dbf7f5d6a8 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -20550,25 +20550,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 3819f775811e5..fcfa99cbdba4f 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -6164,6 +6164,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 =
@@ -6426,6 +6430,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}}
+}
|
|
also Fixes: #38137 |
zwuis
reviewed
Oct 17, 2025
cor3ntin
reviewed
Oct 19, 2025
Comment on lines
64
to
65
| llvm::MapVector<VarDecl *, SmallVector<PossiblyUnreachableDiag, 4>> | ||
| VarDeclPossiblyUnreachableDiags; |
There was a problem hiding this comment.
I feel like a std::multimap<VarDecl *, PossiblyUnreachableDiag> would be a more suited container here
(still preserve order and avoid having lots of SmallVector)
There was a problem hiding this comment.
Hi, this is done in c6ec954 (#163885) but I think it wasn't as elegant as I was expecting. This is because SmallVector<PossiblyUnreachableDiag, 4> PossiblyUnreachableDiags; in ScopeInfo.h is already declared as a vector. The change meant I needed to template emitPossiblyUnreachableDiags and use llvm::make_second_range.
| /// 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; |
There was a problem hiding this comment.
Would that be ever different from ManglingContextDecl? Can we reuse that?
There was a problem hiding this comment.
There was a fixme: // FIXME: Using the mangling context here is a hack.
So I went ahead and made DeclForInitializer. I think this is better because I can type it as VarDecl and its name makes more sense in this context too.
clang/lib/Parse/ParseInit.cpp
Outdated
Comment on lines
591
to
596
| 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; | ||
| } |
There was a problem hiding this comment.
I think we should set DeclForInitializer unconditionally, and check if it's a non-constexpr ref in MarkDeclarationsReferencedInExpr
There was a problem hiding this comment.
I initially tried to move this logic out of the parser but didn't find a viable option. Looking at MarkDeclarationsReferencedInExpr also doesn't make much sense to me. Can you help me understand what I should do here? I don't see how MarkDeclarationsReferencedInExpr can help with deferred diags.
zwuis
reviewed
Oct 19, 2025
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: ClangBuiltLinux/linux#92
Co-Authored-By: Nathan Huckleberry <nhuck@google.com>
Signed-off-by: Justin Stitt <justinstitt@google.com>
Signed-off-by: Justin Stitt <justinstitt@google.com>
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 <justinstitt@google.com>
Signed-off-by: Justin Stitt <justinstitt@google.com>
Signed-off-by: Justin Stitt <justinstitt@google.com>
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
Signed-off-by: Justin Stitt <justinstitt@google.com>
|
LGTM but please wait for other reviewers. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Every once in awhile, we get a new Linux kernel report about false positive clang warnings being emitted for file-scope variables when initialized. See [0] and [1].
To help fix this, back in 2019 Nathan Huckleberry sent a phab PR D63889. Nathan was @nickdesaulniers' intern and most likely had to return to school before that PR landed. I was able to use a good amount of code from that PR with a few workarounds to fix bitrot and clang regression suite failures. So, consider this a spiritual revival of that patchset.
cc @nickdesaulniers @kees @nathanchance (the other Nathan)
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:
Clang previously emitted a
-Wshift-count-overflowwarning 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)
Fixes: ClangBuiltLinux/linux#92
Fixes: #38137