Control analysis-based diagnostics with #pragma

clang/docs/ReleaseNotes.rst

@@ -395,6 +395,9 @@ 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
+
 Improvements to Clang's time-trace
 ----------------------------------
 

clang/docs/UsersManual.rst

@@ -1202,6 +1202,30 @@ Clang also allows you to push and pop the current warning state. This is
 particularly useful when writing a header file that will be compiled by
 other people, because you don't know what warning flags they build with.
 
+Note that the following diagnostic groups, which are ones based on analyzing
+the control flow graph for a function, require the diagnostic group to be
+enabled at the end of the function body (after the closing ``}``) in order to
+run the analysis, in addition to requiring the diagnostic group to be enabled
+at the line being diagnosed:
+
+  * ``-Wconsumed``
+  * ``-Wthread-safety-analysis``
+  * ``-Wunreachable-code``, ``-Wunreachable-code-aggressive``, or warnings
+    controlled by either of those flags
+
+thus, it is generally better for a ``push`` and ``pop`` pair of pragmas
+controlling behavior for an entire function be placed outside of the function
+body rather than within it. e.g.,
+
+.. code-block:: c
+
+  #pragma clang diagnostic push
+  #pragma clang diagnostic warning "-Wwhatever"
+  int d() {
+    // Better to put the pragmas outside of the function rather than within it.
+  }
+  #pragma clang diagnostic pop
+
 In the below example :option:`-Wextra-tokens` is ignored for only a single line
 of code, after which the diagnostics return to whatever state had previously
 existed.

clang/include/clang/Sema/AnalysisBasedWarnings.h

@@ -49,7 +49,6 @@ class AnalysisBasedWarnings {
 
 private:
   Sema &S;
-  Policy DefaultPolicy;
 
   class InterProceduralData;
   std::unique_ptr<InterProceduralData> IPData;
@@ -103,7 +102,8 @@ 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);
 
   void PrintStats() const;
 };

clang/lib/Sema/AnalysisBasedWarnings.cpp

@@ -2493,8 +2493,9 @@ class sema::AnalysisBasedWarnings::InterProceduralData {
   CalledOnceInterProceduralData CalledOnceData;
 };
 
-static unsigned isEnabled(DiagnosticsEngine &D, unsigned diag) {
-  return (unsigned)!D.isIgnored(diag, SourceLocation());
+static bool isEnabled(DiagnosticsEngine &D, unsigned diag,
+                          SourceLocation Loc) {
+  return !D.isIgnored(diag, Loc);
 }
 
 sema::AnalysisBasedWarnings::AnalysisBasedWarnings(Sema &s)
@@ -2504,24 +2505,28 @@ 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);
+  P.enableCheckUnreachable = isEnabled(D, warn_unreachable, Loc) ||
+                             isEnabled(D, warn_unreachable_break, Loc) ||
+                             isEnabled(D, warn_unreachable_return, Loc) ||
+                             isEnabled(D, warn_unreachable_loop_increment, Loc);
 
-  DefaultPolicy.enableThreadSafetyAnalysis = isEnabled(D, warn_double_lock);
+  P.enableThreadSafetyAnalysis = isEnabled(D, warn_double_lock, Loc);
 
-  DefaultPolicy.enableConsumedAnalysis =
-      isEnabled(D, warn_use_in_invalid_state);
+  P.enableConsumedAnalysis = isEnabled(D, warn_use_in_invalid_state, Loc);
+  return P;
 }
 
-// We need this here for unique_ptr with forward declared class.
-sema::AnalysisBasedWarnings::~AnalysisBasedWarnings() = default;
-
 static void flushDiagnostics(Sema &S, const sema::FunctionScopeInfo *fscope) {
   for (const auto &D : fscope->PossiblyUnreachableDiags)
     S.Diag(D.Loc, D.PD);

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

@@ -16574,7 +16574,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,77 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Werror=unreachable-code-aggressive %s
+
+// Test that analysis-based warnings honor #pragma diagnostic controls. These
+// diagnostics are triggered at the end of a function body, so the pragma needs
+// to be enabled through to the closing curly brace in order for the diagnostic
+// to be emitted.
+
+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; // No -Wconsumed diagnostic, analysis is disabled before the closing brace
+  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;
+  return 1;
+  // Both diagnostics are ignored because analysis is disabled before the
+  // closing brace.
+#pragma clang diagnostic pop
+}
+
+int f() {
+  Linear l;
+  return 0; // No -Wconsumed diagnostic, analysis is not enabled at } so it never runs to produce the diagnostic
+  return 1; // Diagnostic ignores because it was disabled at the }
+#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