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

Author: JustinStitt

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

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;

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) {

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() &&

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);

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