https://github.com/usx95 created https://github.com/llvm/llvm-project/pull/169620
None >From 879b4353a2b410541e5199edc531d3e5d323fad6 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena <[email protected]> Date: Wed, 26 Nov 2025 08:32:29 +0000 Subject: [PATCH] Annotation changes --- .../LifetimeSafety/LifetimeAnnotations.h | 5 ++ .../LifetimeSafety/FactsGenerator.cpp | 12 ---- .../LifetimeSafety/LifetimeAnnotations.cpp | 31 +++++++++ clang/lib/Sema/CheckExprLifetime.cpp | 69 +++++-------------- clang/lib/Sema/CheckExprLifetime.h | 3 - clang/lib/Sema/SemaAttr.cpp | 4 +- 6 files changed, 57 insertions(+), 67 deletions(-) diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h index f02969e0a9563..1a16fb82f9a84 100644 --- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h +++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h @@ -38,6 +38,11 @@ bool isAssignmentOperatorLifetimeBound(const CXXMethodDecl *CMD); /// method or because it's a normal assignment operator. bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD); +// Tells whether the type is annotated with [[gsl::Pointer]]. +bool isGslPointerType(QualType QT); +// Tells whether the type is annotated with [[gsl::Owner]]. +bool isGslOwnerType(QualType QT); + } // namespace clang::lifetimes #endif // LLVM_CLANG_ANALYSIS_ANALYSES_LIFETIMEANNOTATIONS_H diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp index f7be472ed15b5..00870c3fd4086 100644 --- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp +++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp @@ -15,18 +15,6 @@ namespace clang::lifetimes::internal { using llvm::isa_and_present; -static bool isGslPointerType(QualType QT) { - if (const auto *RD = QT->getAsCXXRecordDecl()) { - // We need to check the template definition for specializations. - if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(RD)) - return CTSD->getSpecializedTemplate() - ->getTemplatedDecl() - ->hasAttr<PointerAttr>(); - return RD->hasAttr<PointerAttr>(); - } - return false; -} - static bool isPointerType(QualType QT) { return QT->isPointerOrReferenceType() || isGslPointerType(QT); } diff --git a/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp b/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp index ad61a42c0eaeb..54e343fc2ee5e 100644 --- a/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp +++ b/clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp @@ -10,6 +10,7 @@ #include "clang/AST/Attr.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" #include "clang/AST/Type.h" #include "clang/AST/TypeLoc.h" @@ -70,4 +71,34 @@ bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) { return isNormalAssignmentOperator(FD); } +template <typename T> static bool isRecordWithAttr(QualType Type) { + auto *RD = Type->getAsCXXRecordDecl(); + if (!RD) + return false; + // Generally, if a primary template class declaration is annotated with an + // attribute, all its specializations generated from template instantiations + // should inherit the attribute. + // + // However, since lifetime analysis occurs during parsing, we may encounter + // cases where a full definition of the specialization is not required. In + // such cases, the specialization declaration remains incomplete and lacks the + // attribute. Therefore, we fall back to checking the primary template class. + // + // Note: it is possible for a specialization declaration to have an attribute + // even if the primary template does not. + // + // FIXME: What if the primary template and explicit specialization + // declarations have conflicting attributes? We should consider diagnosing + // this scenario. + bool Result = RD->hasAttr<T>(); + + if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(RD)) + Result |= CTSD->getSpecializedTemplate()->getTemplatedDecl()->hasAttr<T>(); + + return Result; +} + +bool isGslPointerType(QualType QT) { return isRecordWithAttr<PointerAttr>(QT); } +bool isGslOwnerType(QualType QT) { return isRecordWithAttr<OwnerAttr>(QT); } + } // namespace clang::lifetimes diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index f9665b5e59831..c91ca751984c9 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -17,6 +17,9 @@ #include "llvm/ADT/PointerIntPair.h" namespace clang::sema { +using lifetimes::isGslOwnerType; +using lifetimes::isGslPointerType; + namespace { enum LifetimeKind { /// The lifetime of a temporary bound to this entity ends at the end of the @@ -257,38 +260,8 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path, Expr *Init, ReferenceKind RK, LocalVisitor Visit); -template <typename T> static bool isRecordWithAttr(QualType Type) { - auto *RD = Type->getAsCXXRecordDecl(); - if (!RD) - return false; - // Generally, if a primary template class declaration is annotated with an - // attribute, all its specializations generated from template instantiations - // should inherit the attribute. - // - // However, since lifetime analysis occurs during parsing, we may encounter - // cases where a full definition of the specialization is not required. In - // such cases, the specialization declaration remains incomplete and lacks the - // attribute. Therefore, we fall back to checking the primary template class. - // - // Note: it is possible for a specialization declaration to have an attribute - // even if the primary template does not. - // - // FIXME: What if the primary template and explicit specialization - // declarations have conflicting attributes? We should consider diagnosing - // this scenario. - bool Result = RD->hasAttr<T>(); - - if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(RD)) - Result |= CTSD->getSpecializedTemplate()->getTemplatedDecl()->hasAttr<T>(); - - return Result; -} - -// Tells whether the type is annotated with [[gsl::Pointer]]. -bool isGLSPointerType(QualType QT) { return isRecordWithAttr<PointerAttr>(QT); } - static bool isPointerLikeType(QualType QT) { - return isGLSPointerType(QT) || QT->isPointerType() || QT->isNullPtrType(); + return isGslPointerType(QT) || QT->isPointerType() || QT->isNullPtrType(); } // Decl::isInStdNamespace will return false for iterators in some STL @@ -331,7 +304,7 @@ static bool isContainerOfOwner(const RecordDecl *Container) { return false; const auto &TAs = CTSD->getTemplateArgs(); return TAs.size() > 0 && TAs[0].getKind() == TemplateArgument::Type && - isRecordWithAttr<OwnerAttr>(TAs[0].getAsType()); + isGslOwnerType(TAs[0].getAsType()); } // Returns true if the given Record is `std::initializer_list<pointer>`. @@ -349,14 +322,13 @@ static bool isStdInitializerListOfPointer(const RecordDecl *RD) { static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee) { if (auto *Conv = dyn_cast_or_null<CXXConversionDecl>(Callee)) - if (isRecordWithAttr<PointerAttr>(Conv->getConversionType()) && + if (isGslPointerType(Conv->getConversionType()) && Callee->getParent()->hasAttr<OwnerAttr>()) return true; if (!isInStlNamespace(Callee->getParent())) return false; - if (!isRecordWithAttr<PointerAttr>( - Callee->getFunctionObjectParameterType()) && - !isRecordWithAttr<OwnerAttr>(Callee->getFunctionObjectParameterType())) + if (!isGslPointerType(Callee->getFunctionObjectParameterType()) && + !isGslOwnerType(Callee->getFunctionObjectParameterType())) return false; if (isPointerLikeType(Callee->getReturnType())) { if (!Callee->getIdentifier()) @@ -393,7 +365,7 @@ static bool shouldTrackFirstArgument(const FunctionDecl *FD) { if (!RD->hasAttr<PointerAttr>() && !RD->hasAttr<OwnerAttr>()) return false; if (FD->getReturnType()->isPointerType() || - isRecordWithAttr<PointerAttr>(FD->getReturnType())) { + isGslPointerType(FD->getReturnType())) { return llvm::StringSwitch<bool>(FD->getName()) .Cases({"begin", "rbegin", "cbegin", "crbegin"}, true) .Cases({"end", "rend", "cend", "crend"}, true) @@ -465,7 +437,7 @@ shouldTrackFirstArgumentForConstructor(const CXXConstructExpr *Ctor) { return true; // RHS must be an owner. - if (!isRecordWithAttr<OwnerAttr>(RHSArgType)) + if (!isGslOwnerType(RHSArgType)) return false; // Bail out if the RHS is Owner<Pointer>. @@ -547,7 +519,7 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, // Once we initialized a value with a non gsl-owner reference, it can no // longer dangle. if (ReturnType->isReferenceType() && - !isRecordWithAttr<OwnerAttr>(ReturnType->getPointeeType())) { + !isGslOwnerType(ReturnType->getPointeeType())) { for (const IndirectLocalPathEntry &PE : llvm::reverse(Path)) { if (PE.Kind == IndirectLocalPathEntry::GslReferenceInit || PE.Kind == IndirectLocalPathEntry::LifetimeBoundCall) @@ -1158,8 +1130,7 @@ static AnalysisResult analyzePathForGSLPointer(const IndirectLocalPath &Path, // auto p2 = Temp().owner; // Here p2 is dangling. if (const auto *FD = llvm::dyn_cast_or_null<FieldDecl>(E.D); FD && !FD->getType()->isReferenceType() && - isRecordWithAttr<OwnerAttr>(FD->getType()) && - LK != LK_MemInitializer) { + isGslOwnerType(FD->getType()) && LK != LK_MemInitializer) { return Report; } return Abandon; @@ -1191,10 +1162,9 @@ static AnalysisResult analyzePathForGSLPointer(const IndirectLocalPath &Path, // const GSLOwner& func(const Foo& foo [[clang::lifetimebound]]) // GSLOwner* func(cosnt Foo& foo [[clang::lifetimebound]]) // GSLPointer func(const Foo& foo [[clang::lifetimebound]]) - if (FD && - ((FD->getReturnType()->isPointerOrReferenceType() && - isRecordWithAttr<OwnerAttr>(FD->getReturnType()->getPointeeType())) || - isGLSPointerType(FD->getReturnType()))) + if (FD && ((FD->getReturnType()->isPointerOrReferenceType() && + isGslOwnerType(FD->getReturnType()->getPointeeType())) || + isGslPointerType(FD->getReturnType()))) return Report; return Abandon; @@ -1206,7 +1176,7 @@ static AnalysisResult analyzePathForGSLPointer(const IndirectLocalPath &Path, // int &p = *localUniquePtr; // someContainer.add(std::move(localUniquePtr)); // return p; - if (!pathContainsInit(Path) && isRecordWithAttr<OwnerAttr>(L->getType())) + if (!pathContainsInit(Path) && isGslOwnerType(L->getType())) return Report; return Abandon; } @@ -1215,8 +1185,7 @@ static AnalysisResult analyzePathForGSLPointer(const IndirectLocalPath &Path, auto *MTE = dyn_cast<MaterializeTemporaryExpr>(L); bool IsGslPtrValueFromGslTempOwner = - MTE && !MTE->getExtendingDecl() && - isRecordWithAttr<OwnerAttr>(MTE->getType()); + MTE && !MTE->getExtendingDecl() && isGslOwnerType(MTE->getType()); // Skipping a chain of initializing gsl::Pointer annotated objects. // We are looking only for the final source to find out if it was // a local or temporary owner or the address of a local @@ -1231,7 +1200,7 @@ static bool shouldRunGSLAssignmentAnalysis(const Sema &SemaRef, bool EnableGSLAssignmentWarnings = !SemaRef.getDiagnostics().isIgnored( diag::warn_dangling_lifetime_pointer_assignment, SourceLocation()); return (EnableGSLAssignmentWarnings && - (isRecordWithAttr<PointerAttr>(Entity.LHS->getType()) || + (isGslPointerType(Entity.LHS->getType()) || lifetimes::isAssignmentOperatorLifetimeBound( Entity.AssignmentOperator))); } @@ -1400,7 +1369,7 @@ checkExprLifetimeImpl(Sema &SemaRef, const InitializedEntity *InitEntity, // Suppress false positives for code like the one below: // Ctor(unique_ptr<T> up) : pointer(up.get()), owner(move(up)) {} // FIXME: move this logic to analyzePathForGSLPointer. - if (DRE && isRecordWithAttr<OwnerAttr>(DRE->getType())) + if (DRE && isGslOwnerType(DRE->getType())) return false; auto *VD = DRE ? dyn_cast<VarDecl>(DRE->getDecl()) : nullptr; diff --git a/clang/lib/Sema/CheckExprLifetime.h b/clang/lib/Sema/CheckExprLifetime.h index 16595d0ca1b36..38b7061988dc7 100644 --- a/clang/lib/Sema/CheckExprLifetime.h +++ b/clang/lib/Sema/CheckExprLifetime.h @@ -18,9 +18,6 @@ namespace clang::sema { -// Tells whether the type is annotated with [[gsl::Pointer]]. -bool isGLSPointerType(QualType QT); - /// Describes an entity that is being assigned. struct AssignedEntity { // The left-hand side expression of the assignment. diff --git a/clang/lib/Sema/SemaAttr.cpp b/clang/lib/Sema/SemaAttr.cpp index 8411a3da8322d..7729c113e422e 100644 --- a/clang/lib/Sema/SemaAttr.cpp +++ b/clang/lib/Sema/SemaAttr.cpp @@ -11,11 +11,11 @@ // //===----------------------------------------------------------------------===// -#include "CheckExprLifetime.h" #include "clang/AST/ASTConsumer.h" #include "clang/AST/Attr.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/Expr.h" +#include "clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h" #include "clang/Basic/TargetInfo.h" #include "clang/Lex/Preprocessor.h" #include "clang/Sema/Lookup.h" @@ -289,7 +289,7 @@ void Sema::inferLifetimeCaptureByAttribute(FunctionDecl *FD) { // We only apply the lifetime_capture_by attribute to parameters of // pointer-like reference types (`const T&`, `T&&`). if (PVD->getType()->isReferenceType() && - sema::isGLSPointerType(PVD->getType().getNonReferenceType())) { + lifetimes::isGslPointerType(PVD->getType().getNonReferenceType())) { int CaptureByThis[] = {LifetimeCaptureByAttr::This}; PVD->addAttr( LifetimeCaptureByAttr::CreateImplicit(Context, CaptureByThis, 1)); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
