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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
