aaron.ballman marked 7 inline comments as done.
aaron.ballman added inline comments.
================
Comment at: lib/Sema/SemaDeclAttr.cpp:8623
+ } else if (A.hasScope()) {
+#define ATTR(A)
+#define ATTR_NAMESPACE(A) .Case(#A, false)
----------------
dblaikie wrote:
> dblaikie wrote:
> > Not sure how it's done elsewhere - but I'd sink these #defines down to
> > immediately before the #include
> I think most other uses of .inc files only #define the macros they need to -
> assuming the others aren't defined, rather than explicitly providing an empty
> definition? (but I'm not sure - I guess that means teh .inc file needs a
> #ifndef X \ #define X #endif - but perhaps .inc files usually have those?)
This file is generated without a default for `ATTR`, so there are a few places
where we have to define it to an empty macro. I could add it to the emitter and
remove a few of these spurious definitions, but that can be done in a follow-up.
================
Comment at: lib/Sema/SemaDeclAttr.cpp:8623-8624
+ } else if (A.hasScope()) {
+#define ATTR(A)
+#define ATTR_NAMESPACE(A) .Case(#A, false)
+ if (llvm::StringSwitch<bool>(A.getScopeName()->getName())
----------------
aaron.ballman wrote:
> dblaikie wrote:
> > dblaikie wrote:
> > > Not sure how it's done elsewhere - but I'd sink these #defines down to
> > > immediately before the #include
> > I think most other uses of .inc files only #define the macros they need to
> > - assuming the others aren't defined, rather than explicitly providing an
> > empty definition? (but I'm not sure - I guess that means teh .inc file
> > needs a #ifndef X \ #define X #endif - but perhaps .inc files usually have
> > those?)
> This file is generated without a default for `ATTR`, so there are a few
> places where we have to define it to an empty macro. I could add it to the
> emitter and remove a few of these spurious definitions, but that can be done
> in a follow-up.
I think it's pretty awkward either way, so I'll go with your approach.
================
Comment at: lib/Sema/SemaDeclAttr.cpp:8629-8630
+ Warning = diag::warn_unknown_attribute_namespace_ignored;
+#undef ATTR_NAMESPACE
+#undef ATTR
+ } else if (A.isC2xAttribute() || A.isCXX11Attribute()) {
----------------
dblaikie wrote:
> Should AttrList.inc handle undefing all its attributes - so all its users
> don't have to?
Good catch -- it already does that for me. I'll remove.
================
Comment at: utils/TableGen/ClangAttrEmitter.cpp:2553
+ void copyAttrNamespaces(std::set<std::string> &S) const {
+ for (const Record *R : Attrs) {
----------------
dblaikie wrote:
> Maybe "addAttrNamespacesTo" (not clear whether this would overwrite the
> contens of the std::set, or add to it)
Good catch, I missed renaming it after a signature change where it was
accepting an iterator instead of the container directly.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60872/new/
https://reviews.llvm.org/D60872
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
