Skip to content

Commit dc22756

Browse files
committed
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]>
1 parent 729aa96 commit dc22756

File tree

9 files changed

+57
-68
lines changed

9 files changed

+57
-68
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,9 @@ Improvements to Clang's diagnostics
319319
-----------------------------------
320320
- Diagnostics messages now refer to ``structured binding`` instead of ``decomposition``,
321321
to align with `P0615R0 <https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0615r0.html>`_ changing the term. (#GH157880)
322+
- Clang now suppresses runtime behavior warnings for unreachable code in file-scope
323+
variable initializers, matching the behavior for functions. This prevents false
324+
positives for operations in unreachable branches of constant expressions.
322325
- Added a separate diagnostic group ``-Wfunction-effect-redeclarations``, for the more pedantic
323326
diagnostics for function effects (``[[clang::nonblocking]]`` and ``[[clang::nonallocating]]``).
324327
Moved the warning for a missing (though implied) attribute on a redeclaration into this group.

clang/include/clang/Parse/Parser.h

Lines changed: 1 addition & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -5223,32 +5223,7 @@ class Parser : public CodeCompletionHandler {
52235223
/// assignment-expression
52245224
/// '{' ...
52255225
/// \endverbatim
5226-
ExprResult ParseInitializer(Decl *DeclForInitializer = nullptr) {
5227-
// Set DeclForInitializer for file-scope variables.
5228-
// For constexpr references, set it to suppress runtime warnings.
5229-
// For non-constexpr references, don't set it to avoid evaluation issues
5230-
// with self-referencing initializers. Local variables (including local
5231-
// constexpr) should emit runtime warnings.
5232-
if (DeclForInitializer && !Actions.ExprEvalContexts.empty()) {
5233-
if (auto *VD = dyn_cast<VarDecl>(DeclForInitializer)) {
5234-
if (VD->isFileVarDecl()) {
5235-
if (!VD->getType()->isReferenceType() || VD->isConstexpr()) {
5236-
Actions.ExprEvalContexts.back().DeclForInitializer =
5237-
DeclForInitializer;
5238-
}
5239-
}
5240-
}
5241-
}
5242-
5243-
ExprResult init;
5244-
if (Tok.isNot(tok::l_brace)) {
5245-
init = ParseAssignmentExpression();
5246-
} else {
5247-
init = ParseBraceInitializer();
5248-
}
5249-
5250-
return init;
5251-
}
5226+
ExprResult ParseInitializer(Decl *DeclForInitializer = nullptr);
52525227

52535228
/// MayBeDesignationStart - Return true if the current token might be the
52545229
/// start of a designator. If we can tell it is impossible that it is a

clang/include/clang/Sema/AnalysisBasedWarnings.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,6 @@ class AnalysisBasedWarnings {
6767
Policy PolicyOverrides;
6868
void clearOverrides();
6969

70-
void FlushDiagnostics(SmallVector<clang::sema::PossiblyUnreachableDiag, 4>);
71-
7270
/// \name Statistics
7371
/// @{
7472

clang/include/clang/Sema/Sema.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6768,7 +6768,7 @@ class Sema final : public SemaBase {
67686768
/// Declaration for initializer if one is currently being
67696769
/// parsed. Used when an expression has a possibly unreachable
67706770
/// diagnostic to reference the declaration as a whole.
6771-
Decl *DeclForInitializer = nullptr;
6771+
VarDecl *DeclForInitializer = nullptr;
67726772

67736773
/// If we are processing a decltype type, a set of call expressions
67746774
/// for which we have deferred checking the completeness of the return type.

clang/lib/Analysis/AnalysisDeclContext.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ Stmt *AnalysisDeclContext::getBody(bool &IsAutosynthesized) const {
118118
else if (const auto *FunTmpl = dyn_cast_or_null<FunctionTemplateDecl>(D))
119119
return FunTmpl->getTemplatedDecl()->getBody();
120120
else if (const auto *VD = dyn_cast_or_null<VarDecl>(D)) {
121-
if (VD->hasGlobalStorage()) {
121+
if (VD->isFileVarDecl()) {
122122
return const_cast<Stmt *>(dyn_cast_or_null<Stmt>(VD->getInit()));
123123
}
124124
}

clang/lib/Parse/ParseInit.cpp

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -581,3 +581,28 @@ bool Parser::ParseMicrosoftIfExistsBraceInitializer(ExprVector &InitExprs,
581581

582582
return !trailingComma;
583583
}
584+
585+
ExprResult Parser::ParseInitializer(Decl *DeclForInitializer) {
586+
// Set DeclForInitializer for file-scope variables.
587+
// For constexpr references, set it to suppress runtime warnings.
588+
// For non-constexpr references, don't set it to avoid evaluation issues
589+
// with self-referencing initializers. Local variables (including local
590+
// constexpr) should emit runtime warnings.
591+
if (DeclForInitializer && !Actions.ExprEvalContexts.empty()) {
592+
if (auto *VD = dyn_cast<VarDecl>(DeclForInitializer);
593+
VD && VD->isFileVarDecl()) {
594+
if (!VD->getType()->isReferenceType() || VD->isConstexpr()) {
595+
Actions.ExprEvalContexts.back().DeclForInitializer = VD;
596+
}
597+
}
598+
}
599+
600+
ExprResult init;
601+
if (Tok.isNot(tok::l_brace)) {
602+
init = ParseAssignmentExpression();
603+
} else {
604+
init = ParseBraceInitializer();
605+
}
606+
607+
return init;
608+
}

clang/lib/Sema/AnalysisBasedWarnings.cpp

Lines changed: 16 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -2734,53 +2734,36 @@ static void flushDiagnostics(Sema &S, const sema::FunctionScopeInfo *fscope) {
27342734
S.Diag(D.Loc, D.PD);
27352735
}
27362736

2737-
void sema::AnalysisBasedWarnings::FlushDiagnostics(
2738-
const SmallVector<clang::sema::PossiblyUnreachableDiag, 4> PUDs) {
2739-
for (const auto &D : PUDs)
2740-
S.Diag(D.Loc, D.PD);
2741-
}
2742-
27432737
void sema::AnalysisBasedWarnings::EmitPossiblyUnreachableDiags(
27442738
AnalysisDeclContext &AC,
27452739
SmallVector<clang::sema::PossiblyUnreachableDiag, 4> PUDs) {
27462740

2747-
if (PUDs.empty()) {
2741+
if (PUDs.empty())
27482742
return;
2749-
}
2750-
2751-
bool Analyzed = false;
27522743

27532744
for (const auto &D : PUDs) {
27542745
for (const Stmt *S : D.Stmts)
27552746
AC.registerForcedBlockExpression(S);
27562747
}
27572748

27582749
if (AC.getCFG()) {
2759-
Analyzed = true;
27602750
for (const auto &D : PUDs) {
2761-
bool AllReachable = true;
2762-
for (const Stmt *St : D.Stmts) {
2763-
const CFGBlock *block = AC.getBlockForRegisteredExpression(St);
2764-
CFGReverseBlockReachabilityAnalysis *cra =
2765-
AC.getCFGReachablityAnalysis();
2766-
if (block && cra) {
2767-
// Can this block be reached from the entrance?
2768-
if (!cra->isReachable(&AC.getCFG()->getEntry(), block)) {
2769-
AllReachable = false;
2770-
break;
2771-
}
2772-
}
2773-
// If we cannot map to a basic block, assume the statement is
2774-
// reachable.
2775-
}
2776-
2777-
if (AllReachable)
2751+
if (llvm::all_of(D.Stmts, [&](const Stmt *St) {
2752+
const CFGBlock *Block = AC.getBlockForRegisteredExpression(St);
2753+
CFGReverseBlockReachabilityAnalysis *Analysis =
2754+
AC.getCFGReachablityAnalysis();
2755+
if (Block && Analysis)
2756+
if (!Analysis->isReachable(&AC.getCFG()->getEntry(), Block))
2757+
return false;
2758+
return true;
2759+
})) {
27782760
S.Diag(D.Loc, D.PD);
2761+
}
27792762
}
2763+
} else {
2764+
for (const auto &D : PUDs)
2765+
S.Diag(D.Loc, D.PD);
27802766
}
2781-
2782-
if (!Analyzed)
2783-
FlushDiagnostics(PUDs);
27842767
}
27852768

27862769
void sema::AnalysisBasedWarnings::RegisterVarDeclWarning(
@@ -2790,12 +2773,10 @@ void sema::AnalysisBasedWarnings::RegisterVarDeclWarning(
27902773

27912774
void sema::AnalysisBasedWarnings::IssueWarningsForRegisteredVarDecl(
27922775
VarDecl *VD) {
2793-
if (VarDeclPossiblyUnreachableDiags.find(VD) ==
2794-
VarDeclPossiblyUnreachableDiags.end()) {
2776+
if (!llvm::is_contained(VarDeclPossiblyUnreachableDiags, VD))
27952777
return;
2796-
}
27972778

2798-
AnalysisDeclContext AC(nullptr, VD);
2779+
AnalysisDeclContext AC(/*Mgr=*/nullptr, VD);
27992780

28002781
AC.getCFGBuildOptions().PruneTriviallyFalseEdges = true;
28012782
AC.getCFGBuildOptions().AddEHEdges = false;

clang/lib/Sema/SemaDecl.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13121,9 +13121,9 @@ namespace {
1312113121
// Skip checking for file-scope constexpr variables - constant evaluation
1312213122
// will produce appropriate errors without needing runtime diagnostics.
1312313123
// Local constexpr should still emit runtime warnings.
13124-
if (auto *VD = dyn_cast<VarDecl>(OrigDecl))
13125-
if (VD->isConstexpr() && VD->isFileVarDecl())
13126-
return;
13124+
if (auto *VD = dyn_cast<VarDecl>(OrigDecl);
13125+
VD && VD->isConstexpr() && VD->isFileVarDecl())
13126+
return;
1312713127

1312813128
E = E->IgnoreParens();
1312913129

clang/test/Sema/warn-unreachable-file-scope.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,3 +28,10 @@ void f(void) {
2828
unsigned long long e3 = 1 ? 0 : 1ULL << 64;
2929
unsigned long long e4 = 0 ? 0 : 1ULL << 64; // expected-warning {{shift count >= width of type}}
3030
}
31+
32+
void statics(void) {
33+
static u8 f1 = (0 ? 0xffff : 0xff);
34+
static u8 f2 = (1 ? 0xffff : 0xff); // expected-warning {{implicit conversion from 'int' to 'u8' (aka 'unsigned char') changes value from 65535 to 255}}
35+
static u8 f3 = (1 ? 0xff : 0xffff);
36+
static u8 f4 = (0 ? 0xff : 0xffff); // expected-warning {{implicit conversion from 'int' to 'u8' (aka 'unsigned char') changes value from 65535 to 255}}
37+
}

0 commit comments

Comments
 (0)