llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-analysis Author: Aaron Ballman (AaronBallman) <details> <summary>Changes</summary> 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. However, the behavior requires some explanation (which is why the documentation is being updated). Analysis-based warnings run at the end of a function body, so the diagnostic needs to be enabled after the closing } for the function in order to run the analysis at all. However, if the analysis emits a diagnostic for line in the function, the diagnostic still needs to be enabled on that specific line in order to be shown to the user. Fixes #<!-- -->42199 --- Full diff: https://github.com/llvm/llvm-project/pull/136323.diff 7 Files Affected: - (modified) clang/docs/ReleaseNotes.rst (+3) - (modified) clang/docs/UsersManual.rst (+24) - (modified) clang/include/clang/Sema/AnalysisBasedWarnings.h (+2-2) - (modified) clang/lib/Sema/AnalysisBasedWarnings.cpp (+17-12) - (modified) clang/lib/Sema/SemaDecl.cpp (+7-1) - (modified) clang/lib/Sema/SemaExpr.cpp (+2-1) - (added) clang/test/Analysis/pragma-diag-control.cpp (+77) ``````````diff diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index f5cd1fbeabcfe..8d139123b47ce 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/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 ---------------------------------- diff --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst index 69256527f40c9..0ba4e2dedd4eb 100644 --- a/clang/docs/UsersManual.rst +++ b/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. diff --git a/clang/include/clang/Sema/AnalysisBasedWarnings.h b/clang/include/clang/Sema/AnalysisBasedWarnings.h index aafe227b84084..49023863f4503 100644 --- a/clang/include/clang/Sema/AnalysisBasedWarnings.h +++ b/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; }; diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 34045a7274021..72962acee66d3 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/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); diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 46933c5c43168..d28a2107d58a9 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/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. diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 9cd1a7e45fb64..a9f353be07ad2 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/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) diff --git a/clang/test/Analysis/pragma-diag-control.cpp b/clang/test/Analysis/pragma-diag-control.cpp new file mode 100644 index 0000000000000..c41793c15bbcc --- /dev/null +++ b/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 `````````` </details> https://github.com/llvm/llvm-project/pull/136323 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
