[LifetimeSafety] Move GSL pointer/owner type detection to LifetimeAnnotations (#169620)

usx95 · web-flow · commit c43ac96331bc · 2025-11-26T16:43:03.000+05:30
Refactored GSL pointer and owner type detection functions to improve code organization and reusability.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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));
