[PATCH] D87396: Sema: add support for `__attribute__((__swift_typedef_bridged__))`

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/include/clang/Basic/Attr.td:2121
 
+def SwiftBridgedTypedef : Attr {
+  let Spellings = [GNU<"swift_bridged_typedef">];
----------------
Should this be inherited on redeclarations?


================
Comment at: clang/include/clang/Basic/Attr.td:2123
+  let Spellings = [GNU<"swift_bridged_typedef">];
+  let Subjects = SubjectList<[TypedefName], ErrorDiag, "typedefs">;
+  let Documentation = [SwiftBridgedTypedefDocs];
----------------
Does the default diagnostic text generate something bad that caused you to add `typedefs` here?


================
Comment at: clang/include/clang/Basic/AttrDocs.td:3389
+Swift type (e.g. Swift's ``String``) rather than the Objective-C type as written
+(e.g. ``NSString``).
+  }];
----------------
I think it would be helpful to show the `String` vs `NSString` behavior in a short code example of how to use the attribute.


================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7543
+  case ParsedAttr::AT_SwiftBridgedTypedef:
+    handleSimpleAttribute<SwiftBridgedTypedefAttr>(S, D, AL);
+    break;
----------------
Should there be any type checking that the underlying type of the typedef is a sensible one to bridge?


================
Comment at: clang/test/SemaObjC/attr-swift_bridged_typedef.m:8
+
+struct __attribute__((swift_bridged_typedef)) S {};
+// expected-error at -1 {{'swift_bridged_typedef' attribute only applies to typedefs}}
----------------
Please also add some examples where the attribute has arguments. Also, should this work in Objective-C++ when using a `using` type alias? If so, an example showing that working would also be helpful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87396