[PATCH] D66179: [LifetimeAnalysis] Support more STL idioms (template forward declaration and DependentNameType)

[Matthias Gehre via Phabricator via cfe-commits]

mgehre marked 4 inline comments as done.
mgehre added a comment.

I now start to wonder if we should instead put the attribute on all declarations (instead of just the canonical). Later redeclarations automatically inherit the attributes.
This would make both the checking easier (just check any declaration, no need to obtain the canonical first), and remove the special
case for adding to the definition of templated record for ClassTemplateDecls.



================
Comment at: clang/lib/Sema/SemaAttr.cpp:200
     CXXRecordDecl *Canonical = Record->getCanonicalDecl();
     if (Canonical->hasAttr<OwnerAttr>() || Canonical->hasAttr<PointerAttr>())
       return;
----------------
gribozavr wrote:
> mgehre wrote:
> > gribozavr wrote:
> > > Should this code not do this short-circuit check? It is prone to going out of sync with the code in `addGslOwnerPointerAttributeIfNotExisting`, like it did just now.
> > > 
> > > If you want to check for attribute before doing the string search, you could pass the string set into `addGslOwnerPointerAttributeIfNotExisting`, and let that decide if it should infer the attribute or not.
> > Yes, the `hasAttr` check here is an optimization to avoid the string search. I don't think I can move the string search into `addGslOwnerPointerAttributeIfNotExisting`, because that function is called 
> > from other call sites that don't care about a set of names.
> Sorry I wasn't clear enough -- the issue that I noticed is that this check looks at the canonical decl, while `addGslOwnerPointerAttributeIfNotExisting` attaches the attribute to the decl itself -- that's what I meant by "out of sync".
I make sure that `addGslOwnerPointerAttributeIfNotExisting` is only called with the canonical decl as argument, which the caller usually has around. The only exception to this is when addGslOwnerPointerAttributeIfNotExisting calls itself to attach an attribute to the definition of templated class.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66179/new/

https://reviews.llvm.org/D66179