[clang] Propagate lifetimebound from formal parameters to those in the canonical declaration and use that for analysis (PR #107627)

Ilya Biryukov via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 18 10:17:22 PST 2024


================
@@ -266,6 +262,20 @@ void Sema::inferLifetimeBoundAttribute(FunctionDecl *FD) {
               LifetimeBoundAttr::CreateImplicit(Context, FD->getLocation()));
       }
     }
+  } else if (auto *CanonDecl = FD->getCanonicalDecl(); FD != CanonDecl) {
+    // Propagate the lifetimebound attribute from parameters to the canonical
+    // declaration.
+    // Note that this doesn't include the implicit 'this' parameter, as the
+    // attribute is applied to the function type in that case.
+    const unsigned int NP = std::min(NumParams, CanonDecl->getNumParams());
+    for (unsigned int I = 0; I < NP; ++I) {
+      auto *CanonParam = CanonDecl->getParamDecl(I);
+      if (!CanonParam->hasAttr<LifetimeBoundAttr>() &&
+          FD->getParamDecl(I)->hasAttr<LifetimeBoundAttr>()) {
----------------
ilya-biryukov wrote:

Should we be looking at the attributes of the most recent decl instead, then?
How do other mergable attributes handle that?

I think it's fairly important to have a single place in the AST that handles the merging logic for the attributes, even if that means changing some mechanism of how other places work.

Looking at that test, I guess what happens is:
- the attributes get propagated to the **new** redeclaration, but never to the original declaration;
- the canonical declaration is the old one that does not have the attribute;
- when checking for the lifetimebound attribute, we look at canonical declaration and don't notice the attribute.

I will look into the code, but could you check if it makes sense to put something like `getMostRecentDecl()` calls when checking for presence of the attributes?

PS the meta-suggestion here is to not add new ways to merge attributes. I am not sure if it there is a new way that would make more sense, e.g. maybe we can explore merging all the attributes into a canonical declaration instead or ensuring there is a helper function that calls `getMostRecentDecl` before checking for an attribute being present.

https://github.com/llvm/llvm-project/pull/107627


More information about the cfe-commits mailing list