[LifetimeSafety] Move GSL pointer/owner type detection to LifetimeAnnotations #169620
+57
−67
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
usx95
changed the title
usx95
added
the
clang:temporal-safety
Graphite App
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-temporal-safety Author: Utkarsh Saxena (usx95) ChangesRefactored GSL pointer and owner type detection functions to improve code organization and reusability. Full diff: https://github.com/llvm/llvm-project/pull/169620.diff 6 Files Affected:
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));
|
Contributor
Author
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/52/builds/13150 Here is the relevant piece of the build log for the reference |
tanji-dg
pushed a commit
to tanji-dg/llvm-project
that referenced
this pull request
Nov 27, 2025
…otations (llvm#169620) Refactored GSL pointer and owner type detection functions to improve code organization and reusability.
GeneraluseAI
pushed a commit
to GeneraluseAI/llvm-project
that referenced
this pull request
Nov 27, 2025
…otations (llvm#169620) Refactored GSL pointer and owner type detection functions to improve code organization and reusability.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactored GSL pointer and owner type detection functions to improve code organization and reusability.