[clang] Introduce [[clang::lifetime_capture_by(X)]]

[usx95]

This implements the RFC https://discourse.llvm.org/t/rfc-introduce-clang-lifetime-capture-by-x/81371

In this PR, we introduce [[clang::lifetime_capture_by(X)]] attribute as discussed in the RFC.

As an implementation detail of this attribute, we store and use param indices instead of raw param expressions. The parameter indices are computed lazily at the end of function declaration since the function decl (and therefore the subsequent parameters) are not visible yet while parsing a parameter annotation.

In subsequent PR, we will infer this attribute for STL containers and perform lifetime analysis to detect dangling cases.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[usx95]

cc: @bricknerb @ilya-biryukov in case you are interested in taking an initial look.

[bricknerb]

Move this change outside this pull request?

[bricknerb]

Can we name CEntity with something more readable?
Perhaps even CaptEntity?

[bricknerb]

IIUC, this function handles different use cases and for different use cases it needs different args to be set.
Since it's becoming more complex with this change, would it make sense to split it to different functions and only reuse the shared logic between the different use cases?
I think this could simplify the asserts and make the calling this method much clearer and safer.

[bricknerb]

Consider making this initialization clearer by writing .Expression = Capturing

[bricknerb]

Is sv here referring to sv in (1) or a typo?

[hokein]

I think it is a typo, it refers to the setsv function argument.

[bricknerb]

I think that using CapturingEntity for both use cases, with one of them keeping a field null makes it much harder to follow.
Is it possible to use a more specific type?

[hokein]

+1.

I'd not change the existing the AssignedEntity, as this structure is designed for the assignment case.

It looks like for the capture_by case, we only need the Expression, I think we could use Expr* directly in the function parameter, e.g. void checkCaptureLifetime(Sema &SemaRef, const Expr* Entity, Expr *Captured);.

[bricknerb]

Do we not have emplace* methods on purpose?

[hokein]

we should probably add category to the dangling group (line 463).

[hokein]

why does UNKNOWN share the same value as GLOBAL? It would be nice to have some documentation about these fields.

[usx95]

This was because GLOBAL and UNKNOWN would have the same enforcements. That is no temporary irrespective of the capturing entity.
I will index differently to avoid confusion.

[hokein]

I think it is a typo, it refers to the setsv function argument.

[hokein]

Why do we need to handle template specializations specifically? The GSL pointer/owner attribute should propagate from the primary template to its specializations.

[hokein]

+1.

I'd not change the existing the AssignedEntity, as this structure is designed for the assignment case.

It looks like for the capture_by case, we only need the Expression, I think we could use Expr* directly in the function parameter, e.g. void checkCaptureLifetime(Sema &SemaRef, const Expr* Entity, Expr *Captured);.

[hokein]

we have a similar function in the CheckExprLifetime.cpp, we can move it to Sema.h to make it reusable.

[hokein]

any reason to comment it out? I think having this should fix some FIXMEs in the test.

[hokein]

I think the [] operator for map-like container is an important case, code like below is probably not rare:

std::map<string_view, ...> m;
m[ReturnString(..)] = ...; // this leaves a dangling string_view key in the map.

[Xazax-hun]

Hey!

Thanks for working on this! Some high level comments based on the description:

1. Class inherited from a pointer-like type.

In this false positive example slicing is happening and we know that the created type (after slicing) is a pointer. Do we always get these false positives with slicing? If that is the case, could we just suppress the warning when derived-to-base conversion happens where the base is a pointer?

2. Not able to differentiate between pointer-type and value type with templates

I think this is a really serious problem that could hinder usability. I'd prefer to not emit warnings for templates until this one is fixed. Do you think this would make sense or would this suppress all of the interesting cases that you want to catch?

[Xazax-hun]

I did not get to the end of this PR yet, but I was wondering if it would make sense to split it up. One PR could add the attribute with the semantic analysis (e.g., warnings for ill-formed arguments). The second PR could introduce the lifetime analysis part. WDYT?

[Xazax-hun]

I think it should be possible since #108631 was merged.

[Xazax-hun]

Probably out of scope for this PR, but are these comments up to date given that we can also warn when the address of a local escapes the function (as opposed to just temporaries)?

[Xazax-hun]

I wish we had one universal way of indexing parameters in Clang. I think probably most attributes start from 1, but there are APINotes that start from 0. Not really an actionable comment here, more like a rant.

[usx95]

I did not get to the end of this PR yet, but I was wondering if it would make sense to split it up. One PR could add the attribute with the semantic analysis (e.g., warnings for ill-formed arguments). The second PR could introduce the lifetime analysis part. WDYT?

Makes sense. I will do this now.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Utkarsh Saxena (usx95)

Changes

This implements the RFC https://discourse.llvm.org/t/rfc-introduce-clang-lifetime-capture-by-x/81371

In this PR, we introduce [[clang::lifetime_capture_by(X)]] attribute as discussed in the RFC.

As an implementation detail of this attribute, we store and use param indices instead of raw param expressions. The parameter indices are computed lazily at the end of function declaration since the function decl (and therefore the subsequent parameters) are not visible yet while parsing a parameter annotation.

In subsequent PR, we will infer this attribute for STL containers and perform lifetime analysis to detect dangling cases.

---

Full diff: https://github.com/llvm/llvm-project/pull/111499.diff

9 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+3)
• (modified) clang/include/clang/Basic/Attr.td (+34)
• (modified) clang/include/clang/Basic/AttrDocs.td (+62)
• (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+12)
• (modified) clang/include/clang/Sema/Sema.h (+4)
• (modified) clang/lib/AST/TypePrinter.cpp (+8)
• (modified) clang/lib/Sema/SemaDecl.cpp (+1)
• (modified) clang/lib/Sema/SemaDeclAttr.cpp (+103)
• (added) clang/test/SemaCXX/attr-lifetime-capture-by.cpp (+40)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c3424e0e6f34c9..93cce21156634d 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..6884ec1b2a0663 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -1889,6 +1889,40 @@ 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 LangOpts = [CPlusPlus];
+  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*, 1>&& Idents,
+               SmallVector<SourceLocation, 1>&& 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..9ba6b4e4993de3 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -3967,6 +3967,68 @@ Attribute ``trivial_abi`` has no effect in the following cases:
   }];
 }
 
+
+def LifetimeCaptureByDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+    The ``lifetime_capture_by(X)`` attribute on a function parameter or implicit object
+parameter indicates that references to arguments passed to such parameters may be
+captured by the capturing entity ``X``.
+
+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 MSInheritanceDocs : Documentation {
   let Category = DocCatDecl;
   let Heading = "__single_inheritance, __multiple_inheritance, __virtual_inheritance";
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index a5d97d7e545ffd..b42dfce130f4c3 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3383,6 +3383,18 @@ 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 of one of 'this', 'global' or 'unknown'">;
+def err_capture_by_references_itself : Error<"'lifetime_capture_by' argument references itself">;
+
 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..fb8cc994d3294e 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -1760,6 +1760,10 @@ class Sema final : public SemaBase {
   /// Add [[gsl::Pointer]] attributes for std:: types.
   void inferGslPointerAttribute(TypedefNameDecl *TD);
 
+  LifetimeCaptureByAttr *ParseLifetimeCaptureByAttr(const ParsedAttr &AL,
+                                                    StringRef ParamName);
+  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..802b2b0fb07e6b 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,12 @@ void TypePrinter::printAttributedAfter(const AttributedType *T,
     OS << " [[clang::lifetimebound]]";
     return;
   }
+  if (T->getAttrKind() == attr::LifetimeCaptureBy) {
+    // FIXME: Print the attribute arguments once we have a way to retrieve these
+    // here.
+    OS << " [[clang::lifetime_capture_by(...)";
+    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 +1983,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..38e9421a456311 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,105 @@ 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 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;
+      }
+      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 +6744,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/test/SemaCXX/attr-lifetime-capture-by.cpp b/clang/test/SemaCXX/attr-lifetime-capture-by.cpp
new file mode 100644
index 00000000000000..192ec9bec640a5
--- /dev/null
+++ b/clang/test/SemaCXX/attr-lifetime-capture-by.cpp
@@ -0,0 +1,40 @@
+// 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 of one of '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 of one of '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 of one of '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 S& u
+  )
+{
+  s.captureInt(x1);
+}
+
+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, x, s)]])
+  {
+    s.captureInt(x);
+  }
+};

[usx95]

The PR now only includes syntax changes without lifetime analysis. I will address the unaddressed comments here related to analysis in subsequent PR.

[Xazax-hun]

Some nits inline, but overall looks good for me.

[Xazax-hun]

Not necessarily for this PR, but I see this kind of loops many times in Sema code. I wonder if this is an indication that TSI should have a more convenient API to get a specific AttributedTypeLoc.

[hokein]

looks good, just some nits.

[usx95]

I will proceed with landing this. Happy to address future comments in a followup.

[nikic]

I've reverted this change, because it causes a large compile-time regression, see https://llvm-compile-time-tracker.com/compare.php?from=4a68e4cbd2423dcacada8162ab7c4bb8d7f7e2cf&to=8c4331c1abeb33eabf3cdbefa7f2b6e0540e7f4f&stat=instructions:u.

[eugenis]

Please note that this change also introduces new memory leaks on the waterfall:
https://lab.llvm.org/buildbot/#/builders/169/builds/5193

[usx95]

Thanks for the revert. I think I know what caused the compile time regression. Looking into the leak now and I can reproduce with asan.