Skip to content

Commit 978ce83

Browse files
usx95Gabor Horvath
authored andcommitted
[LifetimeSafety] Move GSL pointer/owner type detection to LifetimeAnnotations (llvm#169620)
Refactored GSL pointer and owner type detection functions to improve code organization and reusability.
1 parent fadaf5d commit 978ce83

File tree

6 files changed

+57
-67
lines changed

6 files changed

+57
-67
lines changed

clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@ bool isAssignmentOperatorLifetimeBound(const CXXMethodDecl *CMD);
3838
/// method or because it's a normal assignment operator.
3939
bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD);
4040

41+
// Tells whether the type is annotated with [[gsl::Pointer]].
42+
bool isGslPointerType(QualType QT);
43+
// Tells whether the type is annotated with [[gsl::Owner]].
44+
bool isGslOwnerType(QualType QT);
45+
4146
} // namespace clang::lifetimes
4247

4348
#endif // LLVM_CLANG_ANALYSIS_ANALYSES_LIFETIMEANNOTATIONS_H

clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,6 @@
1515
namespace clang::lifetimes::internal {
1616
using llvm::isa_and_present;
1717

18-
static bool isGslPointerType(QualType QT) {
19-
if (const auto *RD = QT->getAsCXXRecordDecl()) {
20-
// We need to check the template definition for specializations.
21-
if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(RD))
22-
return CTSD->getSpecializedTemplate()
23-
->getTemplatedDecl()
24-
->hasAttr<PointerAttr>();
25-
return RD->hasAttr<PointerAttr>();
26-
}
27-
return false;
28-
}
29-
3018
static bool isPointerType(QualType QT) {
3119
return QT->isPointerOrReferenceType() || isGslPointerType(QT);
3220
}

clang/lib/Analysis/LifetimeSafety/LifetimeAnnotations.cpp

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "clang/AST/Attr.h"
1111
#include "clang/AST/Decl.h"
1212
#include "clang/AST/DeclCXX.h"
13+
#include "clang/AST/DeclTemplate.h"
1314
#include "clang/AST/Type.h"
1415
#include "clang/AST/TypeLoc.h"
1516

@@ -70,4 +71,34 @@ bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) {
7071
return isNormalAssignmentOperator(FD);
7172
}
7273

74+
template <typename T> static bool isRecordWithAttr(QualType Type) {
75+
auto *RD = Type->getAsCXXRecordDecl();
76+
if (!RD)
77+
return false;
78+
// Generally, if a primary template class declaration is annotated with an
79+
// attribute, all its specializations generated from template instantiations
80+
// should inherit the attribute.
81+
//
82+
// However, since lifetime analysis occurs during parsing, we may encounter
83+
// cases where a full definition of the specialization is not required. In
84+
// such cases, the specialization declaration remains incomplete and lacks the
85+
// attribute. Therefore, we fall back to checking the primary template class.
86+
//
87+
// Note: it is possible for a specialization declaration to have an attribute
88+
// even if the primary template does not.
89+
//
90+
// FIXME: What if the primary template and explicit specialization
91+
// declarations have conflicting attributes? We should consider diagnosing
92+
// this scenario.
93+
bool Result = RD->hasAttr<T>();
94+
95+
if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(RD))
96+
Result |= CTSD->getSpecializedTemplate()->getTemplatedDecl()->hasAttr<T>();
97+
98+
return Result;
99+
}
100+
101+
bool isGslPointerType(QualType QT) { return isRecordWithAttr<PointerAttr>(QT); }
102+
bool isGslOwnerType(QualType QT) { return isRecordWithAttr<OwnerAttr>(QT); }
103+
73104
} // namespace clang::lifetimes

clang/lib/Sema/CheckExprLifetime.cpp

Lines changed: 19 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@
1717
#include "llvm/ADT/PointerIntPair.h"
1818

1919
namespace clang::sema {
20+
using lifetimes::isGslOwnerType;
21+
using lifetimes::isGslPointerType;
22+
2023
namespace {
2124
enum LifetimeKind {
2225
/// The lifetime of a temporary bound to this entity ends at the end of the
@@ -256,38 +259,8 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path,
256259
Expr *Init, ReferenceKind RK,
257260
LocalVisitor Visit);
258261

259-
template <typename T> static bool isRecordWithAttr(QualType Type) {
260-
auto *RD = Type->getAsCXXRecordDecl();
261-
if (!RD)
262-
return false;
263-
// Generally, if a primary template class declaration is annotated with an
264-
// attribute, all its specializations generated from template instantiations
265-
// should inherit the attribute.
266-
//
267-
// However, since lifetime analysis occurs during parsing, we may encounter
268-
// cases where a full definition of the specialization is not required. In
269-
// such cases, the specialization declaration remains incomplete and lacks the
270-
// attribute. Therefore, we fall back to checking the primary template class.
271-
//
272-
// Note: it is possible for a specialization declaration to have an attribute
273-
// even if the primary template does not.
274-
//
275-
// FIXME: What if the primary template and explicit specialization
276-
// declarations have conflicting attributes? We should consider diagnosing
277-
// this scenario.
278-
bool Result = RD->hasAttr<T>();
279-
280-
if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(RD))
281-
Result |= CTSD->getSpecializedTemplate()->getTemplatedDecl()->hasAttr<T>();
282-
283-
return Result;
284-
}
285-
286-
// Tells whether the type is annotated with [[gsl::Pointer]].
287-
bool isGLSPointerType(QualType QT) { return isRecordWithAttr<PointerAttr>(QT); }
288-
289262
static bool isPointerLikeType(QualType QT) {
290-
return isGLSPointerType(QT) || QT->isPointerType() || QT->isNullPtrType();
263+
return isGslPointerType(QT) || QT->isPointerType() || QT->isNullPtrType();
291264
}
292265

293266
// Decl::isInStdNamespace will return false for iterators in some STL
@@ -330,7 +303,7 @@ static bool isContainerOfOwner(const RecordDecl *Container) {
330303
return false;
331304
const auto &TAs = CTSD->getTemplateArgs();
332305
return TAs.size() > 0 && TAs[0].getKind() == TemplateArgument::Type &&
333-
isRecordWithAttr<OwnerAttr>(TAs[0].getAsType());
306+
isGslOwnerType(TAs[0].getAsType());
334307
}
335308

336309
// Returns true if the given Record is `std::initializer_list<pointer>`.
@@ -348,14 +321,13 @@ static bool isStdInitializerListOfPointer(const RecordDecl *RD) {
348321

349322
static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee) {
350323
if (auto *Conv = dyn_cast_or_null<CXXConversionDecl>(Callee))
351-
if (isRecordWithAttr<PointerAttr>(Conv->getConversionType()) &&
324+
if (isGslPointerType(Conv->getConversionType()) &&
352325
Callee->getParent()->hasAttr<OwnerAttr>())
353326
return true;
354327
if (!isInStlNamespace(Callee->getParent()))
355328
return false;
356-
if (!isRecordWithAttr<PointerAttr>(
357-
Callee->getFunctionObjectParameterType()) &&
358-
!isRecordWithAttr<OwnerAttr>(Callee->getFunctionObjectParameterType()))
329+
if (!isGslPointerType(Callee->getFunctionObjectParameterType()) &&
330+
!isGslOwnerType(Callee->getFunctionObjectParameterType()))
359331
return false;
360332
if (isPointerLikeType(Callee->getReturnType())) {
361333
if (!Callee->getIdentifier())
@@ -392,7 +364,7 @@ static bool shouldTrackFirstArgument(const FunctionDecl *FD) {
392364
if (!RD->hasAttr<PointerAttr>() && !RD->hasAttr<OwnerAttr>())
393365
return false;
394366
if (FD->getReturnType()->isPointerType() ||
395-
isRecordWithAttr<PointerAttr>(FD->getReturnType())) {
367+
isGslPointerType(FD->getReturnType())) {
396368
return llvm::StringSwitch<bool>(FD->getName())
397369
.Cases("begin", "rbegin", "cbegin", "crbegin", true)
398370
.Cases("end", "rend", "cend", "crend", true)
@@ -464,7 +436,7 @@ shouldTrackFirstArgumentForConstructor(const CXXConstructExpr *Ctor) {
464436
return true;
465437

466438
// RHS must be an owner.
467-
if (!isRecordWithAttr<OwnerAttr>(RHSArgType))
439+
if (!isGslOwnerType(RHSArgType))
468440
return false;
469441

470442
// Bail out if the RHS is Owner<Pointer>.
@@ -546,7 +518,7 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call,
546518
// Once we initialized a value with a non gsl-owner reference, it can no
547519
// longer dangle.
548520
if (ReturnType->isReferenceType() &&
549-
!isRecordWithAttr<OwnerAttr>(ReturnType->getPointeeType())) {
521+
!isGslOwnerType(ReturnType->getPointeeType())) {
550522
for (const IndirectLocalPathEntry &PE : llvm::reverse(Path)) {
551523
if (PE.Kind == IndirectLocalPathEntry::GslReferenceInit ||
552524
PE.Kind == IndirectLocalPathEntry::LifetimeBoundCall)
@@ -1157,8 +1129,7 @@ static AnalysisResult analyzePathForGSLPointer(const IndirectLocalPath &Path,
11571129
// auto p2 = Temp().owner; // Here p2 is dangling.
11581130
if (const auto *FD = llvm::dyn_cast_or_null<FieldDecl>(E.D);
11591131
FD && !FD->getType()->isReferenceType() &&
1160-
isRecordWithAttr<OwnerAttr>(FD->getType()) &&
1161-
LK != LK_MemInitializer) {
1132+
isGslOwnerType(FD->getType()) && LK != LK_MemInitializer) {
11621133
return Report;
11631134
}
11641135
return Abandon;
@@ -1190,10 +1161,9 @@ static AnalysisResult analyzePathForGSLPointer(const IndirectLocalPath &Path,
11901161
// const GSLOwner& func(const Foo& foo [[clang::lifetimebound]])
11911162
// GSLOwner* func(cosnt Foo& foo [[clang::lifetimebound]])
11921163
// GSLPointer func(const Foo& foo [[clang::lifetimebound]])
1193-
if (FD &&
1194-
((FD->getReturnType()->isPointerOrReferenceType() &&
1195-
isRecordWithAttr<OwnerAttr>(FD->getReturnType()->getPointeeType())) ||
1196-
isGLSPointerType(FD->getReturnType())))
1164+
if (FD && ((FD->getReturnType()->isPointerOrReferenceType() &&
1165+
isGslOwnerType(FD->getReturnType()->getPointeeType())) ||
1166+
isGslPointerType(FD->getReturnType())))
11971167
return Report;
11981168

11991169
return Abandon;
@@ -1205,7 +1175,7 @@ static AnalysisResult analyzePathForGSLPointer(const IndirectLocalPath &Path,
12051175
// int &p = *localUniquePtr;
12061176
// someContainer.add(std::move(localUniquePtr));
12071177
// return p;
1208-
if (!pathContainsInit(Path) && isRecordWithAttr<OwnerAttr>(L->getType()))
1178+
if (!pathContainsInit(Path) && isGslOwnerType(L->getType()))
12091179
return Report;
12101180
return Abandon;
12111181
}
@@ -1214,8 +1184,7 @@ static AnalysisResult analyzePathForGSLPointer(const IndirectLocalPath &Path,
12141184
auto *MTE = dyn_cast<MaterializeTemporaryExpr>(L);
12151185

12161186
bool IsGslPtrValueFromGslTempOwner =
1217-
MTE && !MTE->getExtendingDecl() &&
1218-
isRecordWithAttr<OwnerAttr>(MTE->getType());
1187+
MTE && !MTE->getExtendingDecl() && isGslOwnerType(MTE->getType());
12191188
// Skipping a chain of initializing gsl::Pointer annotated objects.
12201189
// We are looking only for the final source to find out if it was
12211190
// a local or temporary owner or the address of a local
@@ -1230,7 +1199,7 @@ static bool shouldRunGSLAssignmentAnalysis(const Sema &SemaRef,
12301199
bool EnableGSLAssignmentWarnings = !SemaRef.getDiagnostics().isIgnored(
12311200
diag::warn_dangling_lifetime_pointer_assignment, SourceLocation());
12321201
return (EnableGSLAssignmentWarnings &&
1233-
(isRecordWithAttr<PointerAttr>(Entity.LHS->getType()) ||
1202+
(isGslPointerType(Entity.LHS->getType()) ||
12341203
lifetimes::isAssignmentOperatorLifetimeBound(
12351204
Entity.AssignmentOperator)));
12361205
}
@@ -1399,7 +1368,7 @@ checkExprLifetimeImpl(Sema &SemaRef, const InitializedEntity *InitEntity,
13991368
// Suppress false positives for code like the one below:
14001369
// Ctor(unique_ptr<T> up) : pointer(up.get()), owner(move(up)) {}
14011370
// FIXME: move this logic to analyzePathForGSLPointer.
1402-
if (DRE && isRecordWithAttr<OwnerAttr>(DRE->getType()))
1371+
if (DRE && isGslOwnerType(DRE->getType()))
14031372
return false;
14041373

14051374
auto *VD = DRE ? dyn_cast<VarDecl>(DRE->getDecl()) : nullptr;

clang/lib/Sema/CheckExprLifetime.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,6 @@
1818

1919
namespace clang::sema {
2020

21-
// Tells whether the type is annotated with [[gsl::Pointer]].
22-
bool isGLSPointerType(QualType QT);
23-
2421
/// Describes an entity that is being assigned.
2522
struct AssignedEntity {
2623
// The left-hand side expression of the assignment.

clang/lib/Sema/SemaAttr.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,11 @@
1111
//
1212
//===----------------------------------------------------------------------===//
1313

14-
#include "CheckExprLifetime.h"
1514
#include "clang/AST/ASTConsumer.h"
1615
#include "clang/AST/Attr.h"
1716
#include "clang/AST/DeclCXX.h"
1817
#include "clang/AST/Expr.h"
18+
#include "clang/Analysis/Analyses/LifetimeSafety/LifetimeAnnotations.h"
1919
#include "clang/Basic/TargetInfo.h"
2020
#include "clang/Lex/Preprocessor.h"
2121
#include "clang/Sema/Lookup.h"
@@ -289,7 +289,7 @@ void Sema::inferLifetimeCaptureByAttribute(FunctionDecl *FD) {
289289
// We only apply the lifetime_capture_by attribute to parameters of
290290
// pointer-like reference types (`const T&`, `T&&`).
291291
if (PVD->getType()->isReferenceType() &&
292-
sema::isGLSPointerType(PVD->getType().getNonReferenceType())) {
292+
lifetimes::isGslPointerType(PVD->getType().getNonReferenceType())) {
293293
int CaptureByThis[] = {LifetimeCaptureByAttr::This};
294294
PVD->addAttr(
295295
LifetimeCaptureByAttr::CreateImplicit(Context, CaptureByThis, 1));

0 commit comments

Comments
 (0)