[clang] Control analysis-based diagnostics with #pragma (PR #136323)

[Aaron Ballman via cfe-commits]

AaronBallman wrote:

> > FYI this introduces some overhead: https://llvm-compile-time-tracker.com/compare.php?from=2a9f77f6bd48d757b2d45aadcb6cf76ef4b4ef32&to=71ce9e26aec00e4af27a69ccfab8ca1773ed7018&stat=instructions:u About 0.3% on clang files.
> 
> Thank you for the notification! Shoot. I'm not certain if there's a better way to do this or if this is just the cost of doing business, but that is unfortunate.

I tried a different approach:
```
diff --git a/clang/include/clang/Sema/AnalysisBasedWarnings.h b/clang/include/clang/Sema/AnalysisBasedWarnings.h
index 4103c3f006a8..32bfddec9894 100644
--- a/clang/include/clang/Sema/AnalysisBasedWarnings.h
+++ b/clang/include/clang/Sema/AnalysisBasedWarnings.h
@@ -58,7 +58,7 @@ private:
   enum VisitFlag { NotVisited = 0, Visited = 1, Pending = 2 };
   llvm::DenseMap<const FunctionDecl*, VisitFlag> VisitedFD;
 
-  Policy PolicyOverrides;
+  Policy DefaultPolicy, PolicyOverrides;
   void clearOverrides();
 
   /// \name Statistics
@@ -107,8 +107,8 @@ public:
   // Issue warnings that require whole-translation-unit analysis.
   void IssueWarnings(TranslationUnitDecl *D);
 
-  // Gets the default policy which is in effect at the given source location.
-  Policy getPolicyInEffectAt(SourceLocation Loc);
+  // Gets the policy which is currently in effect.
+  Policy getPolicyInEffect() const;
 
   // Get the policies we may want to override due to things like #pragma clang
   // diagnostic handling. If a caller sets any of these policies to true, that
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 2418aaf8de8e..8726389f43f1 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2493,9 +2493,8 @@ public:
 };
 
 template <typename... Ts>
-static bool areAnyEnabled(DiagnosticsEngine &D, SourceLocation Loc,
-                          Ts... Diags) {
-  return (!D.isIgnored(Diags, Loc) || ...);
+static bool areAnyGloballyEnabled(DiagnosticsEngine &D, Ts... Diags) {
+  return (!D.isIgnored(Diags, {}) || ...);
 }
 
 sema::AnalysisBasedWarnings::AnalysisBasedWarnings(Sema &s)
@@ -2505,29 +2504,35 @@ sema::AnalysisBasedWarnings::AnalysisBasedWarnings(Sema &s)
       NumUninitAnalysisVariables(0), MaxUninitAnalysisVariablesPerFunction(0),
       NumUninitAnalysisBlockVisits(0),
       MaxUninitAnalysisBlockVisitsPerFunction(0) {
+  using namespace diag;
+  DiagnosticsEngine &D = S.getDiagnostics();
+  // Note: The enabled checks should be kept in sync with the switch in
+  // SemaPPCallbacks::PragmaDiagnostic().
+  DefaultPolicy.enableCheckUnreachable = areAnyGloballyEnabled(
+      D, warn_unreachable, warn_unreachable_break, warn_unreachable_return,
+      warn_unreachable_loop_increment);
+
+  DefaultPolicy.enableThreadSafetyAnalysis =
+      areAnyGloballyEnabled(D, warn_double_lock);
+
+  DefaultPolicy.enableConsumedAnalysis =
+      areAnyGloballyEnabled(D, warn_use_in_invalid_state);
 }
 
 // We need this here for unique_ptr with forward declared class.
 sema::AnalysisBasedWarnings::~AnalysisBasedWarnings() = default;
 
 sema::AnalysisBasedWarnings::Policy
-sema::AnalysisBasedWarnings::getPolicyInEffectAt(SourceLocation Loc) {
-  using namespace diag;
-  DiagnosticsEngine &D = S.getDiagnostics();
+sema::AnalysisBasedWarnings::getPolicyInEffect() const {
   Policy P;
-
-  // Note: The enabled checks should be kept in sync with the switch in
-  // SemaPPCallbacks::PragmaDiagnostic().
-  P.enableCheckUnreachable =
-      PolicyOverrides.enableCheckUnreachable ||
-      areAnyEnabled(D, Loc, warn_unreachable, warn_unreachable_break,
-                    warn_unreachable_return, warn_unreachable_loop_increment);
+  P.enableCheckUnreachable = PolicyOverrides.enableCheckUnreachable ||
+                             DefaultPolicy.enableCheckUnreachable;
 
   P.enableThreadSafetyAnalysis = PolicyOverrides.enableThreadSafetyAnalysis ||
-                                 areAnyEnabled(D, Loc, warn_double_lock);
+                                 DefaultPolicy.enableThreadSafetyAnalysis;
 
   P.enableConsumedAnalysis = PolicyOverrides.enableConsumedAnalysis ||
-                             areAnyEnabled(D, Loc, warn_use_in_invalid_state);
+                             DefaultPolicy.enableConsumedAnalysis;
   return P;
 }
 
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 4039601612c6..5c0c17cfa5e4 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -219,7 +219,7 @@ public:
 
     for (diag::kind K : GroupDiags) {
       // Note: the cases in this switch should be kept in sync with the
-      // diagnostics in AnalysisBasedWarnings::getPolicyInEffectAt().
+      // diagnostics in the AnalysisBasedWarnings constructor.
       AnalysisBasedWarnings::Policy &Override =
           S->AnalysisWarnings.getPolicyOverrides();
       switch (K) {
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index d28a2107d58a..372ea1cfd29c 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -16150,13 +16150,7 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,
   if (FSI->UsesFPIntrin && FD && !FD->hasAttr<StrictFPAttr>())
     FD->addAttr(StrictFPAttr::CreateImplicit(Context));
 
-  SourceLocation AnalysisLoc;
-  if (Body)
-    AnalysisLoc = Body->getEndLoc();
-  else if (FD)
-    AnalysisLoc = FD->getEndLoc();
-  sema::AnalysisBasedWarnings::Policy WP =
-      AnalysisWarnings.getPolicyInEffectAt(AnalysisLoc);
+  sema::AnalysisBasedWarnings::Policy WP = AnalysisWarnings.getPolicyInEffect();
   sema::AnalysisBasedWarnings::Policy *ActivePolicy = nullptr;
 
   // If we skip function body, we can't tell if a function is a coroutine.
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 2e6ce17f8bf9..6b8d1274969c 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -16597,8 +16597,7 @@ ExprResult Sema::ActOnBlockStmtExpr(SourceLocation CaretLoc,
   BD->setCaptures(Context, Captures, BSI->CXXThisCaptureIndex != 0);
 
   // Pop the block scope now but keep it alive to the end of this function.
-  AnalysisBasedWarnings::Policy WP =
-      AnalysisWarnings.getPolicyInEffectAt(Body->getEndLoc());
+  AnalysisBasedWarnings::Policy WP = AnalysisWarnings.getPolicyInEffect();
   PoppedFunctionScopePtr ScopeRAII = PopFunctionScopeInfo(&WP, BD, BlockTy);
 
   BlockExpr *Result = new (Context)
```
but this causes test case failures. We process the pragma when we consume the closing `}` from a preceding function body. Without checking for the diagnostic being enabled at a particular source location, we'll fail to catch the diagnostic here:
```
int a() {	
  Linear l;
  return 0; // No -Wconsumed diagnostic, analysis is not enabled.
}

#pragma clang diagnostic push
#pragma clang diagnostic error "-Wconsumed"
int b() {
  Linear l;
  return 0;  // expected-error {{invalid invocation of method '~Linear' on object 'l' while it is in the 'unconsumed' state}}
}
#pragma clang diagnostic pop
```
because we set the override at the end of `a`, reset the override at the end of `a`, and then miss that it's enabled at the end of `b`.

https://github.com/llvm/llvm-project/pull/136323