aaron.ballman added a comment.
Thank you for working on this patch -- be sure to add tests that ensure the
attribute is properly prohibited on the expected targets.
================
Comment at: include/clang/Basic/Attr.td:299-300
list<string> ObjectFormats;
+ // It indicates that a certain attribute is not supported on a specific
+ // platform, turn on support for this attribute by default
+ bit Negated = 0;
----------------
How about: `If set to false, indicates the attribute is supported only on the
given target. If set to true, indicates the attribute is supported on all
targets except the given target.`
================
Comment at: include/clang/Basic/Attr.td:324-325
+// We do not support the alias attribute on Apple platforms,
+// so I define a platform other than the Apple platform.
+def TargetDarwinNegative : TargetArch<["x86", "x86_64"]> {
----------------
This comment is a bit too specific, I'd probably drop the comment entirely as
the code is sufficiently clear.
================
Comment at: include/clang/Basic/Attr.td:326
+// so I define a platform other than the Apple platform.
+def TargetDarwinNegative : TargetArch<["x86", "x86_64"]> {
+ let OSes = ["MacOSX"];
----------------
I think we'll want to pattern the names like `TargetNotFoo`.
One other thing is: this isn't really Darwin -- for instance, there's ARM and
AArch64 variants, not just x86-based targets.
================
Comment at: include/clang/Basic/Attr.td:327
+def TargetDarwinNegative : TargetArch<["x86", "x86_64"]> {
+ let OSes = ["MacOSX"];
+ let Negated = 1;
----------------
I would expect the OS would be named "Darwin" and not "MacOSX" given the name
of the target. Which flavor should be used, or should both be used?
================
Comment at: utils/TableGen/ClangAttrEmitter.cpp:2789
+ // is not supported on this platform
+ if(R->getValueAsBit("Negated"))
+ Test = "!(" + Test + ")";
----------------
The formatting is off here -- there should be a space after the `if`. I would
recommend running your patch through clang-format
(https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting).
Repository:
rC Clang
https://reviews.llvm.org/D46805
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
