[PATCH] D87532: Sema: add support for `__attribute__((__swift_bridge__))`

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/include/clang/Basic/Attr.td:2133
 
+def SwiftBridge : Attr {
+  let Spellings = [GNU<"swift_bridge">];
----------------
Is this a type or a declaration attribute? It looks like a declaration attribute based on the declaration and the list of subjects, but it looks like a type based on the `ExpectedType` diagnostic and the documentation. Or is this one of those unholy GNU attributes that's confused about what it appertains to?

Should this be inherited by redeclarations? Might be worth adding a test:
```
struct __attribute__((swift_bridge)) S;

struct S { // Should still have the attribute
  int i;
};
```


================
Comment at: clang/include/clang/Basic/AttrDocs.td:3483
+  let Content = [{
+The ``swift_bridged`` attribute indicates that the type to which the attribute
+appertains is bridged to the named Swift type.
----------------
If this is a type attribute, should it be listed as a `TypeAttr` above?


================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5535
+  // Don't duplicate annotations that are already set.
+  if (D->hasAttr<SwiftBridgeAttr>()) {
+    S.Diag(AL.getLoc(), diag::warn_duplicate_attribute) << AL.getAttrName();
----------------
Can a type bridge across multiple types? e.g., could you say this bridges to "foo" and "bar"?


================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5536
+  if (D->hasAttr<SwiftBridgeAttr>()) {
+    S.Diag(AL.getLoc(), diag::warn_duplicate_attribute) << AL.getAttrName();
+    return;
----------------
Can drop the `getAttrName()` and just pass in `AL` directly.


================
Comment at: clang/test/SemaObjC/attr-swift_bridged_typedef.m:11
+
+// expected-error at +1 {{'__swift_bridge__' attribute takes one argument}}
+__attribute__((__swift_bridge__))
----------------
I'd appreciate a test that shows it diagnosing when given the wrong subject.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87532