https://github.com/kashika0112 updated https://github.com/llvm/llvm-project/pull/176703
>From e49d9e1fa109dc83add2bae21e01e8a9316a87fd Mon Sep 17 00:00:00 2001 From: Kashika Akhouri <[email protected]> Date: Mon, 19 Jan 2026 08:03:57 +0000 Subject: [PATCH] Add suggestion and inference for implicit this --- .../Analysis/Analyses/LifetimeSafety/Facts.h | 2 +- .../Analyses/LifetimeSafety/LifetimeSafety.h | 4 + .../Analysis/Analyses/LifetimeSafety/Loans.h | 30 ++++-- .../Analyses/LifetimeSafety/Origins.h | 14 ++- .../clang/Basic/DiagnosticSemaKinds.td | 12 +++ clang/lib/Analysis/LifetimeSafety/Checker.cpp | 93 ++++++++++++------- .../LifetimeSafety/FactsGenerator.cpp | 10 +- .../LifetimeSafety/LifetimeAnnotations.cpp | 2 + clang/lib/Analysis/LifetimeSafety/Origins.cpp | 13 +++ clang/lib/Sema/AnalysisBasedWarnings.cpp | 24 +++++ .../Sema/warn-lifetime-safety-suggestions.cpp | 15 ++- 11 files changed, 169 insertions(+), 50 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h index a66925b7302ca..1bb34e6986857 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h @@ -195,7 +195,7 @@ class TestPointFact : public Fact { class FactManager { public: FactManager(const AnalysisDeclContext &AC, const CFG &Cfg) - : OriginMgr(AC.getASTContext()) { + : OriginMgr(AC.getASTContext(), AC.getDecl()) { BlockToFacts.resize(Cfg.getNumBlockIDs()); } diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h index 9c91355355233..2199ae023fb26 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h @@ -60,6 +60,10 @@ class LifetimeSafetyReporter { virtual void suggestAnnotation(SuggestionScope Scope, const ParmVarDecl *ParmToAnnotate, const Expr *EscapeExpr) {} + + // Suggests lifetime bound annotations for implicit this + virtual void suggestAnnotation(SuggestionScope Scope, const CXXMethodDecl *MD, + const Expr *EscapeExpr) {} }; /// The main entry point for the analysis. diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h index ee1634a6f5ea2..a366e3e811cbf 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Loans.h @@ -15,6 +15,7 @@ #define LLVM_CLANG_ANALYSIS_ANALYSES_LIFETIMESAFETY_LOANS_H #include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" #include "clang/AST/ExprCXX.h" #include "clang/Analysis/Analyses/LifetimeSafety/Utils.h" #include "llvm/Support/raw_ostream.h" @@ -100,10 +101,11 @@ class PathLoan : public Loan { static bool classof(const Loan *L) { return L->getKind() == Kind::Path; } }; -/// A placeholder loan held by a function parameter, representing a borrow from -/// the caller's scope. +/// A placeholder loan held by a function parameter or an implicit 'this' +/// object, representing a borrow from the caller's scope. /// -/// Created at function entry for each pointer or reference parameter with an +/// Created at function entry for each pointer or reference parameter or for +/// the implicit 'this' parameter of instance methods, with an /// origin. Unlike PathLoan, placeholder loans: /// - Have no IssueExpr (created at function entry, not at a borrow site) /// - Have no AccessPath (the borrowed object is not visible to the function) @@ -111,17 +113,27 @@ class PathLoan : public Loan { /// invalidations (e.g., vector::push_back) /// /// When a placeholder loan escapes the function (e.g., via return), it -/// indicates the parameter should be marked [[clang::lifetimebound]], enabling -/// lifetime annotation suggestions. +/// indicates the parameter or method should be marked [[clang::lifetimebound]], +/// enabling lifetime annotation suggestions. class PlaceholderLoan : public Loan { - /// The function parameter that holds this placeholder loan. - const ParmVarDecl *PVD; + /// The function parameter or method (representing 'this') that holds this + /// placeholder loan. + llvm::PointerUnion<const ParmVarDecl *, const CXXMethodDecl *> ParamOrMethod; public: PlaceholderLoan(LoanID ID, const ParmVarDecl *PVD) - : Loan(Kind::Placeholder, ID), PVD(PVD) {} + : Loan(Kind::Placeholder, ID), ParamOrMethod(PVD) {} - const ParmVarDecl *getParmVarDecl() const { return PVD; } + PlaceholderLoan(LoanID ID, const CXXMethodDecl *MD) + : Loan(Kind::Placeholder, ID), ParamOrMethod(MD) {} + + const ParmVarDecl *getParmVarDecl() const { + return ParamOrMethod.dyn_cast<const ParmVarDecl *>(); + } + + const CXXMethodDecl *getMethodDecl() const { + return ParamOrMethod.dyn_cast<const CXXMethodDecl *>(); + } void dump(llvm::raw_ostream &OS) const override; diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Origins.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Origins.h index 690faae996f0e..60b607c77da4e 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Origins.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Origins.h @@ -15,6 +15,7 @@ #define LLVM_CLANG_ANALYSIS_ANALYSES_LIFETIMESAFETY_ORIGINS_H #include "clang/AST/Decl.h" +#include "clang/AST/DeclCXX.h" #include "clang/AST/Expr.h" #include "clang/AST/TypeBase.h" #include "clang/Analysis/Analyses/LifetimeSafety/LifetimeStats.h" @@ -124,7 +125,8 @@ bool doesDeclHaveStorage(const ValueDecl *D); /// variables and expressions. class OriginManager { public: - explicit OriginManager(ASTContext &AST) : AST(AST) {} + explicit OriginManager(ASTContext &AST, const Decl *D) + : AST(AST), CurrentDecl(D) {} /// Gets or creates the OriginList for a given ValueDecl. /// @@ -144,6 +146,15 @@ class OriginManager { /// \returns The OriginList, or nullptr for non-pointer rvalues. OriginList *getOrCreateList(const Expr *E); + /// Gets or creates the OriginList for the implicit 'this' parameter of a + /// given CXXMethodDecl. + /// + /// Creates a list structure mirroring the levels of indirection in the + /// method's 'this' type (e.g., `S*` for a non-static method of class `S`). + /// + /// \returns The OriginList for the implicit object parameter. + OriginList *getOrCreateList(const CXXMethodDecl *MD); + const Origin &getOrigin(OriginID ID) const; llvm::ArrayRef<Origin> getOrigins() const { return AllOrigins; } @@ -172,6 +183,7 @@ class OriginManager { llvm::BumpPtrAllocator ListAllocator; llvm::DenseMap<const clang::ValueDecl *, OriginList *> DeclToList; llvm::DenseMap<const clang::Expr *, OriginList *> ExprToList; + const Decl *CurrentDecl; }; } // namespace clang::lifetimes::internal diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 1ab3f537d36a3..c624a719e2583 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -10832,6 +10832,18 @@ def warn_lifetime_safety_cross_tu_suggestion InGroup<LifetimeSafetyCrossTUSuggestions>, DefaultIgnore; +def warn_lifetime_safety_this_intra_tu_suggestion + : Warning<"implict this in intra-TU function should be marked " + "[[clang::lifetimebound]]">, + InGroup<LifetimeSafetyIntraTUSuggestions>, + DefaultIgnore; + +def warn_lifetime_safety_this_cross_tu_suggestion + : Warning<"implicit this in cross-TU function should be marked " + "[[clang::lifetimebound]]">, + InGroup<LifetimeSafetyCrossTUSuggestions>, + DefaultIgnore; + def note_lifetime_safety_suggestion_returned_here : Note<"param returned here">; // For non-floating point, expressions of the form x == x or x != x diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp index f7383126fac38..ffdaad5026b3d 100644 --- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp @@ -49,10 +49,13 @@ struct PendingWarning { Confidence ConfidenceLevel; }; +using AnnotationTarget = + llvm::PointerUnion<const ParmVarDecl *, const CXXMethodDecl *>; + class LifetimeChecker { private: llvm::DenseMap<LoanID, PendingWarning> FinalWarningsMap; - llvm::DenseMap<const ParmVarDecl *, const Expr *> AnnotationWarningsMap; + llvm::DenseMap<AnnotationTarget, const Expr *> AnnotationWarningsMap; const LoanPropagationAnalysis &LoanPropagation; const LiveOriginsAnalysis &LiveOrigins; const FactManager &FactMgr; @@ -88,10 +91,14 @@ class LifetimeChecker { for (LoanID LID : EscapedLoans) { const Loan *L = FactMgr.getLoanMgr().getLoan(LID); if (const auto *PL = dyn_cast<PlaceholderLoan>(L)) { - const ParmVarDecl *PVD = PL->getParmVarDecl(); - if (PVD->hasAttr<LifetimeBoundAttr>()) - continue; - AnnotationWarningsMap.try_emplace(PVD, OEF->getEscapeExpr()); + if (const auto *PVD = PL->getParmVarDecl()) { + if (PVD->hasAttr<LifetimeBoundAttr>()) + continue; + AnnotationWarningsMap.try_emplace(PVD, OEF->getEscapeExpr()); + } else if (const auto *MD = PL->getMethodDecl()) { + if (!implicitObjectParamIsLifetimeBound(MD)) + AnnotationWarningsMap.try_emplace(MD, OEF->getEscapeExpr()); + } } } } @@ -164,50 +171,70 @@ class LifetimeChecker { /// Returns the declaration of a function that is visible across translation /// units, if such a declaration exists and is different from the definition. - static const FunctionDecl *getCrossTUDecl(const ParmVarDecl &PVD, + static const FunctionDecl *getCrossTUDecl(const FunctionDecl &FD, SourceManager &SM) { - const auto *FD = dyn_cast<FunctionDecl>(PVD.getDeclContext()); - if (!FD) - return nullptr; - if (!FD->isExternallyVisible()) + if (!FD.isExternallyVisible()) return nullptr; - const FileID DefinitionFile = SM.getFileID(FD->getLocation()); - for (const FunctionDecl *Redecl : FD->redecls()) + const FileID DefinitionFile = SM.getFileID(FD.getLocation()); + for (const FunctionDecl *Redecl : FD.redecls()) if (SM.getFileID(Redecl->getLocation()) != DefinitionFile) return Redecl; return nullptr; } + static const FunctionDecl *getCrossTUDecl(const ParmVarDecl &PVD, + SourceManager &SM) { + if (const auto *FD = dyn_cast<FunctionDecl>(PVD.getDeclContext())) + return getCrossTUDecl(*FD, SM); + return nullptr; + } + void suggestAnnotations() { if (!Reporter) return; SourceManager &SM = AST.getSourceManager(); - for (const auto &[PVD, EscapeExpr] : AnnotationWarningsMap) { - if (const FunctionDecl *CrossTUDecl = getCrossTUDecl(*PVD, SM)) - Reporter->suggestAnnotation( - SuggestionScope::CrossTU, - CrossTUDecl->getParamDecl(PVD->getFunctionScopeIndex()), - EscapeExpr); - else - Reporter->suggestAnnotation(SuggestionScope::IntraTU, PVD, EscapeExpr); + for (const auto &[Target, EscapeExpr] : AnnotationWarningsMap) { + if (const auto *PVD = Target.dyn_cast<const ParmVarDecl *>()) { + if (const FunctionDecl *CrossTUDecl = getCrossTUDecl(*PVD, SM)) + Reporter->suggestAnnotation( + SuggestionScope::CrossTU, + CrossTUDecl->getParamDecl(PVD->getFunctionScopeIndex()), + EscapeExpr); + else + Reporter->suggestAnnotation(SuggestionScope::IntraTU, PVD, + EscapeExpr); + } else if (const auto *MD = Target.dyn_cast<const CXXMethodDecl *>()) { + if (const FunctionDecl *CrossTUDecl = getCrossTUDecl(*MD, SM)) + Reporter->suggestAnnotation(SuggestionScope::CrossTU, + cast<const CXXMethodDecl>(CrossTUDecl), + EscapeExpr); + else + Reporter->suggestAnnotation(SuggestionScope::IntraTU, MD, EscapeExpr); + } } } void inferAnnotations() { - for (const auto &[ConstPVD, EscapeExpr] : AnnotationWarningsMap) { - ParmVarDecl *PVD = const_cast<ParmVarDecl *>(ConstPVD); - const auto *FD = dyn_cast<FunctionDecl>(PVD->getDeclContext()); - if (!FD) - continue; - // Propagates inferred attributes via the most recent declaration to - // ensure visibility for callers in post-order analysis. - FD = getDeclWithMergedLifetimeBoundAttrs(FD); - ParmVarDecl *InferredPVD = const_cast<ParmVarDecl *>( - FD->getParamDecl(PVD->getFunctionScopeIndex())); - if (!InferredPVD->hasAttr<LifetimeBoundAttr>()) - InferredPVD->addAttr( - LifetimeBoundAttr::CreateImplicit(AST, PVD->getLocation())); + for (const auto &[Target, EscapeExpr] : AnnotationWarningsMap) { + if (const auto *MD = Target.dyn_cast<const CXXMethodDecl *>()) { + CXXMethodDecl *MutableMD = const_cast<CXXMethodDecl *>(MD); + if (!MutableMD->hasAttr<LifetimeBoundAttr>()) + MutableMD->addAttr( + LifetimeBoundAttr::CreateImplicit(AST, MutableMD->getLocation())); + } else if (const auto *PVD = Target.dyn_cast<const ParmVarDecl *>()) { + const auto *FD = dyn_cast<FunctionDecl>(PVD->getDeclContext()); + if (!FD) + continue; + // Propagates inferred attributes via the most recent declaration to + // ensure visibility for callers in post-order analysis. + FD = getDeclWithMergedLifetimeBoundAttrs(FD); + ParmVarDecl *InferredPVD = const_cast<ParmVarDecl *>( + FD->getParamDecl(PVD->getFunctionScopeIndex())); + if (!InferredPVD->hasAttr<LifetimeBoundAttr>()) + InferredPVD->addAttr( + LifetimeBoundAttr::CreateImplicit(AST, PVD->getLocation())); + } } } }; diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index 54e19b58bd7d5..7093f0a0e005f 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -610,10 +610,18 @@ void FactsGenerator::markUseAsWrite(const DeclRefExpr *DRE) { // parameter at the function's entry. llvm::SmallVector<Fact *> FactsGenerator::issuePlaceholderLoans() { const auto *FD = dyn_cast<FunctionDecl>(AC.getDecl()); - if (!FD) + if (!FD || FD->isImplicit()) return {}; llvm::SmallVector<Fact *> PlaceholderLoanFacts; + const Decl *D = AC.getDecl(); + if (const auto *MD = dyn_cast<CXXMethodDecl>(D); MD && MD->isInstance()) { + OriginList *List = FactMgr.getOriginMgr().getOrCreateList(MD); + const PlaceholderLoan *L = + FactMgr.getLoanMgr().createLoan<PlaceholderLoan>(MD); + PlaceholderLoanFacts.push_back( + FactMgr.createFact<IssueFact>(L->getID(), List->getOuterOriginID())); + } for (const ParmVarDecl *PVD : FD->parameters()) { OriginList *List = getOriginsList(*PVD); if (!List) diff --git a/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp b/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp index 2772fe20de19b..284163ebc4bc3 100644 --- a/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp @@ -54,6 +54,8 @@ bool isAssignmentOperatorLifetimeBound(const CXXMethodDecl *CMD) { bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) { FD = getDeclWithMergedLifetimeBoundAttrs(FD); + if (FD->hasAttr<LifetimeBoundAttr>()) + return true; const TypeSourceInfo *TSI = FD->getTypeSourceInfo(); if (!TSI) return false; diff --git a/clang/lib/Analysis/LifetimeSafety/Origins.cpp b/clang/lib/Analysis/LifetimeSafety/Origins.cpp index ca933f612eb08..8349d18267b62 100644 --- a/clang/lib/Analysis/LifetimeSafety/Origins.cpp +++ b/clang/lib/Analysis/LifetimeSafety/Origins.cpp @@ -137,6 +137,12 @@ OriginList *OriginManager::getOrCreateList(const Expr *E) { return It->second; QualType Type = E->getType(); + // Special handling for 'this' expressions to share origins with the method's + // implicit object parameter. + if (isa<CXXThisExpr>(E)) { + if (const auto *MD = dyn_cast<CXXMethodDecl>(CurrentDecl)) + return getOrCreateList(MD); + } // Special handling for DeclRefExpr to share origins with the underlying decl. if (auto *DRE = dyn_cast<DeclRefExpr>(E)) { @@ -169,6 +175,13 @@ OriginList *OriginManager::getOrCreateList(const Expr *E) { return ExprToList[E] = buildListForType(Type, E); } +OriginList *OriginManager::getOrCreateList(const CXXMethodDecl *MD) { + auto It = DeclToList.find(MD); + if (It != DeclToList.end()) + return It->second; + return DeclToList[MD] = buildListForType(MD->getThisType(), MD); +} + void OriginManager::dump(OriginID OID, llvm::raw_ostream &OS) const { OS << OID << " ("; Origin O = getOrigin(OID); diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index d297d9e80cf2f..e79a38dd786f4 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -2919,6 +2919,30 @@ class LifetimeSafetyReporterImpl : public LifetimeSafetyReporter { << EscapeExpr->getSourceRange(); } + void suggestAnnotation(SuggestionScope Scope, const CXXMethodDecl *MD, + const Expr *EscapeExpr) override { + unsigned DiagID = (Scope == SuggestionScope::CrossTU) + ? diag::warn_lifetime_safety_this_cross_tu_suggestion + : diag::warn_lifetime_safety_this_intra_tu_suggestion; + + SourceLocation InsertionPoint; + if (auto *TSI = MD->getTypeSourceInfo()) + InsertionPoint = + Lexer::getLocForEndOfToken(TSI->getTypeLoc().getEndLoc(), 0, + S.getSourceManager(), S.getLangOpts()); + else + InsertionPoint = MD->getLocation(); + + S.Diag(InsertionPoint, DiagID) + << MD->getNameInfo().getSourceRange() + << FixItHint::CreateInsertion(InsertionPoint, + " [[clang::lifetimebound]]"); + + S.Diag(EscapeExpr->getBeginLoc(), + diag::note_lifetime_safety_suggestion_returned_here) + << EscapeExpr->getSourceRange(); + } + private: Sema &S; }; diff --git a/clang/test/Sema/warn-lifetime-safety-suggestions.cpp b/clang/test/Sema/warn-lifetime-safety-suggestions.cpp index 6e3a6f1fd9117..2550346903175 100644 --- a/clang/test/Sema/warn-lifetime-safety-suggestions.cpp +++ b/clang/test/Sema/warn-lifetime-safety-suggestions.cpp @@ -161,16 +161,21 @@ static View return_view_static(View a) { // expected-warning {{parameter in int return a; // expected-note {{param returned here}} } -//===----------------------------------------------------------------------===// -// FIXME Test Cases -//===----------------------------------------------------------------------===// +struct ReturnThis { + const ReturnThis& get() { // expected-warning {{implict this in intra-TU function should be marked [[clang::lifetimebound]]}}. + return *this; // expected-note {{param returned here}} + } +}; struct ReturnsSelf { - const ReturnsSelf& get() const { - return *this; + const ReturnsSelf& get() const { // expected-warning {{implict this in intra-TU function should be marked [[clang::lifetimebound]]}}. + return *this; // expected-note {{param returned here}} } }; +//===----------------------------------------------------------------------===// +// FIXME Test Cases +//===----------------------------------------------------------------------===// struct ViewProvider { MyObj data; View getView() const { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
