Skip to content

Commit 675138b

Browse files
committed
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
1 parent c6ec954 commit 675138b

File tree

6 files changed

+28
-42
lines changed

6 files changed

+28
-42
lines changed

clang/include/clang/Sema/AnalysisBasedWarnings.h

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -113,9 +113,9 @@ class AnalysisBasedWarnings {
113113
// Issue warnings that require whole-translation-unit analysis.
114114
void IssueWarnings(TranslationUnitDecl *D);
115115

116-
void RegisterVarDeclWarning(VarDecl *VD, PossiblyUnreachableDiag PUD);
116+
void registerVarDeclWarning(VarDecl *VD, PossiblyUnreachableDiag PUD);
117117

118-
void IssueWarningsForRegisteredVarDecl(VarDecl *VD);
118+
void issueWarningsForRegisteredVarDecl(VarDecl *VD);
119119

120120
// Gets the default policy which is in effect at the given source location.
121121
Policy getPolicyInEffectAt(SourceLocation Loc);
@@ -126,10 +126,6 @@ class AnalysisBasedWarnings {
126126
Policy &getPolicyOverrides() { return PolicyOverrides; }
127127

128128
void PrintStats() const;
129-
130-
template <typename Iterator>
131-
void EmitPossiblyUnreachableDiags(AnalysisDeclContext &AC,
132-
std::pair<Iterator, Iterator> PUDs);
133129
};
134130

135131
} // namespace sema

clang/lib/Parse/ParseInit.cpp

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -590,11 +590,9 @@ ExprResult Parser::ParseInitializer(Decl *DeclForInitializer) {
590590
// constexpr) should emit runtime warnings.
591591
if (DeclForInitializer && !Actions.ExprEvalContexts.empty()) {
592592
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-
}
593+
VD && VD->isFileVarDecl() &&
594+
(!VD->getType()->isReferenceType() || VD->isConstexpr()))
595+
Actions.ExprEvalContexts.back().DeclForInitializer = VD;
598596
}
599597

600598
ExprResult init;

clang/lib/Sema/AnalysisBasedWarnings.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2725,8 +2725,8 @@ static void flushDiagnostics(Sema &S, const sema::FunctionScopeInfo *fscope) {
27252725
}
27262726

27272727
template <typename Iterator>
2728-
void sema::AnalysisBasedWarnings::EmitPossiblyUnreachableDiags(
2729-
AnalysisDeclContext &AC, std::pair<Iterator, Iterator> PUDs) {
2728+
static void emitPossiblyUnreachableDiags(Sema &S, AnalysisDeclContext &AC,
2729+
std::pair<Iterator, Iterator> PUDs) {
27302730

27312731
if (PUDs.first == PUDs.second)
27322732
return;
@@ -2758,12 +2758,12 @@ void sema::AnalysisBasedWarnings::EmitPossiblyUnreachableDiags(
27582758
}
27592759
}
27602760

2761-
void sema::AnalysisBasedWarnings::RegisterVarDeclWarning(
2761+
void sema::AnalysisBasedWarnings::registerVarDeclWarning(
27622762
VarDecl *VD, clang::sema::PossiblyUnreachableDiag PUD) {
27632763
VarDeclPossiblyUnreachableDiags.emplace(VD, PUD);
27642764
}
27652765

2766-
void sema::AnalysisBasedWarnings::IssueWarningsForRegisteredVarDecl(
2766+
void sema::AnalysisBasedWarnings::issueWarningsForRegisteredVarDecl(
27672767
VarDecl *VD) {
27682768
if (!llvm::is_contained(VarDeclPossiblyUnreachableDiags, VD))
27692769
return;
@@ -2781,8 +2781,8 @@ void sema::AnalysisBasedWarnings::IssueWarningsForRegisteredVarDecl(
27812781
auto Range = VarDeclPossiblyUnreachableDiags.equal_range(VD);
27822782
auto SecondRange =
27832783
llvm::make_second_range(llvm::make_range(Range.first, Range.second));
2784-
EmitPossiblyUnreachableDiags(
2785-
AC, std::make_pair(SecondRange.begin(), SecondRange.end()));
2784+
emitPossiblyUnreachableDiags(
2785+
S, AC, std::make_pair(SecondRange.begin(), SecondRange.end()));
27862786
}
27872787

27882788
// An AST Visitor that calls a callback function on each callable DEFINITION
@@ -2997,7 +2997,7 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings(
29972997

29982998
// Emit delayed diagnostics.
29992999
auto &PUDs = fscope->PossiblyUnreachableDiags;
3000-
EmitPossiblyUnreachableDiags(AC, std::make_pair(PUDs.begin(), PUDs.end()));
3000+
emitPossiblyUnreachableDiags(S, AC, std::make_pair(PUDs.begin(), PUDs.end()));
30013001

30023002
// Warning: check missing 'return'
30033003
if (P.enableCheckFallThrough) {

clang/lib/Sema/SemaDecl.cpp

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
#include "clang/Sema/SemaWasm.h"
6060
#include "clang/Sema/Template.h"
6161
#include "llvm/ADT/STLForwardCompat.h"
62+
#include "llvm/ADT/ScopeExit.h"
6263
#include "llvm/ADT/SmallPtrSet.h"
6364
#include "llvm/ADT/SmallString.h"
6465
#include "llvm/ADT/StringExtras.h"
@@ -13734,15 +13735,10 @@ void Sema::DiagnoseUniqueObjectDuplication(const VarDecl *VD) {
1373413735
}
1373513736

1373613737
void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
13737-
// RAII helper to ensure DeclForInitializer is cleared on all exit paths
13738-
struct ClearDeclForInitializer {
13739-
Sema &S;
13740-
ClearDeclForInitializer(Sema &S) : S(S) {}
13741-
~ClearDeclForInitializer() {
13742-
if (!S.ExprEvalContexts.empty())
13743-
S.ExprEvalContexts.back().DeclForInitializer = nullptr;
13744-
}
13745-
} Clearer(*this);
13738+
auto ResetDeclForInitializer = llvm::make_scope_exit([this]() {
13739+
if (this->ExprEvalContexts.empty())
13740+
this->ExprEvalContexts.back().DeclForInitializer = nullptr;
13741+
});
1374613742

1374713743
// If there is no declaration, there was an error parsing it. Just ignore
1374813744
// the initializer.
@@ -15064,7 +15060,7 @@ void Sema::FinalizeDeclaration(Decl *ThisDecl) {
1506415060

1506515061
// Emit any deferred warnings for the variable's initializer, even if the
1506615062
// variable is invalid
15067-
AnalysisWarnings.IssueWarningsForRegisteredVarDecl(VD);
15063+
AnalysisWarnings.issueWarningsForRegisteredVarDecl(VD);
1506815064

1506915065
// Apply an implicit SectionAttr if '#pragma clang section bss|data|rodata' is active
1507015066
if (VD->hasGlobalStorage() && VD->isThisDeclarationADefinition() &&

clang/lib/Sema/SemaExpr.cpp

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20546,16 +20546,15 @@ void Sema::MarkDeclarationsReferencedInExpr(Expr *E,
2054620546
/// Emit a diagnostic when statements are reachable.
2054720547
bool Sema::DiagIfReachable(SourceLocation Loc, ArrayRef<const Stmt *> Stmts,
2054820548
const PartialDiagnostic &PD) {
20549+
VarDecl *Decl = ExprEvalContexts.back().DeclForInitializer;
2054920550
// The initializer of a constexpr variable or of the first declaration of a
2055020551
// static data member is not syntactically a constant evaluated constant,
2055120552
// but nonetheless is always required to be a constant expression, so we
2055220553
// can skip diagnosing.
20553-
if (auto *VD = dyn_cast_or_null<VarDecl>(
20554-
ExprEvalContexts.back().DeclForInitializer)) {
20555-
if (VD->isConstexpr() ||
20556-
(VD->isStaticDataMember() && VD->isFirstDecl() && !VD->isInline()))
20557-
return false;
20558-
}
20554+
if (Decl &&
20555+
(Decl->isConstexpr() || (Decl->isStaticDataMember() &&
20556+
Decl->isFirstDecl() && !Decl->isInline())))
20557+
return false;
2055920558

2056020559
if (Stmts.empty()) {
2056120560
Diag(Loc, PD);
@@ -20571,13 +20570,10 @@ bool Sema::DiagIfReachable(SourceLocation Loc, ArrayRef<const Stmt *> Stmts,
2057120570
// For non-constexpr file-scope variables with reachability context (non-empty
2057220571
// Stmts), build a CFG for the initializer and check whether the context in
2057320572
// question is reachable.
20574-
if (auto *VD = dyn_cast_or_null<VarDecl>(
20575-
ExprEvalContexts.back().DeclForInitializer)) {
20576-
if (VD->isFileVarDecl()) {
20577-
AnalysisWarnings.RegisterVarDeclWarning(
20578-
VD, sema::PossiblyUnreachableDiag(PD, Loc, Stmts));
20579-
return true;
20580-
}
20573+
if (Decl && Decl->isFileVarDecl()) {
20574+
AnalysisWarnings.registerVarDeclWarning(
20575+
Decl, sema::PossiblyUnreachableDiag(PD, Loc, Stmts));
20576+
return true;
2058120577
}
2058220578

2058320579
Diag(Loc, PD);

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6431,7 +6431,7 @@ void Sema::InstantiateVariableDefinition(SourceLocation PointOfInstantiation,
64316431
Var->setTemplateSpecializationKind(OldVar->getTemplateSpecializationKind(),
64326432
OldVar->getPointOfInstantiation());
64336433
// Emit any deferred warnings for the variable's initializer
6434-
AnalysisWarnings.IssueWarningsForRegisteredVarDecl(Var);
6434+
AnalysisWarnings.issueWarningsForRegisteredVarDecl(Var);
64356435
}
64366436

64376437
// This variable may have local implicit instantiations that need to be

0 commit comments

Comments
 (0)