[PATCH] D60872: Add new warning knob for unknown attribute namespaces

[David Blaikie via Phabricator via cfe-commits]

dblaikie 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:
> 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?)


================
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())
----------------
Not sure how it's done elsewhere - but I'd sink these #defines down to immediately before the #include


================
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()) {
----------------
Should AttrList.inc handle undefing all its attributes - so all its users don't have to?


================
Comment at: utils/TableGen/ClangAttrEmitter.cpp:2553
 
+    void copyAttrNamespaces(std::set<std::string> &S) const {
+      for (const Record *R : Attrs) {
----------------
Maybe "addAttrNamespacesTo" (not clear whether this would overwrite the contens of the std::set, or add to it)


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

https://reviews.llvm.org/D60872