Skip to content

[clang] Add a common definition of isPointerLikeType for lifetime analysis #117315

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Nov 28, 2024
Merged

[clang] Add a common definition of isPointerLikeType for lifetime analysis #117315

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e30ae8a
[clang] Check specialization for annotation
usx95 Nov 22, 2024
db16339
move to Sema.h
usx95 Nov 28, 2024
d7e59a7
Merge branch 'main' into template-spec-annotation
usx95 Nov 28, 2024
9b412ba
reuse in lifetime capture by
usx95 Nov 28, 2024
f9f863f
address comments
usx95 Nov 28, 2024
a847f8b
fix namespace
usx95 Nov 28, 2024
29db012
format
usx95 Nov 28, 2024
241a35c
docs
usx95 Nov 28, 2024
203a349
move to CheckExprLifetime.h
usx95 Nov 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions clang/include/clang/Sema/Sema.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1763,6 +1763,18 @@ class Sema final : public SemaBase {
/// Add [[gsl::Pointer]] attributes for std:: types.
void inferGslPointerAttribute(TypedefNameDecl *TD);

template <typename T> static bool isRecordWithAttr(QualType Type) {
Copy link
Collaborator

@hokein hokein Nov 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd avoid expose this function if possible (as it seems only be used in CheckExprLifetime.cpp and isPointerLikeType). How about?

  • we just keep the isPointerLikeType declaration in Sema.h
  • define isPointerLikeType in CheckExprLifetime.cpp
  • the isRecordWithAttr can still be in CheckExprLifetime.cpp.

I think it is fine as CheckExprLifetime.cpp is a part of Sema library.

auto *RD = Type->getAsCXXRecordDecl();
if (!RD)
return false;
if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(RD))
RD = CTSD->getSpecializedTemplate()->getTemplatedDecl();
return RD->hasAttr<T>();
}

/// ....
static bool isPointerLikeType(QualType Type);

LifetimeCaptureByAttr *ParseLifetimeCaptureByAttr(const ParsedAttr &AL,
StringRef ParamName);
// Processes the argument 'X' in [[clang::lifetime_capture_by(X)]]. Since 'X'
Expand Down
42 changes: 16 additions & 26 deletions clang/lib/Sema/CheckExprLifetime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,12 +252,6 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path,
Expr *Init, ReferenceKind RK,
LocalVisitor Visit);

template <typename T> static bool isRecordWithAttr(QualType Type) {
if (auto *RD = Type->getAsCXXRecordDecl())
return RD->hasAttr<T>();
return false;
}

// Decl::isInStdNamespace will return false for iterators in some STL
// implementations due to them being defined in a namespace outside of the std
// namespace.
Expand All @@ -276,11 +270,6 @@ static bool isInStlNamespace(const Decl *D) {
return DC->isStdNamespace();
}

static bool isPointerLikeType(QualType Type) {
return isRecordWithAttr<PointerAttr>(Type) || Type->isPointerType() ||
Type->isNullPtrType();
}

// Returns true if the given Record decl is a form of `GSLOwner<Pointer>`
// type, e.g. std::vector<string_view>, std::optional<string_view>.
static bool isContainerOfPointer(const RecordDecl *Container) {
Expand All @@ -290,7 +279,7 @@ static bool isContainerOfPointer(const RecordDecl *Container) {
return false;
const auto &TAs = CTSD->getTemplateArgs();
return TAs.size() > 0 && TAs[0].getKind() == TemplateArgument::Type &&
isPointerLikeType(TAs[0].getAsType());
Sema::isPointerLikeType(TAs[0].getAsType());
}
return false;
}
Expand All @@ -303,7 +292,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());
Sema::isRecordWithAttr<OwnerAttr>(TAs[0].getAsType());
}

// Returns true if the given Record is `std::initializer_list<pointer>`.
Expand All @@ -314,23 +303,24 @@ static bool isStdInitializerListOfPointer(const RecordDecl *RD) {
return isInStlNamespace(RD) && RD->getIdentifier() &&
RD->getName() == "initializer_list" && TAs.size() > 0 &&
TAs[0].getKind() == TemplateArgument::Type &&
isPointerLikeType(TAs[0].getAsType());
Sema::isPointerLikeType(TAs[0].getAsType());
}
return false;
}

static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee) {
if (auto *Conv = dyn_cast_or_null<CXXConversionDecl>(Callee))
if (isRecordWithAttr<PointerAttr>(Conv->getConversionType()) &&
if (Sema::isRecordWithAttr<PointerAttr>(Conv->getConversionType()) &&
Callee->getParent()->hasAttr<OwnerAttr>())
return true;
if (!isInStlNamespace(Callee->getParent()))
return false;
if (!isRecordWithAttr<PointerAttr>(
if (!Sema::isRecordWithAttr<PointerAttr>(
Callee->getFunctionObjectParameterType()) &&
!isRecordWithAttr<OwnerAttr>(Callee->getFunctionObjectParameterType()))
!Sema::isRecordWithAttr<OwnerAttr>(
Callee->getFunctionObjectParameterType()))
return false;
if (isPointerLikeType(Callee->getReturnType())) {
if (Sema::isPointerLikeType(Callee->getReturnType())) {
if (!Callee->getIdentifier())
return false;
return llvm::StringSwitch<bool>(Callee->getName())
Expand Down Expand Up @@ -363,7 +353,7 @@ static bool shouldTrackFirstArgument(const FunctionDecl *FD) {
if (!RD->hasAttr<PointerAttr>() && !RD->hasAttr<OwnerAttr>())
return false;
if (FD->getReturnType()->isPointerType() ||
isRecordWithAttr<PointerAttr>(FD->getReturnType())) {
Sema::isRecordWithAttr<PointerAttr>(FD->getReturnType())) {
return llvm::StringSwitch<bool>(FD->getName())
.Cases("begin", "rbegin", "cbegin", "crbegin", true)
.Cases("end", "rend", "cend", "crend", true)
Expand Down Expand Up @@ -435,7 +425,7 @@ shouldTrackFirstArgumentForConstructor(const CXXConstructExpr *Ctor) {
return true;

// RHS must be an owner.
if (!isRecordWithAttr<OwnerAttr>(RHSArgType))
if (!Sema::isRecordWithAttr<OwnerAttr>(RHSArgType))
return false;

// Bail out if the RHS is Owner<Pointer>.
Expand Down Expand Up @@ -562,7 +552,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())) {
!Sema::isRecordWithAttr<OwnerAttr>(ReturnType->getPointeeType())) {
for (const IndirectLocalPathEntry &PE : llvm::reverse(Path)) {
if (PE.Kind == IndirectLocalPathEntry::GslReferenceInit ||
PE.Kind == IndirectLocalPathEntry::LifetimeBoundCall)
Expand Down Expand Up @@ -1108,7 +1098,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()) ||
(Sema::isRecordWithAttr<PointerAttr>(Entity.LHS->getType()) ||
isAssignmentOperatorLifetimeBound(Entity.AssignmentOperator)));
}

Expand Down Expand Up @@ -1143,12 +1133,12 @@ checkExprLifetimeImpl(Sema &SemaRef, const InitializedEntity *InitEntity,
// someContainer.add(std::move(localUniquePtr));
// return p;
if (pathContainsInit(Path) ||
!isRecordWithAttr<OwnerAttr>(L->getType()))
!Sema::isRecordWithAttr<OwnerAttr>(L->getType()))
return false;
} else {
IsGslPtrValueFromGslTempOwner =
MTE && !MTE->getExtendingDecl() &&
isRecordWithAttr<OwnerAttr>(MTE->getType());
Sema::isRecordWithAttr<OwnerAttr>(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 variable/param.
Expand Down Expand Up @@ -1277,7 +1267,7 @@ checkExprLifetimeImpl(Sema &SemaRef, const InitializedEntity *InitEntity,
auto *DRE = dyn_cast<DeclRefExpr>(L);
// Suppress false positives for code like the one below:
// Ctor(unique_ptr<T> up) : pointer(up.get()), owner(move(up)) {}
if (DRE && isRecordWithAttr<OwnerAttr>(DRE->getType()))
if (DRE && Sema::isRecordWithAttr<OwnerAttr>(DRE->getType()))
return false;

auto *VD = DRE ? dyn_cast<VarDecl>(DRE->getDecl()) : nullptr;
Expand Down Expand Up @@ -1438,7 +1428,7 @@ checkExprLifetimeImpl(Sema &SemaRef, const InitializedEntity *InitEntity,
break;
}
case LK_LifetimeCapture: {
if (isPointerLikeType(Init->getType()))
if (Sema::isPointerLikeType(Init->getType()))
Path.push_back({IndirectLocalPathEntry::GslPointerInit, Init});
break;
}
Expand Down
19 changes: 6 additions & 13 deletions clang/lib/Sema/SemaAttr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,11 @@ void Sema::inferGslOwnerPointerAttribute(CXXRecordDecl *Record) {
inferGslPointerAttribute(Record, Record);
}

bool Sema::isPointerLikeType(QualType Type) {
return isRecordWithAttr<PointerAttr>(Type) || Type->isPointerType() ||
Type->isNullPtrType();
}

void Sema::inferLifetimeBoundAttribute(FunctionDecl *FD) {
if (FD->getNumParams() == 0)
return;
Expand Down Expand Up @@ -269,18 +274,6 @@ void Sema::inferLifetimeBoundAttribute(FunctionDecl *FD) {
}
}

static bool isPointerLikeType(QualType QT) {
QT = QT.getNonReferenceType();
if (QT->isPointerType())
return true;
auto *RD = QT->getAsCXXRecordDecl();
if (!RD)
return false;
if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(RD))
RD = CTSD->getSpecializedTemplate()->getTemplatedDecl();
return RD->hasAttr<PointerAttr>();
}

void Sema::inferLifetimeCaptureByAttribute(FunctionDecl *FD) {
if (!FD)
return;
Expand All @@ -299,7 +292,7 @@ void Sema::inferLifetimeCaptureByAttribute(FunctionDecl *FD) {
if (PVD->hasAttr<LifetimeCaptureByAttr>())
return;
for (ParmVarDecl *PVD : MD->parameters()) {
if (isPointerLikeType(PVD->getType())) {
if (isPointerLikeType(PVD->getType().getNonReferenceType())) {
int CaptureByThis[] = {LifetimeCaptureByAttr::THIS};
PVD->addAttr(
LifetimeCaptureByAttr::CreateImplicit(Context, CaptureByThis, 1));
Expand Down