rjmccall added inline comments.
================
Comment at: include/clang/Basic/Attr.td:290
-class LangOpt<string name, bit negated = 0> {
+class LangOpt<string name, bit negated = 0, string customCode = ""> {
string Name = name;
----------------
I think there's a grand total of one use of `negated`, so you might as well
rewrite it to use `customCode`; see below.
================
Comment at: include/clang/Basic/Attr.td:306
+def ObjCNonFragileRuntime : LangOpt<"ObjCNonFragileRuntime", 0,
+ "LangOpts.ObjCRuntime.allowsClassStubs()">;
----------------
This is not an appropriate name for this `LangOpt`. How about
`ObjCAllowsClassStubs`?
================
Comment at: include/clang/Basic/AttrDocs.td:1100
+implementations defined for them. This attribute is intended for use in
+Swift generated headers for classes defined in Swift.
+ }];
----------------
We should add something here to make it clear what the evolution story with
this attribute is. Presumably you can't add it to an existing class without
breaking ABI. Are there circumstances under which we're willing to guarantee
that it can be removed (under a sufficiently-recent deployment target)?
================
Comment at: utils/TableGen/ClangAttrEmitter.cpp:3442
+ if (!Code.empty()) {
+ Test += "S.";
+ Test += Code;
----------------
I think the contract with `CustomCode` ought to be that `LangOpts` will be
bound as an unqualified name, so that the custom code can be an arbitrary
expression in terms of that. That will allow e.g. compound and negated
`LangOpt`s to be defined, like `LangOpts.OpenCL && LangOpts.CPlusPlus`. So you
need to generate `auto &LangOpts = S.LangOpts;` at the top of the function, at
least when custom code is present; and you should probably add parens around
the code just to be safe.
Alternatively, you could make the contract that `CustomCode` should contain
`%0` or some other string that'll be expanded to an expression for the language
options, but that sounds pretty complex compared to just binding `LangOpts` in
the context.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59628/new/
https://reviews.llvm.org/D59628
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
