compnerd marked an inline comment as done.
compnerd added inline comments.
================
Comment at: clang/include/clang/Basic/Attr.td:2121
+def SwiftObjCMembers : Attr {
+ let Spellings = [GNU<"swift_objc_members">];
----------------
aaron.ballman wrote:
> Should this be inherited by redeclarations, or is that not a thing with
> `ObjCInterfaceDecl`s?
Objective-C interfaces cannot be redeclared, so this actually is correct I
believe.
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7546
+ case ParsedAttr::AT_SwiftObjCMembers:
+ handleSimpleAttribute<SwiftObjCMembersAttr>(S, D, AL);
+ break;
----------------
aaron.ballman wrote:
> Does it matter if the user writes this attribute on an interface that exposes
> no members? e.g., are there warnings we may want to give the user about using
> the attribute in a weird way? (Sorry if this should be obvious, but I don't
> have experience with Swift.)
I think that the name is not exactly helpful in this case, but that ship has
sailed since this is in production already. AIUI the attribute should apply to
empty interfaces as it results in the Swift interface being exposed back to
Objective-C.
================
Comment at: clang/test/SemaObjC/attr-swift_objc_members.m:4
+#if !__has_attribute(swift_objc_members)
+#error cannot verify precense of swift_objc_members attribute
+#endif
----------------
aaron.ballman wrote:
> gribozavr2 wrote:
> > aaron.ballman wrote:
> > > gribozavr2 wrote:
> > > >
> > > The typo fix makes sense to me, but for my own understanding, why switch
> > > to a string literal?
> > IIUC, as it is now, the message is tokenized by the lexer -- and I think
> > that's not the intent, none of these words are program code.
> Interesting and somewhat different from my understanding. My mental model for
> `#error` is that it "replays" the tokens into the diagnostic message up to
> the end of the line. Given that I prefer my diagnostics to be `warning: you
> did the wrong thing` and not `warning: "you did the wrong thing"` (with
> quotes), I usually leave the quotes off so that the error looks more
> consistent with other errors.
>
> Neither form is more right than the other in this case, so I don't really
> care for this review (I was interested in it as a standards committee member
> who recently had to look at the specification for `#error` though).
FWIW, the reason for the warning not being quoted currently is exactly what
@aaron.ballman stated ... that is how I process the `#error` directive as well,
and I tend to leave the quotes off to make the error match the other
diagnostics. Is the quoting really that important?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87395/new/
https://reviews.llvm.org/D87395
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
