https://github.com/steakhal created https://github.com/llvm/llvm-project/pull/232300
Users sometimes enable -Weverything and get flooded by the results of the -Wlifetime-safety-suggestions diagnostics. Require these suggestions to be requested explicitly instead, e.g. via -Wlifetime-safety-suggestions or one of its subgroups. I'm not happy with what I recommend here because it's not a real solution - rather just a bandaid for users who enable -Weverything (sometimes with -Werror). --- Under -Weverything, a suggestion is only emitted if the user set a mapping for it explicitly. This is checked both when deciding whether to compute the suggestions at all, and at each emission site, so that enabling one subgroup does not let -Weverything enable the other suggestion groups. Limitation: The explicit-mapping check inspects the current diagnostic state via `DiagnosticsEngine::getDiagnosticMappings()`, not the state at the diagnostic's location. So a suggestion group enabled only within a `#pragma clang diagnostic push`/`pop` region may still be suppressed under -Weverything, especially with the TU-level analysis which runs at the end of the translation unit. Command-line flags work as expected. Authored-By: claude From f97a9d6f068cd0378d9490c311d9eed346f4ab8e Mon Sep 17 00:00:00 2001 From: Balazs Benics <[email protected]> Date: Thu, 15 Oct 2026 13:10:41 +0100 Subject: [PATCH] [LifetimeSafety] Don't enable lifetime suggestions via -Weverything Users sometimes enable -Weverything and get flooded by the results of the -Wlifetime-safety-suggestions diagnostics. Require these suggestions to be requested explicitly instead, e.g. via -Wlifetime-safety-suggestions or one of its subgroups. I'm not happy with what I recommend here because it's not a real solution - rather just a bandaid for users who enable -Weverything (sometimes with -Werror). --- Under -Weverything, a suggestion is only emitted if the user set a mapping for it explicitly. This is checked both when deciding whether to compute the suggestions at all, and at each emission site, so that enabling one subgroup does not let -Weverything enable the other suggestion groups. Limitation: The explicit-mapping check inspects the current diagnostic state via `DiagnosticsEngine::getDiagnosticMappings()`, not the state at the diagnostic's location. So a suggestion group enabled only within a `#pragma clang diagnostic push`/`pop` region may still be suppressed under -Weverything, especially with the TU-level analysis which runs at the end of the translation unit. Command-line flags work as expected. Authored-By: claude --- clang/docs/ReleaseNotes.md | 5 ++ clang/lib/Sema/SemaLifetimeSafety.h | 25 ++++++++- .../annotation-suggestions-weverything.cpp | 56 +++++++++++++++++++ 3 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 clang/test/Sema/LifetimeSafety/annotation-suggestions-weverything.cpp diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index b6658dc7d81afb..f49c82319c5d09 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -510,6 +510,11 @@ features cannot lower the translation-unit ABI level; - Lifetime safety analysis is now enabled for C by default. The `-fexperimental-lifetime-safety-c` flag is renamed to `-flifetime-safety-c`. Use `-fno-lifetime-safety-c` to disable it. +- The lifetime annotation suggestions (`-Wlifetime-safety-suggestions` and its + subgroups) are no longer enabled by `-Weverything`. Users sometimes enable + `-Weverything` and would get flooded by the results of these suggestions. They + now have to be requested explicitly, e.g. via `-Wlifetime-safety-suggestions`. + - Improved `-Wassign-enum` performance by caching enum enumerator values. (#GH176454) - Fixed a false negative in `-Warray-bounds` where the warning was suppressed diff --git a/clang/lib/Sema/SemaLifetimeSafety.h b/clang/lib/Sema/SemaLifetimeSafety.h index 3db298b6fec85c..dd083d5860a21f 100644 --- a/clang/lib/Sema/SemaLifetimeSafety.h +++ b/clang/lib/Sema/SemaLifetimeSafety.h @@ -77,6 +77,25 @@ inline bool ShouldCheckFieldOriginEscape(Sema &S, const Decl *D) { diag::warn_lifetime_safety_field_origin_escape, D->getBeginLoc()); } +/// Returns true if the annotation suggestion \p DiagID should be emitted at +/// \p Loc. +/// Unlike other warnings, the suggestions are not enabled by -Weverything +/// alone; they have to be requested explicitly. +/// Users sometimes enable -Weverything and would otherwise get flooded by the +/// results of the lifetime-safety suggestions. +/// FIXME: Ideally this should be handled in a more generic way. +inline bool isSuggestionEnabled(DiagnosticsEngine &Diags, unsigned DiagID, + SourceLocation Loc) { + if (Diags.isIgnored(DiagID, Loc)) + return false; + if (!Diags.getEnableAllWarnings()) + return true; + for (const auto &[ID, Mapping] : Diags.getDiagnosticMappings()) + if (ID == DiagID) + return Mapping.isUser(); + return false; +} + inline bool ShouldSuggestLifetimeAnnotations(Sema &S, const Decl *D) { DiagnosticsEngine &Diags = S.getDiagnostics(); constexpr unsigned DiagIDs[] = { @@ -87,7 +106,7 @@ inline bool ShouldSuggestLifetimeAnnotations(Sema &S, const Decl *D) { diag::warn_lifetime_safety_intra_tu_this_suggestion, diag::warn_lifetime_safety_cross_tu_this_suggestion}; for (unsigned DiagID : DiagIDs) - if (!Diags.isIgnored(DiagID, D->getBeginLoc())) + if (isSuggestionEnabled(Diags, DiagID, D->getBeginLoc())) return true; return false; } @@ -349,6 +368,8 @@ class LifetimeSafetySemaHelperImpl : public LifetimeSafetySemaHelper { : diag::warn_lifetime_safety_intra_tu_param_suggestion; auto [InsertionPoint, FixItText] = getLifetimeBoundFixIt(ParmToAnnotate); + if (!isSuggestionEnabled(S.getDiagnostics(), DiagID, InsertionPoint)) + return; S.Diag(InsertionPoint, DiagID) << ParmToAnnotate->getSourceRange() @@ -455,6 +476,8 @@ class LifetimeSafetySemaHelperImpl : public LifetimeSafetySemaHelper { : diag::warn_lifetime_safety_intra_tu_this_suggestion; auto [InsertionPoint, FixItText] = getLifetimeBoundFixIt(MD); + if (!isSuggestionEnabled(S.getDiagnostics(), DiagID, InsertionPoint)) + return; S.Diag(InsertionPoint, DiagID) << MD->getNameInfo().getSourceRange() diff --git a/clang/test/Sema/LifetimeSafety/annotation-suggestions-weverything.cpp b/clang/test/Sema/LifetimeSafety/annotation-suggestions-weverything.cpp new file mode 100644 index 00000000000000..6d3a4abf3a7df8 --- /dev/null +++ b/clang/test/Sema/LifetimeSafety/annotation-suggestions-weverything.cpp @@ -0,0 +1,56 @@ +// The lifetime annotation suggestions are not enabled by -Weverything alone. +// +// RUN: %clang_cc1 -fsyntax-only -std=c++17 -flifetime-safety-inference \ +// RUN: -fexperimental-lifetime-safety-tu-analysis -Weverything %s 2>&1 | \ +// RUN: FileCheck %s --check-prefix=NONE --implicit-check-not="should be marked" +// +// RUN: %clang_cc1 -fsyntax-only -std=c++17 -flifetime-safety-inference \ +// RUN: -fexperimental-lifetime-safety-tu-analysis -Weverything \ +// RUN: -Wlifetime-safety-suggestions %s 2>&1 | \ +// RUN: FileCheck %s --check-prefixes=PARAM,CTOR \ +// RUN: --implicit-check-not="should be marked" +// +// RUN: %clang_cc1 -fsyntax-only -std=c++17 -flifetime-safety-inference \ +// RUN: -fexperimental-lifetime-safety-tu-analysis \ +// RUN: -Wlifetime-safety-suggestions -Weverything %s 2>&1 | \ +// RUN: FileCheck %s --check-prefixes=PARAM,CTOR \ +// RUN: --implicit-check-not="should be marked" +// +// Enabling only a subgroup must not let -Weverything enable the rest. +// RUN: %clang_cc1 -fsyntax-only -std=c++17 -flifetime-safety-inference \ +// RUN: -fexperimental-lifetime-safety-tu-analysis -Weverything \ +// RUN: -Wlifetime-safety-intra-tu-constructor-suggestions %s 2>&1 | \ +// RUN: FileCheck %s --check-prefix=CTOR --implicit-check-not="should be marked" +// +// RUN: not %clang_cc1 -fsyntax-only -std=c++17 -flifetime-safety-inference \ +// RUN: -fexperimental-lifetime-safety-tu-analysis -Weverything \ +// RUN: -Werror=lifetime-safety-intra-tu-suggestions %s 2>&1 | \ +// RUN: FileCheck %s --check-prefix=ERROR --implicit-check-not="should be marked" + +// NONE-NOT: should be marked + +struct View; + +struct [[gsl::Owner]] MyObj { + int id; + MyObj(); + ~MyObj(); +}; + +struct [[gsl::Pointer()]] View { + View(const MyObj &); + View(); +}; + +View return_view(View a) { + // PARAM: :[[@LINE-1]]:24: warning: parameter in intra-TU function should be marked {{\[\[}}clang::lifetimebound]] [-Wlifetime-safety-intra-tu-suggestions] + // ERROR: :[[@LINE-2]]:24: error: parameter in intra-TU function should be marked {{\[\[}}clang::lifetimebound]] [-Werror,-Wlifetime-safety-intra-tu-suggestions] + return a; +} + +struct CaptureRefToView { + View v; + CaptureRefToView(const MyObj &obj) : v(obj) {} + // CTOR: :[[@LINE-1]]:36: warning: parameter in intra-TU constructor should be marked {{\[\[}}clang::lifetimebound]] [-Wlifetime-safety-intra-tu-constructor-suggestions] + // ERROR: :[[@LINE-2]]:36: error: parameter in intra-TU constructor should be marked {{\[\[}}clang::lifetimebound]] [-Werror,-Wlifetime-safety-intra-tu-constructor-suggestions] +}; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
