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

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.


================
Comment at: clang/lib/Sema/SemaAttr.cpp:200
     CXXRecordDecl *Canonical = Record->getCanonicalDecl();
     if (Canonical->hasAttr<OwnerAttr>() || Canonical->hasAttr<PointerAttr>())
       return;
----------------
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".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66179