Control analysis-based diagnostics with #pragma (#136323)

Previously, analysis-based diagnostics (like -Wconsumed) had to be
enabled at file scope in order to be run at the end of each function
body. This meant that they did not respect #pragma clang diagnostic
enabling or disabling the diagnostic.

Now, these pragmas can control the diagnostic emission.

Fixes #42199

Author: AaronBallman

clang/docs/ReleaseNotes.rst

@@ -398,6 +398,8 @@ Improvements to Clang's diagnostics
   constructors to initialize their non-modifiable members. The diagnostic is
   not new; being controlled via a warning group is what's new. Fixes #GH41104
 
+- Analysis-based diagnostics (like ``-Wconsumed`` or ``-Wunreachable-code``)
+  can now be correctly controlled by ``#pragma clang diagnostic``. #GH42199
 
 - Improved Clang's error recovery for invalid function calls.
 

clang/include/clang/Sema/AnalysisBasedWarnings.h

@@ -25,6 +25,7 @@ class QualType;
 class Sema;
 namespace sema {
   class FunctionScopeInfo;
+  class SemaPPCallbacks;
 }
 
 namespace sema {
@@ -33,6 +34,7 @@ class AnalysisBasedWarnings {
 public:
   class Policy {
     friend class AnalysisBasedWarnings;
+    friend class SemaPPCallbacks;
     // The warnings to run.
     LLVM_PREFERRED_TYPE(bool)
     unsigned enableCheckFallThrough : 1;
@@ -49,14 +51,16 @@ class AnalysisBasedWarnings {
 
 private:
   Sema &S;
-  Policy DefaultPolicy;
 
   class InterProceduralData;
   std::unique_ptr<InterProceduralData> IPData;
 
   enum VisitFlag { NotVisited = 0, Visited = 1, Pending = 2 };
   llvm::DenseMap<const FunctionDecl*, VisitFlag> VisitedFD;
 
+  Policy PolicyOverrides;
+  void clearOverrides();
+
   /// \name Statistics
   /// @{
 
@@ -103,7 +107,13 @@ class AnalysisBasedWarnings {
   // Issue warnings that require whole-translation-unit analysis.
   void IssueWarnings(TranslationUnitDecl *D);
 
-  Policy getDefaultPolicy() { return DefaultPolicy; }
+  // Gets the default policy which is in effect at the given source location.
+  Policy getPolicyInEffectAt(SourceLocation Loc);
+
+  // 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
+  // will override the policy used to issue warnings.
+  Policy &getPolicyOverrides() { return PolicyOverrides; }
 
   void PrintStats() const;
 };

clang/lib/Sema/AnalysisBasedWarnings.cpp

@@ -2492,9 +2492,11 @@ class sema::AnalysisBasedWarnings::InterProceduralData {
   CalledOnceInterProceduralData CalledOnceData;
 };
 
-static unsigned isEnabled(DiagnosticsEngine &D, unsigned diag) {
-  return (unsigned)!D.isIgnored(diag, SourceLocation());
-}
+template <typename... Ts>
+static bool areAnyEnabled(DiagnosticsEngine &D, SourceLocation Loc,
+                          Ts... Diags) {
+  return (!D.isIgnored(Diags, Loc) || ...);
+};
 
 sema::AnalysisBasedWarnings::AnalysisBasedWarnings(Sema &s)
     : S(s), IPData(std::make_unique<InterProceduralData>()),
@@ -2503,23 +2505,37 @@ sema::AnalysisBasedWarnings::AnalysisBasedWarnings(Sema &s)
       NumUninitAnalysisVariables(0), MaxUninitAnalysisVariablesPerFunction(0),
       NumUninitAnalysisBlockVisits(0),
       MaxUninitAnalysisBlockVisitsPerFunction(0) {
+}
+
+// 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();
+  Policy P;
 
-  DefaultPolicy.enableCheckUnreachable =
-      isEnabled(D, warn_unreachable) || isEnabled(D, warn_unreachable_break) ||
-      isEnabled(D, warn_unreachable_return) ||
-      isEnabled(D, warn_unreachable_loop_increment);
+  // 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);
 
-  DefaultPolicy.enableThreadSafetyAnalysis = isEnabled(D, warn_double_lock);
+  P.enableThreadSafetyAnalysis = PolicyOverrides.enableThreadSafetyAnalysis ||
+                                 areAnyEnabled(D, Loc, warn_double_lock);
 
-  DefaultPolicy.enableConsumedAnalysis =
-      isEnabled(D, warn_use_in_invalid_state);
+  P.enableConsumedAnalysis = PolicyOverrides.enableConsumedAnalysis ||
+                             areAnyEnabled(D, Loc, warn_use_in_invalid_state);
+  return P;
 }
 
-// We need this here for unique_ptr with forward declared class.
-sema::AnalysisBasedWarnings::~AnalysisBasedWarnings() = default;
+void sema::AnalysisBasedWarnings::clearOverrides() {
+  PolicyOverrides.enableCheckUnreachable = false;
+  PolicyOverrides.enableConsumedAnalysis = false;
+  PolicyOverrides.enableThreadSafetyAnalysis = false;
+}
 
 static void flushDiagnostics(Sema &S, const sema::FunctionScopeInfo *fscope) {
   for (const auto &D : fscope->PossiblyUnreachableDiags)
@@ -2870,6 +2886,9 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings(
     AC.getCFG();
   }
 
+  // Clear any of our policy overrides.
+  clearOverrides();
+
   // Collect statistics about the CFG if it was built.
   if (S.CollectStats && AC.isCFGBuilt()) {
     ++NumFunctionsAnalyzed;

clang/lib/Sema/Sema.cpp

@@ -202,6 +202,43 @@ class SemaPPCallbacks : public PPCallbacks {
       break;
     }
   }
+  void PragmaDiagnostic(SourceLocation Loc, StringRef Namespace,
+                        diag::Severity Mapping, StringRef Str) override {
+    // If one of the analysis-based diagnostics was enabled while processing
+    // a function, we want to note it in the analysis-based warnings so they
+    // can be run at the end of the function body even if the analysis warnings
+    // are disabled at that point.
+    SmallVector<diag::kind, 256> GroupDiags;
+    diag::Flavor Flavor =
+        Str[1] == 'W' ? diag::Flavor::WarningOrError : diag::Flavor::Remark;
+    StringRef Group = Str.substr(2);
+
+    if (S->PP.getDiagnostics().getDiagnosticIDs()->getDiagnosticsInGroup(
+            Flavor, Group, GroupDiags))
+      return;
+
+    for (diag::kind K : GroupDiags) {
+      // Note: the cases in this switch should be kept in sync with the
+      // diagnostics in AnalysisBasedWarnings::getPolicyInEffectAt().
+      AnalysisBasedWarnings::Policy &Override =
+          S->AnalysisWarnings.getPolicyOverrides();
+      switch (K) {
+      default: break;
+      case diag::warn_unreachable:
+      case diag::warn_unreachable_break:
+      case diag::warn_unreachable_return:
+      case diag::warn_unreachable_loop_increment:
+        Override.enableCheckUnreachable = true;
+        break;
+      case diag::warn_double_lock:
+        Override.enableThreadSafetyAnalysis = true;
+        break;
+      case diag::warn_use_in_invalid_state:
+        Override.enableConsumedAnalysis = true;
+        break;
+      }
+    }
+  }
 };
 
 } // end namespace sema

clang/lib/Sema/SemaDecl.cpp

@@ -16150,7 +16150,13 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,
   if (FSI->UsesFPIntrin && FD && !FD->hasAttr<StrictFPAttr>())
     FD->addAttr(StrictFPAttr::CreateImplicit(Context));
 
-  sema::AnalysisBasedWarnings::Policy WP = AnalysisWarnings.getDefaultPolicy();
+  SourceLocation AnalysisLoc;
+  if (Body)
+    AnalysisLoc = Body->getEndLoc();
+  else if (FD)
+    AnalysisLoc = FD->getEndLoc();
+  sema::AnalysisBasedWarnings::Policy WP =
+      AnalysisWarnings.getPolicyInEffectAt(AnalysisLoc);
   sema::AnalysisBasedWarnings::Policy *ActivePolicy = nullptr;
 
   // If we skip function body, we can't tell if a function is a coroutine.

clang/lib/Sema/SemaExpr.cpp

@@ -16597,7 +16597,8 @@ 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.getDefaultPolicy();
+  AnalysisBasedWarnings::Policy WP =
+      AnalysisWarnings.getPolicyInEffectAt(Body->getEndLoc());
   PoppedFunctionScopePtr ScopeRAII = PopFunctionScopeInfo(&WP, BD, BlockTy);
 
   BlockExpr *Result = new (Context)

clang/test/Analysis/pragma-diag-control.cpp

@@ -0,0 +1,83 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Werror=unreachable-code-aggressive %s
+
+// Test that analysis-based warnings honor #pragma diagnostic controls.
+
+struct [[clang::consumable(unconsumed)]] Linear {
+  [[clang::return_typestate(unconsumed)]]
+  Linear() {}
+  [[clang::callable_when(consumed)]]
+  ~Linear() {}
+};
+
+int a() {	
+  Linear l;
+  return 0; // No -Wconsumed diagnostic, analysis is not enabled.
+  return 1; // expected-error {{'return' will never be executed}}
+}
+
+#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}}
+  return 1;  // expected-error {{'return' will never be executed}}
+}
+#pragma clang diagnostic pop
+
+int c() {
+#pragma clang diagnostic push
+#pragma clang diagnostic error "-Wconsumed"
+  Linear l;
+  return 0; // expected-error {{invalid invocation of method '~Linear' on object 'l' while it is in the 'unconsumed' state}}
+  return 1; // expected-error {{'return' will never be executed}}
+#pragma clang diagnostic pop
+}
+
+int d() {
+#pragma clang diagnostic push
+#pragma clang diagnostic error "-Wconsumed"
+#pragma clang diagnostic ignored "-Wunreachable-code-aggressive"
+  Linear l;
+  return 0; // expected-error {{invalid invocation of method '~Linear' on object 'l' while it is in the 'unconsumed' state}}
+  return 1; // Diagnostic is ignored
+}
+#pragma clang diagnostic pop
+
+int e() {
+#pragma clang diagnostic push
+#pragma clang diagnostic error "-Wconsumed"
+#pragma clang diagnostic ignored "-Wunreachable-code-aggressive"
+  Linear l;
+  return 0; // expected-error {{invalid invocation of method '~Linear' on object 'l' while it is in the 'unconsumed' state}}
+  return 1; // Diagnostic is ignored
+#pragma clang diagnostic pop
+}
+
+int f() {
+  Linear l;
+  return 0; // No -Wconsumed diagnostic, analysis is not enabled
+  return 1; // expected-error {{'return' will never be executed}}
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunreachable-code-aggressive"
+}
+#pragma clang diagnostic pop	
+
+int g() {
+  Linear l;
+  return 0; // No -Wconsumed diagnostic, the diagnostic generated at } is not enabled on this line.
+  return 1; // expected-error {{'return' will never be executed}}
+#pragma clang diagnostic push
+#pragma clang diagnostic warning "-Wconsumed"
+}
+#pragma clang diagnostic pop
+
+int h() {
+#pragma clang diagnostic push
+#pragma clang diagnostic error "-Wconsumed"
+#pragma clang diagnostic ignored "-Wunreachable-code-aggressive"
+#pragma clang diagnostic pop
+
+  Linear l;
+  return 0; // No -Wconsumed diagnostic, the diagnostic generated at } is not enabled on this line.
+  return 1; // expected-error {{'return' will never be executed}}
+}