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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
