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@+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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits