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]>

Author: JustinStitt

clang/docs/ReleaseNotes.rst

@@ -286,6 +286,10 @@ Attribute Changes in Clang
 
 Improvements to Clang's diagnostics
 -----------------------------------
+- 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.

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

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
   /// @{
 

clang/include/clang/Sema/Sema.h

@@ -6753,7 +6753,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.

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

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;
+}

clang/lib/Sema/AnalysisBasedWarnings.cpp

@@ -2724,53 +2724,36 @@ 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)
       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)
+      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(
@@ -2780,12 +2763,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;

clang/lib/Sema/SemaDecl.cpp

@@ -13103,9 +13103,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();
 

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}}
+}