[clang] Reapply "[clang] Introduce [[clang::lifetime_capture_by(X)]] (PR #115823)

Utkarsh Saxena via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 12 07:21:12 PST 2024


https://github.com/usx95 updated https://github.com/llvm/llvm-project/pull/115823

>From 3c233df64906972016c26909263cfd53940d87a0 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Tue, 12 Nov 2024 04:28:37 +0000
Subject: [PATCH 1/2] Reapply "[clang] Introduce
 [[clang::lifetime_capture_by(X)]] (#111499)"

This reverts commit 3a03513fc6ef8f3272d33be19164243c9dbf0452.
---
 clang/docs/ReleaseNotes.rst                   |   3 +
 clang/include/clang/Basic/Attr.td             |  33 ++++++
 clang/include/clang/Basic/AttrDocs.td         |  69 +++++++++++
 .../clang/Basic/DiagnosticSemaKinds.td        |  14 +++
 clang/include/clang/Sema/Sema.h               |   8 ++
 clang/lib/AST/TypePrinter.cpp                 |  15 +++
 clang/lib/Sema/SemaDecl.cpp                   |   1 +
 clang/lib/Sema/SemaDeclAttr.cpp               | 111 ++++++++++++++++++
 clang/lib/Sema/SemaType.cpp                   |  13 ++
 clang/test/AST/attr-lifetime-capture-by.cpp   |   9 ++
 .../test/SemaCXX/attr-lifetime-capture-by.cpp |  46 ++++++++
 11 files changed, 322 insertions(+)
 create mode 100644 clang/test/AST/attr-lifetime-capture-by.cpp
 create mode 100644 clang/test/SemaCXX/attr-lifetime-capture-by.cpp

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 4ef48bed58d95c..482b30848b57e8 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -449,6 +449,9 @@ Attribute Changes in Clang
 - Fix a bug where clang doesn't automatically apply the ``[[gsl::Owner]]`` or
   ``[[gsl::Pointer]]`` to STL explicit template specialization decls. (#GH109442)
 
+- Clang now supports ``[[clang::lifetime_capture_by(X)]]``. Similar to lifetimebound, this can be
+  used to specify when a reference to a function parameter is captured by another capturing entity ``X``.
+
 Improvements to Clang's diagnostics
 -----------------------------------
 
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index a631e81d40aa68..6a77967c32cbcb 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -1889,6 +1889,39 @@ def LifetimeBound : DeclOrTypeAttr {
   let SimpleHandler = 1;
 }
 
+def LifetimeCaptureBy : DeclOrTypeAttr {
+  let Spellings = [Clang<"lifetime_capture_by", 0>];
+  let Subjects = SubjectList<[ParmVar, ImplicitObjectParameter], ErrorDiag>;
+  let Args = [VariadicParamOrParamIdxArgument<"Params">];
+  let Documentation = [LifetimeCaptureByDocs];
+  let AdditionalMembers = [{
+private:
+  SmallVector<IdentifierInfo*, 1> ArgIdents;
+  SmallVector<SourceLocation, 1> ArgLocs;
+
+public:
+  static constexpr int THIS = 0;
+  static constexpr int INVALID = -1;
+  static constexpr int UNKNOWN = -2;
+  static constexpr int GLOBAL = -3;
+
+  void setArgs(SmallVector<IdentifierInfo*>&& Idents,
+               SmallVector<SourceLocation>&& Locs) { 
+    assert(Idents.size() == Locs.size());
+    assert(Idents.size() == params_Size);
+    ArgIdents = std::move(Idents);
+    ArgLocs = std::move(Locs);
+  }
+  
+  ArrayRef<IdentifierInfo*> getArgIdents() const { return ArgIdents; }
+  ArrayRef<SourceLocation> getArgLocs() const { return ArgLocs; }
+  void setParamIdx(size_t Idx, int Val) { 
+    assert(Idx < params_Size);
+    params_[Idx] = Val;
+  }
+}];
+}
+
 def TrivialABI : InheritableAttr {
   // This attribute does not have a C [[]] spelling because it requires the
   // CPlusPlus language option.
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index b64dbef6332e6a..21fcd183e8969c 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -3918,6 +3918,75 @@ have their lifetimes extended.
   }];
 }
 
+def LifetimeCaptureByDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+    Similar to `lifetimebound`_, the ``lifetime_capture_by(X)`` attribute on a function
+parameter or implicit object parameter indicates that that objects that are referred to
+by that parameter may also be referred to by the capturing entity ``X``.
+
+By default, a reference is considered to refer to its referenced object, a
+pointer is considered to refer to its pointee, a ``std::initializer_list<T>``
+is considered to refer to its underlying array, and aggregates (arrays and
+simple ``struct``\s) are considered to refer to all objects that their
+transitive subobjects refer to.
+
+The capturing entity ``X`` can be one of the following:
+- Another (named) function parameter. 
+  
+  .. code-block:: c++
+
+    void addToSet(std::string_view a [[clang::lifetime_capture_by(s)]], std::set<std::string_view>& s) {
+      s.insert(a);
+    }
+
+- ``this`` (in case of member functions).
+  
+  .. code-block:: c++
+
+    class S {
+      void addToSet(std::string_view a [[clang::lifetime_capture_by(this)]]) {
+        s.insert(a);
+      }
+      std::set<std::string_view> s;
+    };
+
+- 'global', 'unknown' (without quotes).
+  
+  .. code-block:: c++
+
+    std::set<std::string_view> s;
+    void addToSet(std::string_view a [[clang::lifetime_capture_by(global)]]) {
+      s.insert(a);
+    }
+    void addSomewhere(std::string_view a [[clang::lifetime_capture_by(unknown)]]);
+
+The attribute can be applied to the implicit ``this`` parameter of a member
+function by writing the attribute after the function type:
+
+.. code-block:: c++
+
+  struct S {
+    const char *data(std::set<S*>& s) [[clang::lifetime_capture_by(s)]] {
+      s.insert(this);
+    }
+  };
+
+The attribute supports specifying more than one capturing entities:
+
+.. code-block:: c++
+  
+  void addToSets(std::string_view a [[clang::lifetime_capture_by(s1, s2)]],
+                 std::set<std::string_view>& s1,
+                 std::set<std::string_view>& s2) {
+    s1.insert(a);
+    s2.insert(a);
+  }
+
+.. _`lifetimebound`: https://clang.llvm.org/docs/AttributeReference.html#lifetimebound
+  }];
+}
+
 def TrivialABIDocs : Documentation {
   let Category = DocCatDecl;
   let Content = [{
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index a5d97d7e545ffd..f4452fbb57e736 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3383,6 +3383,20 @@ def err_callback_callee_is_variadic : Error<
   "'callback' attribute callee may not be variadic">;
 def err_callback_implicit_this_not_available : Error<
   "'callback' argument at position %0 references unavailable implicit 'this'">;
+
+def err_capture_by_attribute_multiple : Error<
+  "multiple 'lifetime_capture' attributes specified">;
+def err_capture_by_attribute_no_entity : Error<
+  "'lifetime_capture_by' attribute specifies no capturing entity">;
+def err_capture_by_implicit_this_not_available : Error<
+  "'lifetime_capture_by' argument references unavailable implicit 'this'">;
+def err_capture_by_attribute_argument_unknown : Error<
+  "'lifetime_capture_by' attribute argument %0 is not a known function parameter"
+  "; must be a function parameter, 'this', 'global' or 'unknown'">;
+def err_capture_by_references_itself : Error<"'lifetime_capture_by' argument references itself">;
+def err_capture_by_param_uses_reserved_name : Error<
+  "parameter cannot be named '%select{global|unknown}0' while using 'lifetime_capture_by(%select{global|unknown}0)'">;
+
 def err_init_method_bad_return_type : Error<
   "init methods must return an object pointer type, not %0">;
 def err_attribute_invalid_size : Error<
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index fad446a05e782f..d6f3508a5243f3 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -1760,6 +1760,14 @@ class Sema final : public SemaBase {
   /// Add [[gsl::Pointer]] attributes for std:: types.
   void inferGslPointerAttribute(TypedefNameDecl *TD);
 
+  LifetimeCaptureByAttr *ParseLifetimeCaptureByAttr(const ParsedAttr &AL,
+                                                    StringRef ParamName);
+  // Processes the argument 'X' in [[clang::lifetime_capture_by(X)]]. Since 'X'
+  // can be the name of a function parameter, we need to parse the function
+  // declaration and rest of the parameters before processesing 'X'. Therefore
+  // do this lazily instead of processing while parsing the annotation itself.
+  void LazyProcessLifetimeCaptureByParams(FunctionDecl *FD);
+
   /// Add _Nullable attributes for std:: types.
   void inferNullableClassAttribute(CXXRecordDecl *CRD);
 
diff --git a/clang/lib/AST/TypePrinter.cpp b/clang/lib/AST/TypePrinter.cpp
index 6d8db5cf4ffd22..a073a6a4b7d454 100644
--- a/clang/lib/AST/TypePrinter.cpp
+++ b/clang/lib/AST/TypePrinter.cpp
@@ -25,6 +25,7 @@
 #include "clang/AST/TextNodeDumper.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/AddressSpaces.h"
+#include "clang/Basic/AttrKinds.h"
 #include "clang/Basic/ExceptionSpecificationType.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/LLVM.h"
@@ -1909,6 +1910,19 @@ void TypePrinter::printAttributedAfter(const AttributedType *T,
     OS << " [[clang::lifetimebound]]";
     return;
   }
+  if (T->getAttrKind() == attr::LifetimeCaptureBy) {
+    OS << " [[clang::lifetime_capture_by(";
+    if (auto *attr = dyn_cast_or_null<LifetimeCaptureByAttr>(T->getAttr())) {
+      auto Idents = attr->getArgIdents();
+      for (unsigned I = 0; I < Idents.size(); ++I) {
+        OS << Idents[I]->getName();
+        if (I != Idents.size() - 1)
+          OS << ", ";
+      }
+    }
+    OS << ")]]";
+    return;
+  }
 
   // The printing of the address_space attribute is handled by the qualifier
   // since it is still stored in the qualifier. Return early to prevent printing
@@ -1976,6 +1990,7 @@ void TypePrinter::printAttributedAfter(const AttributedType *T,
   case attr::SizedBy:
   case attr::SizedByOrNull:
   case attr::LifetimeBound:
+  case attr::LifetimeCaptureBy:
   case attr::TypeNonNull:
   case attr::TypeNullable:
   case attr::TypeNullableResult:
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 61c29e320d5c73..a3bc8e4191c819 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -16687,6 +16687,7 @@ void Sema::AddKnownFunctionAttributes(FunctionDecl *FD) {
     }
   }
 
+  LazyProcessLifetimeCaptureByParams(FD);
   inferLifetimeBoundAttribute(FD);
   AddKnownFunctionAttributesForReplaceableGlobalAllocationFunction(FD);
 
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index d05d326178e1b8..b4aaa58c082002 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -14,6 +14,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTMutationListener.h"
 #include "clang/AST/CXXInheritance.h"
+#include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
@@ -3867,6 +3868,113 @@ static void handleCallbackAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
       S.Context, AL, EncodingIndices.data(), EncodingIndices.size()));
 }
 
+LifetimeCaptureByAttr *Sema::ParseLifetimeCaptureByAttr(const ParsedAttr &AL,
+                                                        StringRef ParamName) {
+  // Atleast one capture by is required.
+  if (AL.getNumArgs() == 0) {
+    Diag(AL.getLoc(), diag::err_capture_by_attribute_no_entity)
+        << AL.getRange();
+    return nullptr;
+  }
+  SmallVector<IdentifierInfo *, 1> ParamIdents;
+  SmallVector<SourceLocation, 1> ParamLocs;
+  for (unsigned I = 0; I < AL.getNumArgs(); ++I) {
+    if (AL.isArgExpr(I)) {
+      Expr *E = AL.getArgAsExpr(I);
+      Diag(E->getExprLoc(), diag::err_capture_by_attribute_argument_unknown)
+          << E << E->getExprLoc();
+      continue;
+    }
+    assert(AL.isArgIdent(I));
+    IdentifierLoc *IdLoc = AL.getArgAsIdent(I);
+    if (IdLoc->Ident->getName() == ParamName) {
+      Diag(IdLoc->Loc, diag::err_capture_by_references_itself) << IdLoc->Loc;
+      continue;
+    }
+    ParamIdents.push_back(IdLoc->Ident);
+    ParamLocs.push_back(IdLoc->Loc);
+  }
+  SmallVector<int, 1> FakeParamIndices(ParamIdents.size(),
+                                       LifetimeCaptureByAttr::INVALID);
+  LifetimeCaptureByAttr *CapturedBy = ::new (Context) LifetimeCaptureByAttr(
+      Context, AL, FakeParamIndices.data(), FakeParamIndices.size());
+  CapturedBy->setArgs(std::move(ParamIdents), std::move(ParamLocs));
+  return CapturedBy;
+}
+
+static void HandleLifetimeCaptureByAttr(Sema &S, Decl *D,
+                                        const ParsedAttr &AL) {
+  // Do not allow multiple attributes.
+  if (D->hasAttr<LifetimeCaptureByAttr>()) {
+    S.Diag(AL.getLoc(), diag::err_capture_by_attribute_multiple)
+        << AL.getRange();
+    return;
+  }
+  auto *PVD = dyn_cast<ParmVarDecl>(D);
+  assert(PVD);
+  auto *CaptureByAttr = S.ParseLifetimeCaptureByAttr(AL, PVD->getName());
+  if (CaptureByAttr)
+    D->addAttr(CaptureByAttr);
+}
+
+void Sema::LazyProcessLifetimeCaptureByParams(FunctionDecl *FD) {
+  bool HasImplicitThisParam = isInstanceMethod(FD);
+
+  llvm::StringMap<int> NameIdxMapping;
+  NameIdxMapping["global"] = LifetimeCaptureByAttr::GLOBAL;
+  NameIdxMapping["unknown"] = LifetimeCaptureByAttr::UNKNOWN;
+  int Idx = 0;
+  if (HasImplicitThisParam) {
+    NameIdxMapping["this"] = 0;
+    Idx++;
+  }
+  for (const ParmVarDecl *PVD : FD->parameters())
+    NameIdxMapping[PVD->getName()] = Idx++;
+  auto DisallowReservedParams = [&](StringRef Reserved) {
+    for (const ParmVarDecl *PVD : FD->parameters())
+      if (PVD->getName() == Reserved)
+        Diag(PVD->getLocation(), diag::err_capture_by_param_uses_reserved_name)
+            << (PVD->getName() == "unknown");
+  };
+  auto HandleCaptureBy = [&](LifetimeCaptureByAttr *CapturedBy) {
+    if (!CapturedBy)
+      return;
+    const auto &Entities = CapturedBy->getArgIdents();
+    for (size_t I = 0; I < Entities.size(); ++I) {
+      StringRef Name = Entities[I]->getName();
+      auto It = NameIdxMapping.find(Name);
+      if (It == NameIdxMapping.end()) {
+        auto Loc = CapturedBy->getArgLocs()[I];
+        if (!HasImplicitThisParam && Name == "this")
+          Diag(Loc, diag::err_capture_by_implicit_this_not_available) << Loc;
+        else
+          Diag(Loc, diag::err_capture_by_attribute_argument_unknown)
+              << Entities[I] << Loc;
+        continue;
+      }
+      if (Name == "unknown" || Name == "global")
+        DisallowReservedParams(Name);
+      CapturedBy->setParamIdx(I, It->second);
+    }
+  };
+  for (ParmVarDecl *PVD : FD->parameters())
+    HandleCaptureBy(PVD->getAttr<LifetimeCaptureByAttr>());
+  if (!HasImplicitThisParam)
+    return;
+  TypeSourceInfo *TSI = FD->getTypeSourceInfo();
+  if (!TSI)
+    return;
+  AttributedTypeLoc ATL;
+  for (TypeLoc TL = TSI->getTypeLoc();
+       (ATL = TL.getAsAdjusted<AttributedTypeLoc>());
+       TL = ATL.getModifiedLoc()) {
+    auto *A = ATL.getAttrAs<LifetimeCaptureByAttr>();
+    if (!A)
+      continue;
+    HandleCaptureBy(const_cast<LifetimeCaptureByAttr *>(A));
+  }
+}
+
 static bool isFunctionLike(const Type &T) {
   // Check for explicit function types.
   // 'called_once' is only supported in Objective-C and it has
@@ -6644,6 +6752,9 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
   case ParsedAttr::AT_Callback:
     handleCallbackAttr(S, D, AL);
     break;
+  case ParsedAttr::AT_LifetimeCaptureBy:
+    HandleLifetimeCaptureByAttr(S, D, AL);
+    break;
   case ParsedAttr::AT_CalledOnce:
     handleCalledOnceAttr(S, D, AL);
     break;
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 515b9f689a248a..eb7516b3ef1ece 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -8609,6 +8609,15 @@ static void HandleLifetimeBoundAttr(TypeProcessingState &State,
   }
 }
 
+static void HandleLifetimeCaptureByAttr(TypeProcessingState &State,
+                                        QualType &CurType, ParsedAttr &PA) {
+  if (State.getDeclarator().isDeclarationOfFunction()) {
+    auto *Attr = State.getSema().ParseLifetimeCaptureByAttr(PA, "this");
+    if (Attr)
+      CurType = State.getAttributedType(Attr, CurType, CurType);
+  }
+}
+
 static void HandleHLSLParamModifierAttr(TypeProcessingState &State,
                                         QualType &CurType,
                                         const ParsedAttr &Attr, Sema &S) {
@@ -8770,6 +8779,10 @@ static void processTypeAttrs(TypeProcessingState &state, QualType &type,
       if (TAL == TAL_DeclChunk)
         HandleLifetimeBoundAttr(state, type, attr);
       break;
+    case ParsedAttr::AT_LifetimeCaptureBy:
+      if (TAL == TAL_DeclChunk)
+        HandleLifetimeCaptureByAttr(state, type, attr);
+      break;
 
     case ParsedAttr::AT_NoDeref: {
       // FIXME: `noderef` currently doesn't work correctly in [[]] syntax.
diff --git a/clang/test/AST/attr-lifetime-capture-by.cpp b/clang/test/AST/attr-lifetime-capture-by.cpp
new file mode 100644
index 00000000000000..da2eb0cf3d592e
--- /dev/null
+++ b/clang/test/AST/attr-lifetime-capture-by.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 %s -ast-dump | FileCheck %s
+
+// Verify that we print the [[clang::lifetime_capture_by(X)]] attribute.
+
+struct S {
+    void foo(int &a, int &b) [[clang::lifetime_capture_by(a, b, global)]];
+};
+
+// CHECK: CXXMethodDecl {{.*}}clang::lifetime_capture_by(a, b, global)
diff --git a/clang/test/SemaCXX/attr-lifetime-capture-by.cpp b/clang/test/SemaCXX/attr-lifetime-capture-by.cpp
new file mode 100644
index 00000000000000..3115dc8d6150c9
--- /dev/null
+++ b/clang/test/SemaCXX/attr-lifetime-capture-by.cpp
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -std=c++23 -verify %s
+
+struct S {
+  const int *x;
+  void captureInt(const int&x [[clang::lifetime_capture_by(this)]]) { this->x = &x; }
+};
+
+///////////////////////////
+// Test for valid usages.
+///////////////////////////
+[[clang::lifetime_capture_by(unknown)]] // expected-error {{'lifetime_capture_by' attribute only applies to parameters and implicit object parameters}}
+void nonMember(
+    const int &x1 [[clang::lifetime_capture_by(s, t)]],
+    S &s,
+    S &t,
+    const int &x2 [[clang::lifetime_capture_by(12345 + 12)]], // expected-error {{'lifetime_capture_by' attribute argument 12345 + 12 is not a known function parameter; must be a function parameter, 'this', 'global' or 'unknown'}}
+    const int &x3 [[clang::lifetime_capture_by(abcdefgh)]],   // expected-error {{'lifetime_capture_by' attribute argument 'abcdefgh' is not a known function parameter; must be a function parameter, 'this', 'global' or 'unknown'}}
+    const int &x4 [[clang::lifetime_capture_by("abcdefgh")]], // expected-error {{'lifetime_capture_by' attribute argument "abcdefgh" is not a known function parameter; must be a function parameter, 'this', 'global' or 'unknown'}}
+    const int &x5 [[clang::lifetime_capture_by(this)]], // expected-error {{'lifetime_capture_by' argument references unavailable implicit 'this'}}
+    const int &x6 [[clang::lifetime_capture_by()]], // expected-error {{'lifetime_capture_by' attribute specifies no capturing entity}}
+    const int& x7 [[clang::lifetime_capture_by(u, 
+                                               x7)]], // expected-error {{'lifetime_capture_by' argument references itself}}
+    const int &x8 [[clang::lifetime_capture_by(global)]],
+    const int &x9 [[clang::lifetime_capture_by(unknown)]],
+    const S& u
+  )
+{
+  s.captureInt(x1);
+}
+
+void unknown_param_name(const int& unknown, // expected-error {{parameter cannot be named 'unknown' while using 'lifetime_capture_by(unknown)'}}
+                        const int& s [[clang::lifetime_capture_by(unknown)]]);
+void global_param_name(const int& global, // expected-error {{parameter cannot be named 'global' while using 'lifetime_capture_by(global)'}}
+                       const int& s [[clang::lifetime_capture_by(global)]]);
+struct T {
+  void member(
+    const int &x [[clang::lifetime_capture_by(s)]], 
+    S &s,
+    S &t,            
+    const int &y [[clang::lifetime_capture_by(s)]],
+    const int &z [[clang::lifetime_capture_by(this, x, y)]],
+    const int &u [[clang::lifetime_capture_by(global, unknown, x, s)]])
+  {
+    s.captureInt(x);
+  }
+};

>From 826c5562980cb39c3d6b1a89a4e1d2a076407741 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Tue, 12 Nov 2024 06:18:54 +0000
Subject: [PATCH 2/2] Fix compile time regression and memory leak

---
 clang/include/clang/Basic/Attr.td             | 19 +++--
 clang/lib/Sema/SemaDeclAttr.cpp               | 71 ++++++++++---------
 .../test/SemaCXX/attr-lifetime-capture-by.cpp |  1 +
 3 files changed, 47 insertions(+), 44 deletions(-)

diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 6a77967c32cbcb..fc79f847236f6c 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -1895,9 +1895,9 @@ def LifetimeCaptureBy : DeclOrTypeAttr {
   let Args = [VariadicParamOrParamIdxArgument<"Params">];
   let Documentation = [LifetimeCaptureByDocs];
   let AdditionalMembers = [{
-private:
-  SmallVector<IdentifierInfo*, 1> ArgIdents;
-  SmallVector<SourceLocation, 1> ArgLocs;
+public:
+  IdentifierInfo** ArgIdents;
+  SourceLocation* ArgLocs;
 
 public:
   static constexpr int THIS = 0;
@@ -1905,16 +1905,13 @@ public:
   static constexpr int UNKNOWN = -2;
   static constexpr int GLOBAL = -3;
 
-  void setArgs(SmallVector<IdentifierInfo*>&& Idents,
-               SmallVector<SourceLocation>&& Locs) { 
-    assert(Idents.size() == Locs.size());
-    assert(Idents.size() == params_Size);
-    ArgIdents = std::move(Idents);
-    ArgLocs = std::move(Locs);
+  void setArgs(IdentifierInfo **Idents, SourceLocation *Locs) {
+    ArgIdents = Idents;
+    ArgLocs = Locs;
   }
   
-  ArrayRef<IdentifierInfo*> getArgIdents() const { return ArgIdents; }
-  ArrayRef<SourceLocation> getArgLocs() const { return ArgLocs; }
+  auto getArgIdents() const { return ArrayRef<IdentifierInfo*>(ArgIdents, params_Size); }
+  auto getArgLocs() const { return ArrayRef<SourceLocation>(ArgLocs, params_Size); }
   void setParamIdx(size_t Idx, int Val) { 
     assert(Idx < params_Size);
     params_[Idx] = Val;
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index b4aaa58c082002..03b57c5e9440fd 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -65,6 +65,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Demangle/Demangle.h"
 #include "llvm/IR/Assumptions.h"
+#include "llvm/IR/DerivedTypes.h"
 #include "llvm/MC/MCSectionMachO.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/MathExtras.h"
@@ -3876,33 +3877,38 @@ LifetimeCaptureByAttr *Sema::ParseLifetimeCaptureByAttr(const ParsedAttr &AL,
         << AL.getRange();
     return nullptr;
   }
-  SmallVector<IdentifierInfo *, 1> ParamIdents;
-  SmallVector<SourceLocation, 1> ParamLocs;
-  for (unsigned I = 0; I < AL.getNumArgs(); ++I) {
+  unsigned N = AL.getNumArgs();
+  IdentifierInfo **ParamIdents = new (Context) IdentifierInfo *[N];
+  SourceLocation *ParamLocs = new (Context) SourceLocation[N];
+  bool IsValid = true;
+  for (unsigned I = 0; I < N; ++I) {
     if (AL.isArgExpr(I)) {
       Expr *E = AL.getArgAsExpr(I);
       Diag(E->getExprLoc(), diag::err_capture_by_attribute_argument_unknown)
           << E << E->getExprLoc();
+      IsValid = false;
       continue;
     }
     assert(AL.isArgIdent(I));
     IdentifierLoc *IdLoc = AL.getArgAsIdent(I);
     if (IdLoc->Ident->getName() == ParamName) {
       Diag(IdLoc->Loc, diag::err_capture_by_references_itself) << IdLoc->Loc;
+      IsValid = false;
       continue;
     }
-    ParamIdents.push_back(IdLoc->Ident);
-    ParamLocs.push_back(IdLoc->Loc);
+    ParamIdents[I] = IdLoc->Ident;
+    ParamLocs[I] = IdLoc->Loc;
   }
-  SmallVector<int, 1> FakeParamIndices(ParamIdents.size(),
-                                       LifetimeCaptureByAttr::INVALID);
+  if (!IsValid)
+    return nullptr;
+  SmallVector<int> FakeParamIndices(N, LifetimeCaptureByAttr::INVALID);
   LifetimeCaptureByAttr *CapturedBy = ::new (Context) LifetimeCaptureByAttr(
       Context, AL, FakeParamIndices.data(), FakeParamIndices.size());
-  CapturedBy->setArgs(std::move(ParamIdents), std::move(ParamLocs));
+  CapturedBy->setArgs(ParamIdents, ParamLocs);
   return CapturedBy;
 }
 
-static void HandleLifetimeCaptureByAttr(Sema &S, Decl *D,
+static void handleLifetimeCaptureByAttr(Sema &S, Decl *D,
                                         const ParsedAttr &AL) {
   // Do not allow multiple attributes.
   if (D->hasAttr<LifetimeCaptureByAttr>()) {
@@ -3919,10 +3925,27 @@ static void HandleLifetimeCaptureByAttr(Sema &S, Decl *D,
 
 void Sema::LazyProcessLifetimeCaptureByParams(FunctionDecl *FD) {
   bool HasImplicitThisParam = isInstanceMethod(FD);
-
-  llvm::StringMap<int> NameIdxMapping;
-  NameIdxMapping["global"] = LifetimeCaptureByAttr::GLOBAL;
-  NameIdxMapping["unknown"] = LifetimeCaptureByAttr::UNKNOWN;
+  SmallVector<LifetimeCaptureByAttr *, 1> Attrs;
+  for (ParmVarDecl *PVD : FD->parameters())
+    if (auto *A = PVD->getAttr<LifetimeCaptureByAttr>())
+      Attrs.push_back(A);
+  if (HasImplicitThisParam) {
+    TypeSourceInfo *TSI = FD->getTypeSourceInfo();
+    if (!TSI)
+      return;
+    AttributedTypeLoc ATL;
+    for (TypeLoc TL = TSI->getTypeLoc();
+         (ATL = TL.getAsAdjusted<AttributedTypeLoc>());
+         TL = ATL.getModifiedLoc()) {
+      if (auto *A = ATL.getAttrAs<LifetimeCaptureByAttr>())
+        Attrs.push_back(const_cast<LifetimeCaptureByAttr *>(A));
+    }
+  }
+  if (Attrs.empty())
+    return;
+  llvm::StringMap<int> NameIdxMapping = {
+      {"global", LifetimeCaptureByAttr::GLOBAL},
+      {"unknown", LifetimeCaptureByAttr::UNKNOWN}};
   int Idx = 0;
   if (HasImplicitThisParam) {
     NameIdxMapping["this"] = 0;
@@ -3936,9 +3959,7 @@ void Sema::LazyProcessLifetimeCaptureByParams(FunctionDecl *FD) {
         Diag(PVD->getLocation(), diag::err_capture_by_param_uses_reserved_name)
             << (PVD->getName() == "unknown");
   };
-  auto HandleCaptureBy = [&](LifetimeCaptureByAttr *CapturedBy) {
-    if (!CapturedBy)
-      return;
+  for (auto *CapturedBy : Attrs) {
     const auto &Entities = CapturedBy->getArgIdents();
     for (size_t I = 0; I < Entities.size(); ++I) {
       StringRef Name = Entities[I]->getName();
@@ -3956,22 +3977,6 @@ void Sema::LazyProcessLifetimeCaptureByParams(FunctionDecl *FD) {
         DisallowReservedParams(Name);
       CapturedBy->setParamIdx(I, It->second);
     }
-  };
-  for (ParmVarDecl *PVD : FD->parameters())
-    HandleCaptureBy(PVD->getAttr<LifetimeCaptureByAttr>());
-  if (!HasImplicitThisParam)
-    return;
-  TypeSourceInfo *TSI = FD->getTypeSourceInfo();
-  if (!TSI)
-    return;
-  AttributedTypeLoc ATL;
-  for (TypeLoc TL = TSI->getTypeLoc();
-       (ATL = TL.getAsAdjusted<AttributedTypeLoc>());
-       TL = ATL.getModifiedLoc()) {
-    auto *A = ATL.getAttrAs<LifetimeCaptureByAttr>();
-    if (!A)
-      continue;
-    HandleCaptureBy(const_cast<LifetimeCaptureByAttr *>(A));
   }
 }
 
@@ -6753,7 +6758,7 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
     handleCallbackAttr(S, D, AL);
     break;
   case ParsedAttr::AT_LifetimeCaptureBy:
-    HandleLifetimeCaptureByAttr(S, D, AL);
+    handleLifetimeCaptureByAttr(S, D, AL);
     break;
   case ParsedAttr::AT_CalledOnce:
     handleCalledOnceAttr(S, D, AL);
diff --git a/clang/test/SemaCXX/attr-lifetime-capture-by.cpp b/clang/test/SemaCXX/attr-lifetime-capture-by.cpp
index 3115dc8d6150c9..be1cd730ed40ce 100644
--- a/clang/test/SemaCXX/attr-lifetime-capture-by.cpp
+++ b/clang/test/SemaCXX/attr-lifetime-capture-by.cpp
@@ -22,6 +22,7 @@ void nonMember(
                                                x7)]], // expected-error {{'lifetime_capture_by' argument references itself}}
     const int &x8 [[clang::lifetime_capture_by(global)]],
     const int &x9 [[clang::lifetime_capture_by(unknown)]],
+    const int &test_memory_leak[[clang::lifetime_capture_by(x1,x2, x3, x4, x5, x6, x7, x8, x9)]],
     const S& u
   )
 {



More information about the cfe-commits mailing list