[PATCH] D87395: Sema: add support for `__attribute__((__swift_objc_members__))`

[Saleem Abdulrasool via Phabricator via cfe-commits]

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