aaron.ballman added inline comments.
================
Comment at: clang/include/clang/Basic/Attr.td:1797
+def NoUniqueAddressMSVC : InheritableAttr,
TargetSpecificAttr<TargetMicrosoftCXXABI> {
+ let Spellings = [CXX11<"msvc", "no_unique_address", 201803>];
----------------
erichkeane wrote:
> I think I would combine this and the one above as a single attribute. Then,
> only 'add' it in SemaDeclAttr.cpp when the spelling is the appropriate one
> for the current target. IMO, msvc::no_unique_address should ONLY be valid in
> MSVC mode, else we encourage its propagation.
>
> We don't need it to be a 'TargetSpecificAttr', just do the 'ignored' warning
> in the handler in SemaDeclAttr.td.
>
> The accessor that we need is to just query which spelling.
That's the approach I was thinking of, but then we lose some of the declarative
expressiveness from Attr.td, which is unfortunate. But this is a pretty unique
situation and so perhaps it's reasonable and we can try to think of a tablegen
approach once we get a second attribute in the same situation.
================
Comment at: clang/lib/AST/Decl.cpp:4523-4524
bool FieldDecl::isPotentiallyOverlapping() const {
- return hasAttr<NoUniqueAddressAttr>() && getType()->getAsCXXRecordDecl();
+ return (hasAttr<NoUniqueAddressAttr>() ||
+ hasAttr<NoUniqueAddressMSVCAttr>()) &&
+ getType()->getAsCXXRecordDecl();
----------------
akhuang wrote:
> aaron.ballman wrote:
> > dblaikie wrote:
> > > Having to check both of these in several places seems problematic - can
> > > we wrap that up somewhere? (or, maybe ideally, is there a way for
> > > `msvc::no_unique_address` to map to the actual NoUniqueAddressAttr as a
> > > different spelling of the same thing?)
> > This was why I was hoping we could merge the two in Attr.td, but I'm not
> > certain that will be easy.
> What does merging the two in Attr.td mean? Could we just put the two
> spellings in one attribute, or would that make it impossible for clang-cl to
> ignore the [[no_unique_address]] spelling
We can have multiple syntactic spellings for the same semantic attribute (e.g.,
`__attribute__((foo))` and `__attribute__((bar))` can both map to a single
`FooBarAttr` AST node), and we have "accessors" on the AST node that let you
tell which spelling was used:
https://github.com/llvm/llvm-project/blob/90ecadde62f30275c35fdf7928e3477a41691d21/clang/include/clang/Basic/Attr.td#L4095
The suggestion Erich and I are thinking of is:
1) Add the additional spelling to `NoUniqueAddress`.
2) Add accessors to differentiate the spellings.
3) Remove the `TargetSpecificAttr` from `NoUniqueAddress`, manually implement
those checks in an attribute handler in SemaDeclAttr.cpp.
Then, anywhere you care about the attribute in general, you can look for
`isa<NoUniqueAddress>`, and anywhere you care about which spelling, you can use
`cast<NoUniqueAddress>(A)->isMSVC()` (or whatever you name the accessors).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157762/new/
https://reviews.llvm.org/D157762
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
