[PrintAsClang] Ensure that all macro-generated decls get printed.

allevato · allevato · commit eeeb73ad8ad2 · 2024-10-25T10:39:06.000-04:00
Some macro-generated declarations are not being printed in the Obj-C/C++ generated header. Members introduced by attached `member` macros on a type appear to be fine, but those introduced by a attached `peer` or freestanding `declaration` macros don't show up. This change updates the header writer to call `getAllMembers` throughout instead of `getMembers`, which makes sure that everything gets collected. Likewise, we update the top-level logic from `getTopLevelDecls` to `getTopLevelDeclsWithAuxiliaryDecls` to pick up freestanding decls introduced at file scope. Fixes #68170.
diff --git a/lib/PrintAsClang/DeclAndTypePrinter.cpp b/lib/PrintAsClang/DeclAndTypePrinter.cpp
@@ -188,7 +188,7 @@ class DeclAndTypePrinter::Implementation
   }
 
   bool isEmptyExtensionDecl(const ExtensionDecl *ED) {
-    auto members = ED->getMembers();
+    auto members = ED->getAllMembers();
     auto hasMembers = std::any_of(members.begin(), members.end(),
                                   [this](const Decl *D) -> bool {
       if (auto VD = dyn_cast<ValueDecl>(D))
@@ -349,7 +349,7 @@ class DeclAndTypePrinter::Implementation
     if (outputLang == OutputLanguageMode::Cxx) {
       // FIXME: Non objc class.
       ClangClassTypePrinter(os).printClassTypeDecl(
-          CD, [&]() { printMembers(CD->getMembers()); }, owningPrinter);
+          CD, [&]() { printMembers(CD->getAllMembers()); }, owningPrinter);
       recordEmittedDeclInCurrentCxxLexicalScope(CD);
       return;
     }
@@ -389,7 +389,7 @@ class DeclAndTypePrinter::Implementation
       os << " : " << getNameForObjC(superDecl);
     printProtocols(CD->getLocalProtocols(ConformanceLookupKind::OnlyExplicit));
     os << "\n";
-    printMembers(CD->getMembers());
+    printMembers(CD->getAllMembers());
     os << "@end\n";
   }
 
@@ -402,13 +402,13 @@ class DeclAndTypePrinter::Implementation
     printer.printValueTypeDecl(
         SD, /*bodyPrinter=*/
         [&]() {
-          printMembers(SD->getMembers());
+          printMembers(SD->getAllMembers());
           for (const auto *ed :
                owningPrinter.interopContext.getExtensionsForNominalType(SD)) {
             if (!cxx_translation::isExposableToCxx(ed->getGenericSignature()))
               continue;
 
-            printMembers(ed->getMembers());
+            printMembers(ed->getAllMembers());
           }
         },
         owningPrinter);
@@ -428,7 +428,7 @@ class DeclAndTypePrinter::Implementation
     os << " (SWIFT_EXTENSION(" << ED->getModuleContext()->getName() << "))";
     printProtocols(ED->getLocalProtocols(ConformanceLookupKind::OnlyExplicit));
     os << "\n";
-    printMembers(ED->getMembers());
+    printMembers(ED->getAllMembers());
     os << "@end\n";
   }
 
@@ -449,7 +449,7 @@ class DeclAndTypePrinter::Implementation
 
     printProtocols(PD->getInheritedProtocols());
     os << "\n";
-    printMembers(PD->getMembers());
+    printMembers(PD->getAllMembers());
     os << "@end\n";
   }
 
@@ -884,14 +884,14 @@ class DeclAndTypePrinter::Implementation
           os << "  }\n"; // operator cases()'s closing bracket
           os << "\n";
 
-          printMembers(ED->getMembers());
+          printMembers(ED->getAllMembers());
 
           for (const auto *ext :
                owningPrinter.interopContext.getExtensionsForNominalType(ED)) {
             if (!cxx_translation::isExposableToCxx(ext->getGenericSignature()))
               continue;
 
-            printMembers(ext->getMembers());
+            printMembers(ext->getAllMembers());
           }
         },
         owningPrinter);
diff --git a/lib/PrintAsClang/ModuleContentsWriter.cpp b/lib/PrintAsClang/ModuleContentsWriter.cpp
@@ -350,7 +350,8 @@ class ModuleWriter {
     }
   }
 
-  bool forwardDeclareMemberTypes(DeclRange members, const Decl *container) {
+  bool forwardDeclareMemberTypes(ArrayRef<Decl *> members,
+                                 const Decl *container) {
     PrettyStackTraceDecl
         entry("printing forward declarations needed by members of", container);
     switch (container->getKind()) {
@@ -480,7 +481,7 @@ class ModuleWriter {
     if (!allRequirementsSatisfied)
       return false;
 
-    (void)forwardDeclareMemberTypes(CD->getMembers(), CD);
+    (void)forwardDeclareMemberTypes(CD->getAllMembers(), CD);
     auto [it, inserted] =
         seenTypes.try_emplace(CD, EmissionState::NotYetDefined, false);
     if (outputLangMode == OutputLanguageMode::Cxx &&
@@ -515,10 +516,10 @@ class ModuleWriter {
     if (addImport(SD))
       return true;
     if (outputLangMode == OutputLanguageMode::Cxx) {
-      (void)forwardDeclareMemberTypes(SD->getMembers(), SD);
+      (void)forwardDeclareMemberTypes(SD->getAllMembers(), SD);
       for (const auto *ed :
            printer.getInteropContext().getExtensionsForNominalType(SD)) {
-        (void)forwardDeclareMemberTypes(ed->getMembers(), SD);
+        (void)forwardDeclareMemberTypes(ed->getAllMembers(), SD);
       }
       forwardDeclareCxxValueTypeIfNeeded(SD);
     }
@@ -545,7 +546,7 @@ class ModuleWriter {
     if (!allRequirementsSatisfied)
       return false;
 
-    if (!forwardDeclareMemberTypes(PD->getMembers(), PD))
+    if (!forwardDeclareMemberTypes(PD->getAllMembers(), PD))
       return false;
 
     seenTypes[PD] = { EmissionState::Defined, true };
@@ -572,7 +573,7 @@ class ModuleWriter {
     // This isn't rolled up into the previous set of requirements because
     // it /also/ prints forward declarations, and the header is a little
     // prettier if those are as close as possible to the necessary extension.
-    if (!forwardDeclareMemberTypes(ED->getMembers(), ED))
+    if (!forwardDeclareMemberTypes(ED->getAllMembers(), ED))
       return false;
 
     os << '\n';
@@ -585,7 +586,7 @@ class ModuleWriter {
       return true;
 
     if (outputLangMode == OutputLanguageMode::Cxx) {
-      forwardDeclareMemberTypes(ED->getMembers(), ED);
+      forwardDeclareMemberTypes(ED->getAllMembers(), ED);
       forwardDeclareCxxValueTypeIfNeeded(ED);
     }
 
@@ -617,7 +618,7 @@ class ModuleWriter {
 
   void write() {
     SmallVector<Decl *, 64> decls;
-    M.getTopLevelDecls(decls);
+    M.getTopLevelDeclsWithAuxiliaryDecls(decls);
     llvm::DenseSet<const ValueDecl *> removedValueDecls;
 
     auto newEnd =
@@ -649,7 +650,7 @@ class ModuleWriter {
         if (!ext ||
             ext->getExtendedNominal() != M.getASTContext().getStringDecl())
           continue;
-        for (auto *m : ext->getMembers()) {
+        for (auto *m : ext->getAllMembers()) {
           if (auto *sd = dyn_cast<StructDecl>(m)) {
             if (sd->getBaseIdentifier().str() == "UTF8View" ||
                 sd->getBaseIdentifier().str() == "Index") {
@@ -761,8 +762,8 @@ class ModuleWriter {
       // Break ties in extensions by putting smaller extensions last (in reverse
       // order).
       // FIXME: This will end up taking linear time.
-      auto lhsMembers = cast<ExtensionDecl>(*lhs)->getMembers();
-      auto rhsMembers = cast<ExtensionDecl>(*rhs)->getMembers();
+      auto lhsMembers = cast<ExtensionDecl>(*lhs)->getAllMembers();
+      auto rhsMembers = cast<ExtensionDecl>(*rhs)->getAllMembers();
       unsigned numLHSMembers = std::distance(lhsMembers.begin(),
                                              lhsMembers.end());
       unsigned numRHSMembers = std::distance(rhsMembers.begin(),
@@ -794,18 +795,19 @@ class ModuleWriter {
       // Still nothing? Fine, we'll pick the one with the alphabetically first
       // member instead.
       {
-        auto mismatch =
-          std::mismatch(cast<ExtensionDecl>(*lhs)->getMembers().begin(),
-                        cast<ExtensionDecl>(*lhs)->getMembers().end(),
-                        cast<ExtensionDecl>(*rhs)->getMembers().begin(),
-                        [] (const Decl *nextLHSDecl, const Decl *nextRHSDecl) {
-          if (isa<ValueDecl>(nextLHSDecl) && isa<ValueDecl>(nextRHSDecl)) {
-            return cast<ValueDecl>(nextLHSDecl)->getName() !=
-                     cast<ValueDecl>(nextRHSDecl)->getName();
-          }
-          return isa<ValueDecl>(nextLHSDecl) != isa<ValueDecl>(nextRHSDecl);
-        });
-        if (mismatch.first != cast<ExtensionDecl>(*lhs)->getMembers().end()) {
+        auto mismatch = std::mismatch(
+            cast<ExtensionDecl>(*lhs)->getAllMembers().begin(),
+            cast<ExtensionDecl>(*lhs)->getAllMembers().end(),
+            cast<ExtensionDecl>(*rhs)->getAllMembers().begin(),
+            [](const Decl *nextLHSDecl, const Decl *nextRHSDecl) {
+              if (isa<ValueDecl>(nextLHSDecl) && isa<ValueDecl>(nextRHSDecl)) {
+                return cast<ValueDecl>(nextLHSDecl)->getName() !=
+                       cast<ValueDecl>(nextRHSDecl)->getName();
+              }
+              return isa<ValueDecl>(nextLHSDecl) != isa<ValueDecl>(nextRHSDecl);
+            });
+        if (mismatch.first !=
+            cast<ExtensionDecl>(*lhs)->getAllMembers().end()) {
           auto *lhsMember = dyn_cast<ValueDecl>(*mismatch.first),
                *rhsMember = dyn_cast<ValueDecl>(*mismatch.second);
           if (!rhsMember && lhsMember)
diff --git a/test/Macros/Inputs/objc_cxx_macro_definitions.swift b/test/Macros/Inputs/objc_cxx_macro_definitions.swift
@@ -0,0 +1,129 @@
+import SwiftParser
+import SwiftSyntax
+import SwiftSyntaxBuilder
+import SwiftSyntaxMacros
+
+struct ObjCMemberFuncMacro: MemberMacro {
+  static func expansion(
+    of node: AttributeSyntax,
+    providingMembersOf declaration: some DeclGroupSyntax,
+    conformingTo protocols: [TypeSyntax], 
+    in context: some MacroExpansionContext
+  ) throws -> [DeclSyntax] {
+    guard let decl = declaration.asProtocol(NamedDeclSyntax.self) else {
+      return []
+    }
+    return ["@objc public func member_\(raw: decl.name.text)() {}"]
+  }
+}
+
+struct ObjCPeerFuncMacro: PeerMacro {
+  static func expansion(
+    of node: AttributeSyntax,
+    providingPeersOf declaration: some DeclSyntaxProtocol,
+    in context: some MacroExpansionContext
+  ) throws -> [DeclSyntax] {
+    guard let decl = declaration.asProtocol(NamedDeclSyntax.self) else {
+      return []
+    }
+    return ["@objc public func peer_\(raw: decl.name.text)() {}"]
+  }
+}
+
+struct ObjCFreestandingFuncMacro: DeclarationMacro {
+  static func expansion(
+    of node: some FreestandingMacroExpansionSyntax,
+    in context: some MacroExpansionContext
+  ) throws -> [DeclSyntax] {
+    return ["@objc public func member_freestanding() {}"]
+  }
+}
+
+struct ObjCFreestandingClassMacro: DeclarationMacro {
+  static func expansion(
+    of node: some FreestandingMacroExpansionSyntax,
+    in context: some MacroExpansionContext
+  ) throws -> [DeclSyntax] {
+    return ["""
+      @objc public class MacroExpandedObjCClass: NSObject {
+        @objc public func member() {}
+      }
+      """]
+  }
+}
+
+struct CDeclFreestandingFuncMacro: DeclarationMacro {
+  static func expansion(
+    of node: some FreestandingMacroExpansionSyntax,
+    in context: some MacroExpansionContext
+  ) throws -> [DeclSyntax] {
+    return [#"@_cdecl("c_freestanding") public func cFreestanding() {}"#]
+  }
+}
+
+struct ObjCExtensionMacro: ExtensionMacro {
+  static func expansion(
+    of node: AttributeSyntax,
+    attachedTo declaration: some DeclGroupSyntax,
+    providingExtensionsOf type: some TypeSyntaxProtocol,
+    conformingTo protocols: [TypeSyntax],
+    in context: some MacroExpansionContext
+  ) throws -> [ExtensionDeclSyntax] {
+    let decl: DeclSyntax = """
+      extension \(type): MyObjCProtocol {
+        public func objcRequirement() {}
+      }
+      """
+    return [decl.as(ExtensionDeclSyntax.self)!]
+  }
+}
+
+struct MemberFuncMacro: MemberMacro {
+  static func expansion(
+    of node: AttributeSyntax,
+    providingMembersOf declaration: some DeclGroupSyntax,
+    conformingTo protocols: [TypeSyntax], 
+    in context: some MacroExpansionContext
+  ) throws -> [DeclSyntax] {
+    guard let decl = declaration.asProtocol(NamedDeclSyntax.self) else {
+      return []
+    }
+    return ["public func member_\(raw: decl.name.text)() {}"]
+  }
+}
+
+struct PeerFuncMacro: PeerMacro {
+  static func expansion(
+    of node: AttributeSyntax,
+    providingPeersOf declaration: some DeclSyntaxProtocol,
+    in context: some MacroExpansionContext
+  ) throws -> [DeclSyntax] {
+    guard let decl = declaration.asProtocol(NamedDeclSyntax.self) else {
+      return []
+    }
+    return ["public func peer_\(raw: decl.name.text)() {}"]
+  }
+}
+
+struct CxxFreestandingFuncMacro: DeclarationMacro {
+  static func expansion(
+    of node: some FreestandingMacroExpansionSyntax,
+    in context: some MacroExpansionContext
+  ) throws -> [DeclSyntax] {
+    return ["public func cxxFreestanding() {}"]
+  }
+}
+
+struct CxxFreestandingStructMacro: DeclarationMacro {
+  static func expansion(
+    of node: some FreestandingMacroExpansionSyntax,
+    in context: some MacroExpansionContext
+  ) throws -> [DeclSyntax] {
+    return ["""
+      public struct MacroExpandedStruct {
+        private let x: Int = 0
+        public func member() {}
+      }
+      """]
+  }
+}
diff --git a/test/Macros/macro_generated_header_cxx.swift b/test/Macros/macro_generated_header_cxx.swift
@@ -0,0 +1,43 @@
+// REQUIRES: swift_swift_parser
+
+// RUN: %empty-directory(%t)
+// RUN: %host-build-swift -swift-version 5 -emit-library -o %t/%target-library-name(MacroDefinition) -module-name=MacroDefinition %S/Inputs/objc_cxx_macro_definitions.swift -g -no-toolchain-stdlib-rpath
+// RUN: %target-build-swift -swift-version 5 -load-plugin-library %t/%target-library-name(MacroDefinition) %s -o %t/main -module-name MacroUser -swift-version 5
+// RUN: %target-codesign %t/main
+
+// RUN: %target-swift-frontend(mock-sdk: -sdk %S/../Inputs/clang-importer-sdk) -typecheck -swift-version 5 -parse-as-library -clang-header-expose-decls=all-public -emit-clang-header-path %t/MacroUser.h -load-plugin-library %t/%target-library-name(MacroDefinition) %s -module-name MacroUser
+// RUN: %FileCheck --check-prefix CHECK %s < %t/MacroUser.h
+
+@attached(member, names: prefixed(member_))
+public macro MemberFunc() = #externalMacro(module: "MacroDefinition", type: "MemberFuncMacro")
+
+@attached(peer, names: prefixed(peer_))
+public macro PeerFunc() = #externalMacro(module: "MacroDefinition", type: "PeerFuncMacro")
+
+@freestanding(declaration, names: named(cxxFreestanding))
+public macro CxxFreestandingFunc() = #externalMacro(module: "MacroDefinition", type: "CxxFreestandingFuncMacro")
+
+@freestanding(declaration, names: named(MacroExpandedStruct))
+public macro CxxFreestandingStruct() = #externalMacro(module: "MacroDefinition", type: "CxxFreestandingStructMacro")
+
+// ---
+
+// CHECK:     class SWIFT_SYMBOL("s:9MacroUser0A14ExpandedStructV") MacroExpandedStruct final {
+// CHECK-DAG: SWIFT_INLINE_THUNK void member() const SWIFT_SYMBOL("s:9MacroUser0A14ExpandedStructV6memberyyF");
+#CxxFreestandingStruct
+
+// CHECK: class SWIFT_SYMBOL("s:9MacroUser10SomeStructV") SomeStruct final {
+@MemberFunc
+public struct SomeStruct {
+  private var someProperty: Int = 0
+
+  @PeerFunc
+  public func someMethod() {}
+
+  #CxxFreestandingFunc
+
+  // CHECK-DAG: SWIFT_INLINE_THUNK void peer_someMethod() const SWIFT_SYMBOL("s:9MacroUser10SomeStructV15peer_someMethodyyF");
+  // CHECK-DAG: SWIFT_INLINE_THUNK void someMethod() const SWIFT_SYMBOL("s:9MacroUser10SomeStructV10someMethodyyF");
+  // CHECK-DAG: SWIFT_INLINE_THUNK void cxxFreestanding() const SWIFT_SYMBOL("s:9MacroUser10SomeStructV15cxxFreestandingyyF");
+  // CHECK-DAG: SWIFT_INLINE_THUNK void member_SomeStruct() const SWIFT_SYMBOL("s:9MacroUser10SomeStructV07member_cD0yyF");
+}
diff --git a/test/Macros/macro_generated_header_objc.swift b/test/Macros/macro_generated_header_objc.swift
