aaron.ballman created this revision.
aaron.ballman added a reviewer: compnerd.
aaron.ballman requested review of this revision.
Herald added a project: clang.

The `swift_bridge` attribute warns when the attribute is applied multiple times 
to the same declaration. However, it warns about the arguments being different 
to the attribute without ever checking if the arguments actually are different. 
If the arguments are different, diagnose, otherwise silently accept the code. 
Either way, drop the duplicated attribute.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96130

Files:
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/SemaObjC/attr-swift_bridge.m


Index: clang/test/SemaObjC/attr-swift_bridge.m
===================================================================
--- clang/test/SemaObjC/attr-swift_bridge.m
+++ clang/test/SemaObjC/attr-swift_bridge.m
@@ -31,3 +31,8 @@
 typedef NSArray *NSArrayAlias __attribute__((__swift_bridge__("ArrayAlias")));
 
 struct __attribute__((__swift_bridge__("StructT"))) T {};
+
+// Duplicate attributes with the same arguments are fine.
+struct __attribute__((swift_bridge("foo"), swift_bridge("foo"))) S;
+// Duplicate attributes with different arguments are not.
+struct __attribute__((swift_bridge("foo"), swift_bridge("bar"))) S; // 
expected-warning {{attribute 'swift_bridge' is already applied with different 
arguments}}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===================================================================
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -5751,9 +5751,11 @@
   if (!S.checkStringLiteralArgumentAttr(AL, 0, BT))
     return;
 
-  // Don't duplicate annotations that are already set.
-  if (D->hasAttr<SwiftBridgeAttr>()) {
-    S.Diag(AL.getLoc(), diag::warn_duplicate_attribute) << AL;
+  // Warn about duplicate attributes if they have different arguments, but drop
+  // any duplicate attributes regardless.
+  if (const auto *Other = D->getAttr<SwiftBridgeAttr>()) {
+    if (Other->getSwiftType() != BT)
+      S.Diag(AL.getLoc(), diag::warn_duplicate_attribute) << AL;
     return;
   }
 


Index: clang/test/SemaObjC/attr-swift_bridge.m
===================================================================
--- clang/test/SemaObjC/attr-swift_bridge.m
+++ clang/test/SemaObjC/attr-swift_bridge.m
@@ -31,3 +31,8 @@
 typedef NSArray *NSArrayAlias __attribute__((__swift_bridge__("ArrayAlias")));
 
 struct __attribute__((__swift_bridge__("StructT"))) T {};
+
+// Duplicate attributes with the same arguments are fine.
+struct __attribute__((swift_bridge("foo"), swift_bridge("foo"))) S;
+// Duplicate attributes with different arguments are not.
+struct __attribute__((swift_bridge("foo"), swift_bridge("bar"))) S; // expected-warning {{attribute 'swift_bridge' is already applied with different arguments}}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===================================================================
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -5751,9 +5751,11 @@
   if (!S.checkStringLiteralArgumentAttr(AL, 0, BT))
     return;
 
-  // Don't duplicate annotations that are already set.
-  if (D->hasAttr<SwiftBridgeAttr>()) {
-    S.Diag(AL.getLoc(), diag::warn_duplicate_attribute) << AL;
+  // Warn about duplicate attributes if they have different arguments, but drop
+  // any duplicate attributes regardless.
+  if (const auto *Other = D->getAttr<SwiftBridgeAttr>()) {
+    if (Other->getSwiftType() != BT)
+      S.Diag(AL.getLoc(), diag::warn_duplicate_attribute) << AL;
     return;
   }
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D96130: Correct... Aaron Ballman via Phabricator via cfe-commits

Reply via email to