[ClangImporter] don't emit empty .method()

hnrklssn · hnrklssn · commit 14c79d292c6c · 2025-10-24T18:56:45.000-07:00
This updates SwiftifyImportProtocolPrinter such that it no longer emits
anything for methods without bounds or lifetime info. Previously we
would not attach the macro if no methods in the protocol contained
bounds or lifetime info. However if one of the methods did, we would
still emit `.method(signature: "func foo()", paramInfo: [])` for the
methods without bounds of lifetime info. This would result in overloads
being generated, like so:
```
@_alwaysEmitIntoClient @_disfavoredOverload public
func foo() {
  return unsafe foo()
}
```

As part of this change, SwiftifyImportPrinter is now an abstract parent
type for SwiftifyImportProtocolPrinter, and the new
SwiftifyImportFunctionPrinter. Instead of SwiftifyImportProtocolPrinter
inheriting the function printing, `printMethod` instead creates a new
SwiftifyImportFunctionPrinter each time, with a new output string. If it
output anything interesting the result is forwarded, otherwise it is
discarded.
diff --git a/lib/ClangImporter/SwiftifyDecl.cpp b/lib/ClangImporter/SwiftifyDecl.cpp
@@ -42,74 +42,25 @@ ValueDecl *getKnownSingleDecl(ASTContext &SwiftContext, StringRef DeclName) {
   return decls[0];
 }
 
-class SwiftifyInfoPrinter {
-public:
+struct SwiftifyInfoPrinter {
   static const ssize_t SELF_PARAM_INDEX = -2;
   static const ssize_t RETURN_VALUE_INDEX = -1;
   clang::ASTContext &ctx;
   ASTContext &SwiftContext;
-  llvm::raw_ostream &out;
+  llvm::raw_svector_ostream &out;
   MacroDecl &SwiftifyImportDecl;
   bool firstParam = true;
-  llvm::StringMap<std::string> typeMapping;
+  llvm::StringMap<std::string> &typeMapping;
 
+protected:
   SwiftifyInfoPrinter(clang::ASTContext &ctx, ASTContext &SwiftContext,
-                      llvm::raw_ostream &out, MacroDecl &SwiftifyImportDecl)
+                      llvm::raw_svector_ostream &out,
+                      MacroDecl &SwiftifyImportDecl,
+                      llvm::StringMap<std::string> &typeMapping)
       : ctx(ctx), SwiftContext(SwiftContext), out(out),
-        SwiftifyImportDecl(SwiftifyImportDecl) {
-    out << "(";
-  }
-  ~SwiftifyInfoPrinter() { out << ")"; }
-
-  void printCountedBy(const clang::CountAttributedType *CAT,
-                      ssize_t pointerIndex) {
-    printSeparator();
-    clang::Expr *countExpr = CAT->getCountExpr();
-    bool isSizedBy = CAT->isCountInBytes();
-    out << ".";
-    if (isSizedBy)
-      out << "sizedBy";
-    else
-      out << "countedBy";
-    out << "(pointer: ";
-    printParamOrReturn(pointerIndex);
-    out << ", ";
-    if (isSizedBy)
-      out << "size";
-    else
-      out << "count";
-    out << ": \"";
-    countExpr->printPretty(
-        out, {}, {ctx.getLangOpts()}); // TODO: map clang::Expr to Swift Expr
-    out << "\")";
-  }
-
-  void printNonEscaping(int idx) {
-    printSeparator();
-    out << ".nonescaping(pointer: ";
-    printParamOrReturn(idx);
-    out << ")";
-  }
-
-  void printLifetimeboundReturn(int idx, bool borrow) {
-    printSeparator();
-    out << ".lifetimeDependence(dependsOn: ";
-    printParamOrReturn(idx);
-    out << ", pointer: .return, type: ";
-    out << (borrow ? ".borrow" : ".copy");
-    out << ")";
-  }
-
-  bool registerStdSpanTypeMapping(Type swiftType, const clang::QualType clangType) {
-    const auto *decl = clangType->getAsTagDecl();
-    if (decl && decl->isInStdNamespace() && decl->getName() == "span") {
-      typeMapping.insert(std::make_pair(
-          swiftType->getString(), swiftType->getDesugaredType()->getString()));
-      return true;
-    }
-    return false;
-  }
+        SwiftifyImportDecl(SwiftifyImportDecl), typeMapping(typeMapping) {}
 
+public:
   void printTypeMapping() {
     printSeparator();
     out << "typeMappings: [";
@@ -130,7 +81,6 @@ class SwiftifyInfoPrinter {
     out << "spanAvailability: ";
     printAvailabilityOfType("Span");
   }
-
 private:
   bool hasMacroParameter(StringRef ParamName) {
     for (auto *Param : *SwiftifyImportDecl.parameterList)
@@ -160,6 +110,63 @@ class SwiftifyInfoPrinter {
       firstParam = false;
     }
   }
+};
+
+struct SwiftifyInfoFunctionPrinter : public SwiftifyInfoPrinter {
+  SwiftifyInfoFunctionPrinter(clang::ASTContext &ctx, ASTContext &SwiftContext,
+                      llvm::raw_svector_ostream &out,
+                      MacroDecl &SwiftifyImportDecl,
+                      llvm::StringMap<std::string> &typeMapping)
+      : SwiftifyInfoPrinter(ctx, SwiftContext, out, SwiftifyImportDecl, typeMapping) {}
+
+  void printCountedBy(const clang::CountAttributedType *CAT,
+                      ssize_t pointerIndex) {
+    printSeparator();
+    clang::Expr *countExpr = CAT->getCountExpr();
+    bool isSizedBy = CAT->isCountInBytes();
+    out << ".";
+    if (isSizedBy)
+      out << "sizedBy";
+    else
+      out << "countedBy";
+    out << "(pointer: ";
+    printParamOrReturn(pointerIndex);
+    out << ", ";
+    if (isSizedBy)
+      out << "size";
+    else
+      out << "count";
+    out << ": \"";
+    countExpr->printPretty(
+        out, {}, {ctx.getLangOpts()}); // TODO: map clang::Expr to Swift Expr
+    out << "\")";
+  }
+
+  void printNonEscaping(int idx) {
+    printSeparator();
+    out << ".nonescaping(pointer: ";
+    printParamOrReturn(idx);
+    out << ")";
+  }
+
+  void printLifetimeboundReturn(int idx, bool borrow) {
+    printSeparator();
+    out << ".lifetimeDependence(dependsOn: ";
+    printParamOrReturn(idx);
+    out << ", pointer: .return, type: ";
+    out << (borrow ? ".borrow" : ".copy");
+    out << ")";
+  }
+
+  bool registerStdSpanTypeMapping(Type swiftType, const clang::QualType clangType) {
+    const auto *decl = clangType->getAsTagDecl();
+    if (decl && decl->isInStdNamespace() && decl->getName() == "span") {
+      typeMapping.insert(std::make_pair(
+          swiftType->getString(), swiftType->getDesugaredType()->getString()));
+      return true;
+    }
+    return false;
+  }
 
 private:
   void printParamOrReturn(ssize_t pointerIndex) {
@@ -315,7 +322,7 @@ static size_t getNumParams(const clang::FunctionDecl* D) {
 
 template<typename T>
 static bool swiftifyImpl(ClangImporter::Implementation &Self,
-                         SwiftifyInfoPrinter &printer,
+                         SwiftifyInfoFunctionPrinter &printer,
                          const AbstractFunctionDecl *MappedDecl,
                          const T *ClangDecl) {
   DLOG("Checking " << *ClangDecl << " for bounds and lifetime info\n");
@@ -451,28 +458,29 @@ class SwiftifyProtocolInfoPrinter : public SwiftifyInfoPrinter {
 public:
   SwiftifyProtocolInfoPrinter(ClangImporter::Implementation &ImporterImpl,
                               clang::ASTContext &ctx, ASTContext &SwiftContext,
-                              llvm::raw_ostream &out,
-                              MacroDecl &SwiftifyImportDecl)
-      : SwiftifyInfoPrinter(ctx, SwiftContext, out, SwiftifyImportDecl),
+                              llvm::raw_svector_ostream &out,
+                              MacroDecl &SwiftifyImportDecl,
+                              llvm::StringMap<std::string> &typeMapping)
+      : SwiftifyInfoPrinter(ctx, SwiftContext, out, SwiftifyImportDecl, typeMapping),
         ImporterImpl(ImporterImpl) {}
 
   bool printMethod(const FuncDecl *Method) {
-    // don't emit .method() if we know it's going to be empty
     auto ClangDecl = dyn_cast_or_null<clang::ObjCMethodDecl>(Method->getClangDecl());
     if (!ClangDecl)
       return false;
 
-    printSeparator();
-    out << ".method(signature: \"";
-    printMethodSignature(Method);
-    out << "\", paramInfo: [";
-    // reset firstParam inside paramInfo array. At this point firstParam will
-    // always be false, so no need to save the current value.
-    ASSERT(!firstParam);
-    firstParam = true;
-    bool hadAttributes = swiftifyImpl(ImporterImpl, *this, Method, ClangDecl);
-    firstParam = false;
-    out << "])";
+    llvm::SmallString<128> paramInfoString;
+    llvm::raw_svector_ostream tmpOut(paramInfoString);
+
+    SwiftifyInfoFunctionPrinter methodPrinter(ctx, SwiftContext, tmpOut,
+                                              SwiftifyImportDecl, typeMapping);
+    bool hadAttributes = swiftifyImpl(ImporterImpl, methodPrinter, Method, ClangDecl);
+    if (hadAttributes) {
+      printSeparator();
+      out << ".method(signature: \"";
+      printMethodSignature(Method);
+      out << "\", paramInfo: [" << paramInfoString << "])";
+    }
     return hadAttributes;
   }
 
@@ -507,13 +515,16 @@ void ClangImporter::Implementation::swiftify(AbstractFunctionDecl *MappedDecl) {
   llvm::SmallString<128> MacroString;
   {
     llvm::raw_svector_ostream out(MacroString);
-    out << "@_SwiftifyImport";
+    out << "@_SwiftifyImport(";
 
-    SwiftifyInfoPrinter printer(getClangASTContext(), SwiftContext, out, *SwiftifyImportDecl);
+    llvm::StringMap<std::string> typeMapping;
+    SwiftifyInfoFunctionPrinter printer(getClangASTContext(), SwiftContext, out,
+                                        *SwiftifyImportDecl, typeMapping);
     if (!swiftifyImpl(*this, printer, MappedDecl, ClangDecl))
       return;
     printer.printAvailability();
     printer.printTypeMapping();
+    out << ")";
   }
 
   DLOG("Attaching safe interop macro: " << MacroString << "\n");
@@ -553,11 +564,13 @@ void ClangImporter::Implementation::swiftifyProtocol(
   llvm::SmallString<128> MacroString;
   {
     llvm::raw_svector_ostream out(MacroString);
-    out << "@_SwiftifyImportProtocol";
+    out << "@_SwiftifyImportProtocol(";
 
     bool hasBoundsAttributes = false;
+    llvm::StringMap<std::string> typeMapping;
     SwiftifyProtocolInfoPrinter printer(*this, getClangASTContext(),
-                                        SwiftContext, out, *SwiftifyImportDecl);
+                                        SwiftContext, out, *SwiftifyImportDecl,
+                                        typeMapping);
     for (Decl *SwiftMember :
          cast<IterableDeclContext>(MappedDecl)->getAllMembers()) {
       FuncDecl *SwiftDecl = dyn_cast<FuncDecl>(SwiftMember);
@@ -570,6 +583,7 @@ void ClangImporter::Implementation::swiftifyProtocol(
       return;
     printer.printAvailability();
     printer.printTypeMapping();
+    out << ")";
   }
 
   DLOG("Attaching safe interop macro: " << MacroString << "\n");
diff --git a/test/Interop/ObjC/swiftify-import/negative.swift b/test/Interop/ObjC/swiftify-import/negative.swift
@@ -0,0 +1,97 @@
+// REQUIRES: swift_feature_SafeInteropWrappers
+
+// RUN: %empty-directory(%t)
+// RUN: split-file %s %t
+
+// RUN: %target-swift-frontend -emit-module -plugin-path %swift-plugin-dir -I %t -enable-experimental-feature SafeInteropWrappers %t/test.swift -verify -Xcc -Wno-nullability-completeness
+// RUN: env SWIFT_BACKTRACE="" %target-swift-frontend -emit-module -plugin-path %swift-plugin-dir -I %t -enable-experimental-feature SafeInteropWrappers %t/test.swift -dump-macro-expansions 2> %t/expansions.out
+// RUN: %diff %t/expansions.out %t/expansions.expected
+
+//--- test.h
+#pragma once
+
+#define __counted_by(x) __attribute__((__counted_by__(x)))
+
+@protocol NoBounds
+ - (void) ignore;
+@end
+
+@protocol NoYesNo
+ - (void) ignore;
+ - (void) simple:(int)len :(int * __counted_by(len))p;
+ - (void) ignore2;
+@end
+
+@protocol YesNo
+ - (void) simple:(int)len :(int * __counted_by(len))p;
+ - (void) ignore;
+@end
+
+@protocol YesNoYes
+ - (void) simple:(int)len :(int * __counted_by(len))p;
+ - (void) ignore;
+ - (void) simple2:(int)len :(int * __counted_by(len))p;
+@end
+
+//--- expansions.expected
+@__swiftmacro_So05NoYesA023_SwiftifyImportProtocolfMe_.swift
+------------------------------
+extension NoYesNo {
+    /// This is an auto-generated wrapper for safer interop
+@_alwaysEmitIntoClient @_disfavoredOverload public
+        func simple(_ p: UnsafeMutableBufferPointer<Int32>) {
+        let len = Int32(exactly: p.count)!
+        return unsafe simple(len, p.baseAddress!)
+    }
+}
+------------------------------
+@__swiftmacro_So5YesNo23_SwiftifyImportProtocolfMe_.swift
+------------------------------
+extension YesNo {
+    /// This is an auto-generated wrapper for safer interop
+@_alwaysEmitIntoClient @_disfavoredOverload public
+        func simple(_ p: UnsafeMutableBufferPointer<Int32>) {
+        let len = Int32(exactly: p.count)!
+        return unsafe simple(len, p.baseAddress!)
+    }
+}
+------------------------------
+@__swiftmacro_So05YesNoA023_SwiftifyImportProtocolfMe_.swift
+------------------------------
+extension YesNoYes {
+    /// This is an auto-generated wrapper for safer interop
+@_alwaysEmitIntoClient @_disfavoredOverload public
+        func simple(_ p: UnsafeMutableBufferPointer<Int32>) {
+        let len = Int32(exactly: p.count)!
+        return unsafe simple(len, p.baseAddress!)
+    }
+    /// This is an auto-generated wrapper for safer interop
+@_alwaysEmitIntoClient @_disfavoredOverload public
+        func simple2(_ p: UnsafeMutableBufferPointer<Int32>) {
+        let len = Int32(exactly: p.count)!
+        return unsafe simple2(len, p.baseAddress!)
+    }
+}
+------------------------------
+//--- test.swift
+import TestClang
+
+@inlinable
+public func call(p: UnsafeMutableBufferPointer<CInt>, x: NoBounds, y: NoYesNo, z: YesNo, å: YesNoYes) {
+  x.ignore()
+  y.ignore()
+  y.simple(p)
+  z.ignore()
+  z.simple(p)
+  å.ignore()
+  å.simple(p)
+  å.simple2(p)
+}
+
+//--- module.modulemap
+module TestClang {
+    header "test.h"
+    export *
+}
+
+
