[SourceKit] Don’t use C++ to Swift interop to run NameMatcher

Author: ahoppen

Diff for: cmake/modules/AddPureSwift.cmake

@@ -106,10 +106,6 @@ endfunction()
 # EMIT_MODULE
 #   Emit '.swiftmodule' to
 #
-# GENERATE_CXX_BRIDGING_HEADER
-#   Emit a briding header with all public declarations for this library that can
-#   be imported from C++ to include/ASTGen/<target-name>-Swift.h
-#
 # DEPENDENCIES
 #   Target names to pass target_link_library
 #
@@ -129,8 +125,7 @@ function(add_pure_swift_host_library name)
   set(options
         SHARED
         STATIC
-        EMIT_MODULE
-        GENERATE_CXX_BRIDGING_HEADER)
+        EMIT_MODULE)
   set(single_parameter_options)
   set(multiple_parameter_options
         DEPENDENCIES
@@ -236,34 +231,6 @@ function(add_pure_swift_host_library name)
         >)
   endif()
 
-  if(APSHL_GENERATE_CXX_BRIDGING_HEADER)
-    set(bridging_header_dir "${CMAKE_BINARY_DIR}/include/swift/ASTGen")
-    set(bridging_header_path "${bridging_header_dir}/${name}-Swift.h")
-	  file(MAKE_DIRECTORY ${bridging_header_dir})
-    target_compile_options(${name} PRIVATE 
-      "SHELL: -Xfrontend -emit-clang-header-path -Xfrontend ${bridging_header_path}"
-    )
-    if (CMAKE_Swift_COMPILER_VERSION VERSION_LESS 5.9)
-      # The 5.8 compiler does not expose all public declarations in the bridging header by default.
-      target_compile_options(${name} PRIVATE 
-        "SHELL: -Xfrontend -clang-header-expose-decls=all-public"
-      )
-      # Explicitly link swiftCxx because it's not automatically liked by a Swift 5.8 compiler
-      target_link_libraries(${name} PRIVATE -lswiftCxx)
-    endif()
-
-    add_custom_command(
-      TARGET ${name}
-      POST_BUILD
-      COMMAND ""
-      BYPRODUCTS ${bridging_header_path}
-      DEPENDS ${name}
-      COMMENT "Empty command that targets can depend on so that the C++ bridging header gets generate before they are built"
-    )
-
-    add_custom_target("${name}-cxx-briding-header" DEPENDS ${bridging_header_path})
-  endif()
-
   if(LLVM_USE_LINKER)
     target_link_options(${name} PRIVATE
       "-use-ld=${LLVM_USE_LINKER}"

Diff for: include/swift/CxxToSwift/IDEUtilsBridging.h

@@ -90,6 +90,41 @@ typedef std::vector<ResolvedLoc> ResolvedLocVector;
 SWIFT_NAME("ResolvedLocVector.empty()")
 ResolvedLocVector ResolvedLocVector_createEmpty();
 
+/// A heap-allocated `std::vector<ResoledLoc>` that can be represented by an
+/// opaque pointer value.
+///
+/// This allows us to perform all the memory management for the heap-allocated
+/// vector in C++. This makes it easier to manage because creating and
+/// destorying an object in C++ is consistent with whether elements within the
+/// vector are destroyed as well.
+///
+/// - Note: This should no longer be necessary when we use C++ to Swift interop.
+///   In that case `swift_SwiftIDEUtilsBridging_runNameMatcher` can return a
+///   `ResolvedLocVector`.
+class BridgedResolvedLocVector {
+  const std::vector<ResolvedLoc> *vector;
+
+public:
+  /// Create heap-allocaed vector with the same elements as `vector`.
+  BridgedResolvedLocVector(const std::vector<ResolvedLoc> &vector);
+
+  /// Create a `BridgedResolvedLocVector` from an opaque value obtained from
+  /// `getOpaqueValue`.
+  BridgedResolvedLocVector(const void *opaqueValue);
+
+  /// Get the underlying vector.
+  const std::vector<ResolvedLoc> &unbridged();
+
+  /// Delete the heap-allocated memory owned by this object. Accessing
+  /// `unbridged` is illegal after calling `destroy`.
+  void destroy();
+
+  /// Get an opaque pointer value that describes this 
+  /// `BridgedResolvedLocVector`. This opaque value can be returned by a
+  /// `@_cdecl` function in Swift.
+  const void *getOpaqueValue() const;
+};
+
 typedef std::vector<BridgedSourceLoc> SourceLocVector;
 
 /// Needed so that we can manually conform `SourceLocVectorIterator` to
@@ -103,4 +138,24 @@ inline bool SourceLocVectorIterator_equal(const SourceLocVectorIterator &lhs,
   return lhs == rhs;
 }
 
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/// Entry point to run the NameMatcher written in swift-syntax.
+/// 
+/// - Parameters:
+///   - sourceFilePtr: A pointer to an `ExportedSourceFile`, used to access the
+///     syntax tree
+///   - locations: Pointer to a buffer of `BridgedSourceLoc` that should be
+///     resolved by the name matcher.
+///   - locationsCount: Number of elements in `locations`.
+/// - Returns: The opaque value of a `BridgedResolvedLocVector`.
+void *swift_SwiftIDEUtilsBridging_runNameMatcher(const void *sourceFilePtr,
+                                                 BridgedSourceLoc *locations,
+                                                 size_t locationsCount);
+#ifdef __cplusplus
+}
+#endif
+
 #endif

Diff for: include/swift/IDE/IDEBridging.h

@@ -63,7 +63,7 @@ add_pure_swift_host_library(swiftASTGen STATIC
     ${ASTGen_Swift_dependencies}
 )
 
-add_pure_swift_host_library(swiftIDEUtilsBridging GENERATE_CXX_BRIDGING_HEADER
+add_pure_swift_host_library(swiftIDEUtilsBridging
   Sources/SwiftIDEUtilsBridging/NameMatcherBridging.swift
 
   DEPENDENCIES

Diff for: lib/ASTGen/CMakeLists.txt

@@ -109,12 +109,15 @@ fileprivate extension IDEBridging.ResolvedLocContext {
 
 // MARK: - Run NameMatcher
 
+@_cdecl("swift_SwiftIDEUtilsBridging_runNameMatcher")
 public func runNameMatcher(
   sourceFilePtr: UnsafeRawPointer,
-  locations: SourceLocVector
-) -> ResolvedLocVector {
+  locations: UnsafePointer<BridgedSourceLoc>,
+  locationsCount: UInt
+) -> UnsafeRawPointer {
   let sourceFile = sourceFilePtr.bindMemory(to: ExportedSourceFile.self, capacity: 1).pointee
-  let positions: [AbsolutePosition] = locations.compactMap { sourceFile.position(of: $0) }
+  let locations = UnsafeBufferPointer(start: locations, count: Int(locationsCount))
+  let positions: [AbsolutePosition] =  locations.compactMap { sourceFile.position(of: $0) }
   let resolvedLocs = NameMatcher.resolve(baseNamePositions: positions, in: sourceFile.syntax)
-  return ResolvedLocVector(from: resolvedLocs, in: sourceFile)
+  return BridgedResolvedLocVector(ResolvedLocVector(from: resolvedLocs, in: sourceFile)).getOpaqueValue()
 }

Diff for: lib/ASTGen/Sources/SwiftIDEUtilsBridging/NameMatcherBridging.swift

@@ -11,6 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "swift/IDE/IDEBridging.h"
+#include "llvm/Support/raw_ostream.h"
 #include <climits>
 
 ResolvedLoc::ResolvedLoc(BridgedCharSourceRange range,
@@ -37,3 +38,20 @@ ResolvedLoc::ResolvedLoc() {}
 ResolvedLocVector ResolvedLocVector_createEmpty() {
   return ResolvedLocVector();
 }
+
+BridgedResolvedLocVector::BridgedResolvedLocVector(
+    const std::vector<ResolvedLoc> &vector)
+    : vector(new std::vector<ResolvedLoc>(vector)) {}
+
+BridgedResolvedLocVector::BridgedResolvedLocVector(const void *opaqueValue)
+    : vector(static_cast<const std::vector<ResolvedLoc> *>(opaqueValue)) {}
+
+const std::vector<ResolvedLoc> &BridgedResolvedLocVector::unbridged() {
+  return *vector;
+}
+
+void BridgedResolvedLocVector::destroy() { delete vector; }
+
+const void *BridgedResolvedLocVector::getOpaqueValue() const {
+  return static_cast<const void *>(vector);
+}

Diff for: lib/IDE/IDEBridging.cpp

@@ -55,7 +55,5 @@ target_link_libraries(swiftRefactoring PRIVATE
   swiftParse
   swiftSema)
 
-add_dependencies(swiftRefactoring "swiftIDEUtilsBridging-cxx-briding-header")
-
 set_swift_llvm_is_available(swiftRefactoring)
 

Diff for: lib/Refactoring/CMakeLists.txt

@@ -13,7 +13,6 @@
 #include "swift/AST/ASTContext.h"
 #include "swift/AST/DiagnosticsRefactoring.h"
 #include "swift/AST/SourceFile.h"
-#include "swift/CxxToSwift/IDEUtilsBridging.h"
 #include "swift/Frontend/PrintingDiagnosticConsumer.h"
 #include "swift/IDE/IDEBridging.h"
 #include "swift/Parse/Lexer.h"
@@ -78,19 +77,24 @@ swift::ide::resolveRenameLocations(ArrayRef<RenameLoc> RenameLocs,
     BridgedUnresolvedLocs.push_back(BridgedSourceLoc(Loc));
   }
 
-  std::vector<ResolvedLoc> resolvedLocsInSourceOrder =
-      swiftIDEUtilsBridging::runNameMatcher(SF.getExportedSourceFile(),
-                                            BridgedUnresolvedLocs);
+  BridgedResolvedLocVector bridgedResolvedLocs =
+      swift_SwiftIDEUtilsBridging_runNameMatcher(SF.getExportedSourceFile(),
+                                                 BridgedUnresolvedLocs.data(),
+                                                 BridgedUnresolvedLocs.size());
+  SWIFT_DEFER { bridgedResolvedLocs.destroy(); };
+  const std::vector<ResolvedLoc> &resolvedLocsInSourceOrder =
+      bridgedResolvedLocs.unbridged();
 
   // Callers expect the resolved locs in the same order as the unresolved locs.
   // Sort them.
   // FIXME: (NameMatcher) Can we change the callers to not rely on this?
   std::vector<ResolvedLoc> resolvedLocsInRequestedOrder;
   for (SourceLoc unresolvedLoc : UnresolvedLocs) {
-    auto found = llvm::find_if(
-        resolvedLocsInSourceOrder, [unresolvedLoc](ResolvedLoc &resolved) {
-          return resolved.range.getStart() == unresolvedLoc;
-        });
+    auto found =
+        llvm::find_if(resolvedLocsInSourceOrder,
+                      [unresolvedLoc](const ResolvedLoc &resolved) {
+                        return resolved.range.getStart() == unresolvedLoc;
+                      });
     if (found == resolvedLocsInSourceOrder.end()) {
       resolvedLocsInRequestedOrder.push_back(
           ResolvedLoc(CharSourceRange(),