void added inline comments.
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:8550-8556
+ case ParsedAttr::AT_RandomizeLayout:
+ handleSimpleAttribute<RandomizeLayoutAttr>(S, D, AL);
+ break;
+ case ParsedAttr::AT_NoRandomizeLayout:
+ // Drop the "randomize_layout" attribute if it's on the decl.
+ D->dropAttr<RandomizeLayoutAttr>();
+ break;
----------------
aaron.ballman wrote:
> void wrote:
> > aaron.ballman wrote:
> > > I don't think this is sufficient. Consider redeclaration merging cases:
> > > ```
> > > struct [[gnu::randomize_layout]] S;
> > > struct [[gnu::no_randomize_layout]] S {};
> > >
> > > struct [[gnu::no_randomize_layout]] T;
> > > struct [[gnu::randomize_layout]] T {};
> > > ```
> > > I think if the user accidentally does something like this, it should be
> > > diagnosed. I would have assumed this would warrant an error diagnostic
> > > because the user is obviously confused as to what they want, but it seems
> > > GCC ignores the subsequent diagnostic with a warning:
> > > https://godbolt.org/z/1q8dazYPW.
> > >
> > > There's also the "confused attribute list" case which GCC just... does...
> > > things... with: https://godbolt.org/z/rnfsn7hG1. I think we want to
> > > behave more consistently with how we treat these cases.
> > >
> > > Either way, we shouldn't be silent.
> > The GCC feature is done via a plugin in Linux. So godbolt probably won't
> > work in this case. I'll check to see how GCC responds to these attribute
> > situations.
> Thoughts on this?
Okay, finally tested it with the GCC plugin. It doesn't produce a diagnostic.
I'm not sure if that's the correct behavior, but at least it matches. How is
such a thing handled with similar attributes?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121556/new/
https://reviews.llvm.org/D121556
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
