Author: NeKon69 Date: 2026-05-24T19:02:56+02:00 New Revision: 69a5cf515fd317bcf918e48de9137dd8549870c5
URL: https://github.com/llvm/llvm-project/commit/69a5cf515fd317bcf918e48de9137dd8549870c5 DIFF: https://github.com/llvm/llvm-project/commit/69a5cf515fd317bcf918e48de9137dd8549870c5.diff LOG: [LifetimeSafety] Extend suggestions for `lifetimebound` to also warn on canonical declarations (#198784) With this patch, we suggest adding the `clang::lifetimebound` attribute on the canonical declaration and on the earliest redeclaration in each other file, preserving diagnostics for declarations visible from other translation units while avoiding duplicate suggestions within the same file. Fixes #198624 Fixes #198628 Added: Modified: clang/lib/Analysis/LifetimeSafety/Checker.cpp clang/test/Sema/warn-lifetime-safety-fixits.cpp clang/test/Sema/warn-lifetime-safety-misplaced-lifetimebound-cross-tu.cpp clang/test/Sema/warn-lifetime-safety-misplaced-lifetimebound-intra-tu.cpp clang/test/Sema/warn-lifetime-safety-suggestions.cpp Removed: ################################################################################ diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp index d6a15139aa4ea..53899251b9643 100644 --- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp @@ -316,103 +316,83 @@ class LifetimeChecker { } } - std::pair<const FunctionDecl *, WarningScope> - getCanonicalFunctionDeclForAttr(const FunctionDecl *FDef) { + // Returns declarations that should be annotated with lifetime attributes + // in order to annotate FDef: the canonical declaration and the earliest + // redeclarations in each other file. This defines the placement policy for + // lifetime annotations. Each target is paired with its corresponding warning + // scope. + llvm::SmallVector<std::pair<const FunctionDecl *, WarningScope>, 2> + getTargetDeclsForAttr(const FunctionDecl *FDef) { if (!FDef) - return {nullptr, WarningScope::IntraTU}; + return {}; assert(FDef->isThisDeclarationADefinition() && "Expected FunctionDecl to be a definition"); const auto &SM = FDef->getASTContext().getSourceManager(); - const FileID DefFile = - SM.getFileID(SM.getExpansionLoc(FDef->getLocation())); - const FunctionDecl *CanonicalDecl = FDef->getCanonicalDecl(); - WarningScope Scope = WarningScope::IntraTU; - - Scope = SM.getFileID(SM.getExpansionLoc(CanonicalDecl->getLocation())) != - DefFile - ? WarningScope::CrossTU - : WarningScope::IntraTU; - - return {CanonicalDecl, Scope}; - } - std::pair<const CXXMethodDecl *, WarningScope> - getCanonicalDeclForAttr(const CXXMethodDecl *MDef) { - auto [CanonicalFDecl, Scope] = getCanonicalFunctionDeclForAttr(MDef); - return {cast_or_null<CXXMethodDecl>(CanonicalFDecl), Scope}; - } + auto GetFile = [&SM](const FunctionDecl *FD) { + return SM.getFileID(SM.getExpansionLoc(FD->getLocation())); + }; - std::pair<const ParmVarDecl *, WarningScope> - getCanonicalDeclForAttr(const FunctionDecl *FDef, const ParmVarDecl *PVDDef) { - auto [CanonicalFDecl, Scope] = getCanonicalFunctionDeclForAttr(FDef); - if (!CanonicalFDecl) - return {nullptr, Scope}; - return {CanonicalFDecl->getParamDecl(PVDDef->getFunctionScopeIndex()), - Scope}; - } + const FileID DefFile = GetFile(FDef); + const FunctionDecl *CanonicalDecl = FDef->getCanonicalDecl(); + llvm::SmallVector<std::pair<const FunctionDecl *, WarningScope>, 2> Targets{ + {CanonicalDecl, GetFile(CanonicalDecl) == DefFile + ? WarningScope::IntraTU + : WarningScope::CrossTU}}; + + // Find the earliest redeclaration in each file other than the definition + // file. + auto AddCrossTUDecl = [&](const FunctionDecl *FD) { + FileID File = GetFile(FD); + if (File == DefFile) + return; + for (auto [SeenFD, _] : Targets) + if (GetFile(SeenFD) == File) + return; + Targets.push_back({FD, WarningScope::CrossTU}); + }; - /// Returns the declaration of a function that is visible across translation - /// units, if such a declaration exists and is diff erent from the definition. - static const FunctionDecl *getCrossTUDecl(const FunctionDecl &FD, - SourceManager &SM) { - if (!FD.isExternallyVisible()) - return nullptr; - const FileID DefinitionFile = SM.getFileID(FD.getLocation()); - for (const FunctionDecl *Redecl : FD.redecls()) - if (SM.getFileID(Redecl->getLocation()) != DefinitionFile) - return Redecl; - - return nullptr; - } + // We iterate in reverse order (from most recent to oldest) to find + // the first declaration in each file. + for (const FunctionDecl *Redecl : + llvm::reverse(llvm::to_vector(FDef->redecls()))) + AddCrossTUDecl(Redecl); - static const FunctionDecl *getCrossTUDecl(const ParmVarDecl &PVD, - SourceManager &SM) { - if (const auto *FD = dyn_cast<FunctionDecl>(PVD.getDeclContext())) - return getCrossTUDecl(*FD, SM); - return nullptr; + return Targets; } - static void suggestWithScopeForParmVar(LifetimeSafetySemaHelper *SemaHelper, - const ParmVarDecl *PVD, - SourceManager &SM, - EscapingTarget EscapeTarget) { + void suggestWithScopeForParmVar(const ParmVarDecl *PVD, + EscapingTarget EscapeTarget) { if (llvm::isa<const VarDecl *>(EscapeTarget)) return; - if (const FunctionDecl *CrossTUDecl = getCrossTUDecl(*PVD, SM)) - SemaHelper->suggestLifetimeboundToParmVar( - WarningScope::CrossTU, - CrossTUDecl->getParamDecl(PVD->getFunctionScopeIndex()), - EscapeTarget); - else - SemaHelper->suggestLifetimeboundToParmVar(WarningScope::IntraTU, PVD, + for (auto [Decl, Scope] : getTargetDeclsForAttr(cast<FunctionDecl>(FD))) { + const auto *ParmToAnnotate = + Decl->getParamDecl(PVD->getFunctionScopeIndex()); + SemaHelper->suggestLifetimeboundToParmVar(Scope, ParmToAnnotate, EscapeTarget); + } } - static void - suggestWithScopeForImplicitThis(LifetimeSafetySemaHelper *SemaHelper, - const CXXMethodDecl *MD, SourceManager &SM, - const Expr *EscapeExpr) { - if (const FunctionDecl *CrossTUDecl = getCrossTUDecl(*MD, SM)) + void suggestWithScopeForImplicitThis(const CXXMethodDecl *MD, + const Expr *EscapeExpr) { + for (auto [Decl, Scope] : getTargetDeclsForAttr(MD)) { SemaHelper->suggestLifetimeboundToImplicitThis( - WarningScope::CrossTU, cast<CXXMethodDecl>(CrossTUDecl), EscapeExpr); - else - SemaHelper->suggestLifetimeboundToImplicitThis(WarningScope::IntraTU, MD, - EscapeExpr); + Scope, cast<CXXMethodDecl>(Decl), EscapeExpr); + } } void suggestAnnotations() { if (!SemaHelper) return; - SourceManager &SM = AST.getSourceManager(); for (auto [Target, EscapeTarget] : AnnotationWarningsMap) { if (const auto *PVD = Target.dyn_cast<const ParmVarDecl *>()) - suggestWithScopeForParmVar(SemaHelper, PVD, SM, EscapeTarget); + suggestWithScopeForParmVar(PVD, EscapeTarget); else if (const auto *MD = Target.dyn_cast<const CXXMethodDecl *>()) { if (const auto *EscapeExpr = EscapeTarget.dyn_cast<const Expr *>()) - suggestWithScopeForImplicitThis(SemaHelper, MD, SM, EscapeExpr); + suggestWithScopeForImplicitThis(MD, EscapeExpr); else llvm_unreachable("Implicit this can only escape via Expr (return)"); } @@ -458,13 +438,17 @@ class LifetimeChecker { const FunctionDecl *FDef = dyn_cast<FunctionDecl>(FD); if (!FDef) return; + + auto TargetDecls = getTargetDeclsForAttr(FDef); // Check if implicit 'this' has lifetimebound on definition but not on // declaration. if (const auto *MDef = dyn_cast<CXXMethodDecl>(FDef); MDef && getDirectImplicitObjectLifetimeBoundAttr(MDef)) - if (auto [MDecl, Scope] = getCanonicalDeclForAttr(MDef); - MDecl && !getDirectImplicitObjectLifetimeBoundAttr(MDecl)) - SemaHelper->reportMisplacedLifetimebound(Scope, MDef, MDecl); + for (auto [Decl, Scope] : TargetDecls) { + const auto *MDecl = cast<CXXMethodDecl>(Decl); + if (!getDirectImplicitObjectLifetimeBoundAttr(MDecl)) + SemaHelper->reportMisplacedLifetimebound(Scope, MDef, MDecl); + } // Check each parameter for explicit lifetimebound on definition but not on // declaration. @@ -472,9 +456,11 @@ class LifetimeChecker { const auto *Attr = PDef->getAttr<LifetimeBoundAttr>(); if (!Attr || Attr->isImplicit()) continue; - if (auto [PDecl, Scope] = getCanonicalDeclForAttr(FDef, PDef); - PDecl && !PDecl->hasAttr<LifetimeBoundAttr>()) - SemaHelper->reportMisplacedLifetimebound(Scope, PDef, PDecl); + for (auto [Decl, Scope] : TargetDecls) { + const auto *PDecl = Decl->getParamDecl(PDef->getFunctionScopeIndex()); + if (!PDecl->hasAttr<LifetimeBoundAttr>()) + SemaHelper->reportMisplacedLifetimebound(Scope, PDef, PDecl); + } } } diff --git a/clang/test/Sema/warn-lifetime-safety-fixits.cpp b/clang/test/Sema/warn-lifetime-safety-fixits.cpp index 88d8bd379de8b..d9c7e8d3f0519 100644 --- a/clang/test/Sema/warn-lifetime-safety-fixits.cpp +++ b/clang/test/Sema/warn-lifetime-safety-fixits.cpp @@ -69,12 +69,11 @@ int *arr_default(int a[2] = nullptr) { return a; } -// FIXME: Iterate over redecls and add [[clang::lifetimebound]] View multi_decl(View a); +// CHECK: :[[@LINE-1]]:17: warning: parameter in intra-TU function should be marked +// CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:23-[[@LINE-2]]:23}:" {{\[\[}}clang::lifetimebound]]" View multi_decl(View a); View multi_decl(View a) { - // CHECK: :[[@LINE-1]]:17: warning: parameter in intra-TU function should be marked - // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:23-[[@LINE-2]]:23}:" {{\[\[}}clang::lifetimebound]]" return a; } @@ -145,10 +144,10 @@ struct OutOfLine { OutOfLine() {} ~OutOfLine() {} const OutOfLine &get() const; + // CHECK: :[[@LINE-1]]:31: warning: implicit this in intra-TU function should be marked + // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:31-[[@LINE-2]]:31}:" {{\[\[}}clang::lifetimebound]]" }; const OutOfLine &OutOfLine::get() const { - // CHECK: :[[@LINE-1]]:40: warning: implicit this in intra-TU function should be marked - // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:40-[[@LINE-2]]:40}:" {{\[\[}}clang::lifetimebound]]" return *this; } diff --git a/clang/test/Sema/warn-lifetime-safety-misplaced-lifetimebound-cross-tu.cpp b/clang/test/Sema/warn-lifetime-safety-misplaced-lifetimebound-cross-tu.cpp index 5fd49023a042a..583fba6988573 100644 --- a/clang/test/Sema/warn-lifetime-safety-misplaced-lifetimebound-cross-tu.cpp +++ b/clang/test/Sema/warn-lifetime-safety-misplaced-lifetimebound-cross-tu.cpp @@ -14,8 +14,18 @@ struct HeaderS { HeaderObj &header_this(); // expected-warning {{'lifetimebound' attribute on this definition is not visible to callers in other translation units; add it to the declaration instead}} }; +//--- cross_1.h +struct HeaderObj; +HeaderObj &multi_header_param(HeaderObj &obj); // expected-warning {{'lifetimebound' attribute on this definition is not visible to callers in other translation units; add it to the declaration instead}} + +//--- cross_2.h +struct HeaderObj; +HeaderObj &multi_header_param(HeaderObj &obj); // expected-warning {{'lifetimebound' attribute on this definition is not visible to callers in other translation units; add it to the declaration instead}} + //--- cross.cpp #include "cross.h" +#include "cross_1.h" +#include "cross_2.h" HeaderObj &header_param(HeaderObj &obj [[clang::lifetimebound]]) { // expected-note {{'lifetimebound' attribute appears here on the definition}} return obj; @@ -24,3 +34,7 @@ HeaderObj &header_param(HeaderObj &obj [[clang::lifetimebound]]) { // expected-n HeaderObj &HeaderS::header_this() [[clang::lifetimebound]] { // expected-note {{'lifetimebound' attribute appears here on the definition}} return data; } + +HeaderObj &multi_header_param(HeaderObj &obj [[clang::lifetimebound]]) { // expected-note 2 {{'lifetimebound' attribute appears here on the definition}} + return obj; +} diff --git a/clang/test/Sema/warn-lifetime-safety-misplaced-lifetimebound-intra-tu.cpp b/clang/test/Sema/warn-lifetime-safety-misplaced-lifetimebound-intra-tu.cpp index caf805cea5416..998e92b467819 100644 --- a/clang/test/Sema/warn-lifetime-safety-misplaced-lifetimebound-intra-tu.cpp +++ b/clang/test/Sema/warn-lifetime-safety-misplaced-lifetimebound-intra-tu.cpp @@ -9,6 +9,12 @@ MyObj &free_param(MyObj &obj [[clang::lifetimebound]]) { // expected-note {{'lif return obj; } +MyObj &earliest_redecl_param(MyObj &obj); // expected-warning {{'lifetimebound' attribute on this definition is not visible to callers before the definition; add it to the declaration instead}} +MyObj &earliest_redecl_param(MyObj &obj); +MyObj &earliest_redecl_param(MyObj &obj [[clang::lifetimebound]]) { // expected-note {{'lifetimebound' attribute appears here on the definition}} + return obj; +} + struct S { MyObj data; const MyObj &implicit_this_only(); // expected-warning {{'lifetimebound' attribute on this definition is not visible to callers before the definition; add it to the declaration instead}} diff --git a/clang/test/Sema/warn-lifetime-safety-suggestions.cpp b/clang/test/Sema/warn-lifetime-safety-suggestions.cpp index 973c610eb58ab..d9aa308986913 100644 --- a/clang/test/Sema/warn-lifetime-safety-suggestions.cpp +++ b/clang/test/Sema/warn-lifetime-safety-suggestions.cpp @@ -31,6 +31,8 @@ struct [[gsl::Pointer()]] View { View definition_before_header(View a); // expected-warning {{parameter in cross-TU function should be marked [[clang::lifetimebound]]}} +View redeclared_before_header_include(View a); // expected-warning {{parameter in cross-TU function should be marked [[clang::lifetimebound]]}} + View return_view_directly(View a); // expected-warning {{parameter in cross-TU function should be marked [[clang::lifetimebound]]}} View conditional_return_view( @@ -46,8 +48,8 @@ inline View inline_header_return_view(View a) { // expected-warning {{parameter return a; // expected-note {{param returned here}} } -View redeclared_in_header(View a); -inline View redeclared_in_header(View a) { // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}} +View redeclared_in_header(View a); // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}} +inline View redeclared_in_header(View a) { return a; // expected-note {{param returned here}} } @@ -65,15 +67,48 @@ struct ReturnThisPointer { #endif // TEST_HEADER_H +//--- test_redecls_header.h + +View earliest_decl_in_header(View a); // expected-warning {{parameter in cross-TU function should be marked [[clang::lifetimebound]]}} +View earliest_decl_in_header(View a); + +View multi_redecl_one_file(View a); // expected-warning {{parameter in cross-TU function should be marked [[clang::lifetimebound]]}} +View multi_redecl_one_file(View a); +View multi_redecl_one_file(View a); + +View source_and_header(View a); // expected-warning {{parameter in cross-TU function should be marked [[clang::lifetimebound]]}} + +//--- test_redecls_header_1.h + +View in_two_headers(View a); // expected-warning {{parameter in cross-TU function should be marked [[clang::lifetimebound]]}} + +//--- test_redecls_header_2.h + +View in_two_headers(View a); // expected-warning {{parameter in cross-TU function should be marked [[clang::lifetimebound]]}} + //--- test_source.cpp +struct View; +View redeclared_before_header_include(View a); // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}} +View source_and_header(View a); // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}} + #include "test_header.h" +#include "test_redecls_header.h" +#include "test_redecls_header_1.h" +#include "test_redecls_header_2.h" #include "Inputs/lifetime-analysis.h" +View earliest_decl_in_source(View a); // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}} +View earliest_decl_in_source(View a); + View definition_before_header(View a) { return a; // expected-note {{param returned here}} } +View redeclared_before_header_include(View a) { + return a; // expected-note 2 {{param returned here}} +} + View return_view_directly(View a) { return a; // expected-note {{param returned here}} } @@ -103,6 +138,26 @@ MyObj& return_unnamed_ref(MyObj& a, bool c) { return a; // expected-note {{param returned here}} } +View earliest_decl_in_header(View a) { + return a; // expected-note {{param returned here}} +} + +View earliest_decl_in_source(View a) { + return a; // expected-note {{param returned here}} +} + +View multi_redecl_one_file(View a) { + return a; // expected-note {{param returned here}} +} + +View in_two_headers(View a) { + return a; // expected-note 2 {{param returned here}} +} + +View source_and_header(View a) { + return a; // expected-note 2 {{param returned here}} +} + MyObj& return_reference(MyObj& a, // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}} MyObj& b, // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}} bool c) { @@ -177,8 +232,8 @@ View reassigned_to_another_parameter( return a; // expected-note {{param returned here}} } -View intra_tu_func_redecl(View a); -View intra_tu_func_redecl(View a) { // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}}. +View intra_tu_func_redecl(View a); // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}}. +View intra_tu_func_redecl(View a) { return a; // expected-note {{param returned here}} } } @@ -282,25 +337,25 @@ MyObj* return_pointer_by_func(MyObj* a) { // expected-warning {{paramete } // namespace correct_order_inference namespace incorrect_order_inference_view { -View return_view_callee(View a); +View return_view_callee(View a); // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}}. View return_view_caller(View a) { // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}}. return return_view_callee(a); // expected-note {{param returned here}} } -View return_view_callee(View a) { // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}}. +View return_view_callee(View a) { return a; // expected-note {{param returned here}} } } // namespace incorrect_order_inference_view namespace incorrect_order_inference_object { -MyObj* return_object_callee(MyObj* a); +MyObj* return_object_callee(MyObj* a); // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}}. MyObj* return_object_caller(MyObj* a) { // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}}. return return_object_callee(a); // expected-note {{param returned here}} } -MyObj* return_object_callee(MyObj* a) { // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}}. +MyObj* return_object_callee(MyObj* a) { return a; // expected-note {{param returned here}} } } // namespace incorrect_order_inference_object @@ -322,13 +377,13 @@ View inference_top_level_return_stack_view() { } // namespace simple_annotation_inference namespace inference_in_order_with_redecls { -View inference_callee_return_identity(View a); -View inference_callee_return_identity(View a) { // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}}. +View inference_callee_return_identity(View a); // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}}. +View inference_callee_return_identity(View a) { return a; // expected-note {{param returned here}} } -View inference_caller_forwards_callee(View a); -View inference_caller_forwards_callee(View a) { // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}}. +View inference_caller_forwards_callee(View a); // expected-warning {{parameter in intra-TU function should be marked [[clang::lifetimebound]]}}. +View inference_caller_forwards_callee(View a) { return inference_callee_return_identity(a); // expected-note {{param returned here}} } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
