Filter bad attrs out of module interface @abis

When printing an `@abi` attribute’s decl, we now filter out any attrs that are not valid in that position. Fixes a broken test.

Author: beccadax

include/swift/AST/AttrKind.h

@@ -145,6 +145,8 @@ enum class DeclAttrKind : unsigned {
 #include "swift/AST/DeclAttr.def"
 };
 
+StringRef getDeclAttrKindID(DeclAttrKind kind);
+
 enum : unsigned {
   NumDeclAttrKinds = static_cast<unsigned>(DeclAttrKind::Last_DeclAttr) + 1,
   NumDeclAttrKindBits = countBitsUsed(NumDeclAttrKinds - 1),

lib/AST/ASTPrinter.cpp

@@ -1284,6 +1284,15 @@ void PrintAST::printAttributes(const Decl *D) {
   // for each decl They cannot be shared across different decls.
   assert(Options.ExcludeCustomAttrList.empty());
 
+  // If there is an `@abi` attr, we need to print it first so that it isn't
+  // affected by subsequent mutation of `Options.ExcludeAttrList`.
+  if (auto abiAttr = attrs.getAttribute<ABIAttr>()) {
+    if (Options.PrintImplicitAttrs && !Options.excludeAttr(abiAttr)) {
+      abiAttr->print(Printer, Options, D);
+      Options.ExcludeAttrList.push_back(DeclAttrKind::ABI);
+    }
+  }
+
   if (Options.PrintSyntheticSILGenName
         && !D->getAttrs().hasAttribute<SILGenNameAttr>()) {
     if (canPrintSyntheticSILGenName(D)) {
@@ -1334,7 +1343,8 @@ void PrintAST::printAttributes(const Decl *D) {
     // Add SPIs to both private and package interfaces
     if (!Options.printPublicInterface() &&
         DeclAttribute::canAttributeAppearOnDeclKind(
-            DeclAttrKind::SPIAccessControl, D->getKind())) {
+            DeclAttrKind::SPIAccessControl, D->getKind()) &&
+        !Options.excludeAttrKind(DeclAttrKind::SPIAccessControl)) {
       interleave(D->getSPIGroups(),
              [&](Identifier spiName) {
                Printer.printAttrName("_spi", true);
@@ -1358,7 +1368,7 @@ void PrintAST::printAttributes(const Decl *D) {
     // If the declaration is implicitly @objc, print the attribute now.
     if (auto VD = dyn_cast<ValueDecl>(D)) {
       if (VD->isObjC() && !isa<EnumElementDecl>(VD) &&
-          !attrs.hasAttribute<ObjCAttr>()) {
+          !attrs.hasAttribute<ObjCAttr>() && ABIRoleInfo(D).providesAPI()) {
         Printer.printAttrName("@objc");
         Printer << " ";
       }

lib/AST/Attr.cpp

@@ -68,6 +68,15 @@ static_assert(IsTriviallyDestructible<DeclAttributes>::value,
   static_assert(TypeAttrKind::Id <= TypeAttrKind::Last_TypeAttr);
 #include "swift/AST/TypeAttr.def"
 
+LLVM_ATTRIBUTE_USED StringRef swift::getDeclAttrKindID(DeclAttrKind kind) {
+    switch (kind) {
+#define DECL_ATTR(_, CLASS, ...)                               \
+  case DeclAttrKind::CLASS:                                    \
+    return #CLASS;
+#include "swift/AST/DeclAttr.def"
+  }
+}
+
 StringRef swift::getAccessLevelSpelling(AccessLevel value) {
   switch (value) {
   case AccessLevel::Private: return "private";
@@ -1688,8 +1697,26 @@ bool DeclAttribute::printImpl(ASTPrinter &Printer, const PrintOptions &Options,
       abiDecl = cast<PatternBindingDecl>(abiDecl)
                     ->getVarAtSimilarStructuralPosition(
                                       const_cast<VarDecl *>(cast<VarDecl>(D)));
-    if (abiDecl)
-      abiDecl->print(Printer, Options);
+    if (abiDecl) {
+      auto optionsCopy = Options;
+
+      // Don't print any attributes marked with `ForbiddenInABIAttr`.
+      // (Reminder: There is manual logic in `PrintAST::printAttributes()`
+      // to handle non-ABI attributes when `PrintImplicitAttrs` is set.)
+      for (auto rawAttrKind : range(0, unsigned(DeclAttrKind::Last_DeclAttr))) {
+        DeclAttrKind attrKind{rawAttrKind}; 
+        if (!(DeclAttribute::getBehaviors(attrKind)
+                & DeclAttribute::ForbiddenInABIAttr))
+          continue;
+
+        if (attrKind == DeclAttrKind::AccessControl)
+          optionsCopy.PrintAccess = false;
+        else
+          optionsCopy.ExcludeAttrList.push_back(attrKind);
+      }
+
+      abiDecl->print(Printer, optionsCopy);
+    }
     Printer << ")";
 
     break;
@@ -1754,7 +1781,7 @@ uint64_t DeclAttribute::getBehaviors(DeclAttrKind DK) {
     return BEHAVIORS;
 #include "swift/AST/DeclAttr.def"
   }
-  llvm_unreachable("bad DeclAttrKind");
+  return 0;
 }
 
 std::optional<Feature> DeclAttribute::getRequiredFeature(DeclAttrKind DK) {

test/ModuleInterface/attrs.swift

@@ -1,10 +1,13 @@
 // RUN: %target-swift-emit-module-interface(%t.swiftinterface) %s -module-name attrs \
+// RUN:  -emit-private-module-interface-path %t.private.swiftinterface \
 // RUN:  -enable-experimental-feature ABIAttribute \
 // RUN:  -enable-experimental-feature ExecutionAttribute
 
 // RUN: %target-swift-typecheck-module-from-interface(%t.swiftinterface) -module-name attrs
+// RUN: %target-swift-typecheck-module-from-interface(%t.private.swiftinterface) -module-name attrs
 
-// RUN: %FileCheck %s --input-file %t.swiftinterface
+// RUN: %FileCheck %s --check-prefixes CHECK,PUBLIC-CHECK --input-file %t.swiftinterface
+// RUN: %FileCheck %s --check-prefixes CHECK,PRIVATE-CHECK --input-file %t.private.swiftinterface
 
 // REQUIRES: swift_feature_ABIAttribute
 // REQUIRES: swift_feature_ExecutionAttribute
@@ -35,26 +38,55 @@ internal func __specialize_someGenericFunction<T>(_ t: T) -> Int {
     fatalError("don't call")
 }
 
-@abi(public func __abi__abiAttrOnFunction(param: Int))
+@abi(func __abi__abiAttrOnFunction(param: Int))
 public func abiAttrOnFunction(param: Int) {}
 // CHECK: #if {{.*}} $ABIAttribute
-// CHECK: @abi(public func __abi__abiAttrOnFunction(param: Swift.Int))
+// CHECK: @abi(func __abi__abiAttrOnFunction(param: Swift.Int))
 // CHECK: public func abiAttrOnFunction(param: Swift.Int)
 // CHECK: #else
 // CHECK: @_silgen_name("$s5attrs07__abi__B14AttrOnFunction5paramySi_tF")
 // CHECK: public func abiAttrOnFunction(param: Swift.Int)
 // CHECK: #endif
 
-@abi(public let __abi__abiAttrOnVar: Int)
+@abi(let __abi__abiAttrOnVar: Int)
 public var abiAttrOnVar: Int = 42
 // CHECK: #if {{.*}} $ABIAttribute
-// CHECK: @abi(public var __abi__abiAttrOnVar: Swift.Int)
+// CHECK: @abi(var __abi__abiAttrOnVar: Swift.Int)
 // CHECK: public var abiAttrOnVar: Swift.Int
 // CHECK: #else
 // CHECK: @available(*, unavailable, message: "this compiler cannot match the ABI specified by the @abi attribute")
 // CHECK: public var abiAttrOnVar: Swift.Int
 // CHECK: #endif
 
+public struct MutatingTest {
+  // CHECK: #if {{.*}} $ABIAttribute
+  // CHECK: @abi(mutating func abiMutFunc())
+  // CHECK: public mutating func abiMutFunc()
+  // CHECK: #else
+  // CHECK: @_silgen_name("$s5attrs12MutatingTestV10abiMutFuncyyF")
+  // CHECK: public mutating func abiMutFunc()
+  // CHECK: #endif
+  @abi(mutating func abiMutFunc())
+  public mutating func abiMutFunc() {}
+}
+
+// PUBLIC-CHECK-NOT: #if {{.*}} $ABIAttribute
+// PUBLIC-CHECK-NOT: @abi(func abiSpiFunc())
+// PUBLIC-CHECK-NOT: public func abiSpiFunc()
+// PUBLIC-CHECK-NOT: #else
+// PUBLIC-CHECK-NOT: @_silgen_name("$s5attrs10abiSpiFuncyyF")
+// PUBLIC-CHECK-NOT: public func abiSpiFunc()
+// PUBLIC-CHECK-NOT: #endif
+// PRIVATE-CHECK: #if {{.*}} $ABIAttribute
+// PRIVATE-CHECK: @abi(func abiSpiFunc())
+// PRIVATE-CHECK: public func abiSpiFunc()
+// PRIVATE-CHECK: #else
+// PRIVATE-CHECK: @_silgen_name("$s5attrs10abiSpiFuncyyF")
+// PRIVATE-CHECK: public func abiSpiFunc()
+// PRIVATE-CHECK: #endif
+@abi(func abiSpiFunc())
+@_spi(spiGroup) public func abiSpiFunc() {}
+
 @execution(concurrent)
 public func testExecutionConcurrent() async {}
 // CHECK: @execution(concurrent) public func testExecutionConcurrent() async

test/ModuleInterface/attrs_objc.swift

@@ -0,0 +1,45 @@
+// RUN: %target-swift-emit-module-interface(%t.swiftinterface) %s \
+// RUN:  -enable-objc-interop -module-name attrs_objc \
+// RUN:  -enable-experimental-feature ABIAttribute
+
+// RUN: %target-swift-typecheck-module-from-interface(%t.swiftinterface) -module-name attrs_objc
+
+// RUN: %FileCheck %s --input-file %t.swiftinterface
+
+// REQUIRES: objc_interop
+// REQUIRES: swift_feature_ABIAttribute
+
+import Foundation
+
+@objcMembers
+public class ObjCTest: NSObject {
+  // CHECK: #if {{.*}} $ABIAttribute
+  // CHECK: @abi(func abiObjCFunc())
+  // CHECK: @objc public func abiObjCFunc()
+  // CHECK: #else
+  // CHECK: @_silgen_name("$s10attrs_objc8ObjCTestC03abiC5CFuncyyF")
+  // CHECK: @objc public func abiObjCFunc()
+  // CHECK: #endif
+  @abi(func abiObjCFunc())
+  @objc public func abiObjCFunc() {}
+
+  // CHECK: #if {{.*}} $ABIAttribute
+  // CHECK: @abi(func abiImplicitObjCFunc())
+  // CHECK: @objc public func abiImplicitObjCFunc()
+  // CHECK: #else
+  // CHECK: @_silgen_name("$s10attrs_objc8ObjCTestC011abiImplicitC5CFuncyyF")
+  // CHECK: @objc public func abiImplicitObjCFunc()
+  // CHECK: #endif
+  @abi(func abiImplicitObjCFunc())
+  public func abiImplicitObjCFunc() {}
+
+  // CHECK: #if {{.*}} $ABIAttribute
+  // CHECK: @abi(func abiIBActionFunc(_: Any))
+  // CHECK: @objc @IBAction @_Concurrency.MainActor @preconcurrency public func abiIBActionFunc(_: Any)
+  // CHECK: #else
+  // CHECK: @_silgen_name("$s10attrs_objc8ObjCTestC15abiIBActionFuncyyypF")
+  // CHECK: @objc @IBAction @_Concurrency.MainActor @preconcurrency public func abiIBActionFunc(_: Any)
+  // CHECK: #endif
+  @abi(func abiIBActionFunc(_: Any))
+  @IBAction public func abiIBActionFunc(_: Any) {}
+}