[AST] Merge BridgedDiagnosticEngine + BridgedDiagEngine

hamishknight · hamishknight · commit fce1cb54d54b · 2023-10-30T23:49:55.000Z
Introduce a macro that can stamp out wrapper
classes for underlying C++ pointers, and use
it to define BridgedDiagnosticEngine in
ASTBridging. Then, migrate users of
BridgedDiagEngine onto it.
diff --git a/SwiftCompilerSources/Sources/AST/DiagnosticEngine.swift b/SwiftCompilerSources/Sources/AST/DiagnosticEngine.swift
@@ -52,16 +52,16 @@ public struct DiagnosticFixIt {
 }
 
 public struct DiagnosticEngine {
-  private let bridged: BridgedDiagEngine
+  private let bridged: BridgedDiagnosticEngine
 
-  public init(bridged: BridgedDiagEngine) {
+  public init(bridged: BridgedDiagnosticEngine) {
     self.bridged = bridged
   }
-  public init?(bridged: BridgedOptionalDiagnosticEngine) {
-    guard let object = bridged.object else {
+  public init?(bridged: BridgedNullableDiagnosticEngine) {
+    guard let raw = bridged.raw else {
       return nil
     }
-    self.bridged = BridgedDiagEngine(object: object)
+    self.bridged = BridgedDiagnosticEngine(raw: raw)
   }
 
   public func diagnose(_ position: SourceLoc?,
diff --git a/SwiftCompilerSources/Sources/Parse/Regex.swift b/SwiftCompilerSources/Sources/Parse/Regex.swift
@@ -46,7 +46,7 @@ private func _RegexLiteralLexingFn(
   _ curPtrPtr: UnsafeMutablePointer<UnsafePointer<CChar>>,
   _ bufferEndPtr: UnsafePointer<CChar>,
   _ mustBeRegex: CBool,
-  _ bridgedDiagnosticEngine: BridgedOptionalDiagnosticEngine
+  _ bridgedDiagnosticEngine: BridgedNullableDiagnosticEngine
 ) -> /*CompletelyErroneous*/ CBool {
   let inputPtr = curPtrPtr.pointee
 
@@ -92,7 +92,7 @@ public func _RegexLiteralParsingFn(
   _ captureStructureOut: UnsafeMutableRawPointer,
   _ captureStructureSize: CUnsignedInt,
   _ bridgedDiagnosticBaseLoc: BridgedSourceLoc,
-  _ bridgedDiagnosticEngine: BridgedDiagEngine
+  _ bridgedDiagnosticEngine: BridgedDiagnosticEngine
 ) -> Bool {
   let str = String(cString: inputPtr)
   let captureBuffer = UnsafeMutableRawBufferPointer(
diff --git a/include/swift/AST/ASTBridging.h b/include/swift/AST/ASTBridging.h
@@ -27,6 +27,7 @@ SWIFT_BEGIN_NULLABILITY_ANNOTATIONS
 
 namespace swift {
   class DiagnosticArgument;
+  class DiagnosticEngine;
 }
 
 //===----------------------------------------------------------------------===//
@@ -40,14 +41,8 @@ typedef enum ENUM_EXTENSIBILITY_ATTR(open) BridgedDiagID : uint32_t {
 #include "swift/AST/DiagnosticsAll.def"
 } BridgedDiagID;
 
-// The name must not collide with BridgedDiagnosticEngine in CASTBridging.h.
-struct BridgedDiagEngine {
-  void * _Nonnull object;
-};
-
-struct BridgedOptionalDiagnosticEngine {
-  void *_Nullable object;
-};
+BRIDGING_WRAPPER_NONNULL(DiagnosticEngine)
+BRIDGING_WRAPPER_NULLABLE(DiagnosticEngine)
 
 class BridgedDiagnosticArgument {
   int64_t storage[3];
@@ -83,12 +78,13 @@ class BridgedDiagnosticFixIt {
 };
 
 // FIXME: Can we bridge InFlightDiagnostic?
-void DiagnosticEngine_diagnose(BridgedDiagEngine, BridgedSourceLoc loc,
+void DiagnosticEngine_diagnose(BridgedDiagnosticEngine, BridgedSourceLoc loc,
                                BridgedDiagID diagID, BridgedArrayRef arguments,
-                               BridgedSourceLoc highlightStart, uint32_t hightlightLength,
+                               BridgedSourceLoc highlightStart,
+                               uint32_t hightlightLength,
                                BridgedArrayRef fixIts);
 
-bool DiagnosticEngine_hadAnyError(BridgedDiagEngine);
+bool DiagnosticEngine_hadAnyError(BridgedDiagnosticEngine);
 
 SWIFT_END_NULLABILITY_ANNOTATIONS
 
diff --git a/include/swift/AST/BridgingUtils.h b/include/swift/AST/BridgingUtils.h
@@ -18,14 +18,6 @@
 
 namespace swift {
 
-inline BridgedDiagEngine getBridgedDiagnosticEngine(DiagnosticEngine *D) {
-  return {(void *)D};
-}
-inline BridgedOptionalDiagnosticEngine
-getBridgedOptionalDiagnosticEngine(DiagnosticEngine *D) {
-  return {(void *)D};
-}
-
 } // namespace swift
 
 #endif
diff --git a/include/swift/AST/CASTBridging.h b/include/swift/AST/CASTBridging.h
@@ -13,7 +13,7 @@
 #ifndef SWIFT_C_AST_ASTBRIDGING_H
 #define SWIFT_C_AST_ASTBRIDGING_H
 
-#include "swift/Basic/BasicBridging.h"
+#include "swift/AST/ASTBridging.h"
 #include "swift/Basic/CBasicBridging.h"
 #include "swift/Basic/Compiler.h"
 #include "swift/Basic/Nullability.h"
@@ -170,10 +170,6 @@ typedef struct BridgedDiagnostic {
   void *raw;
 } BridgedDiagnostic;
 
-typedef struct BridgedDiagnosticEngine {
-  void *raw;
-} BridgedDiagnosticEngine;
-
 typedef enum ENUM_EXTENSIBILITY_ATTR(open) BridgedMacroDefinitionKind : size_t {
   /// An expanded macro.
   BridgedExpandedMacro = 0,
diff --git a/include/swift/Basic/BasicBridging.h b/include/swift/Basic/BasicBridging.h
@@ -43,6 +43,13 @@
 #define SWIFT_NAME(NAME)
 #endif
 
+#if __has_attribute(availability)
+#define SWIFT_UNAVAILABLE(msg) \
+  __attribute__((availability(swift, unavailable, message=msg)))
+#else
+#define SWIFT_UNAVAILABLE(msg)
+#endif
+
 #ifdef PURE_BRIDGING_MODE
 // In PURE_BRIDGING_MODE, briding functions are not inlined
 #define BRIDGED_INLINE
@@ -55,6 +62,43 @@ SWIFT_BEGIN_NULLABILITY_ANNOTATIONS
 typedef intptr_t SwiftInt;
 typedef uintptr_t SwiftUInt;
 
+// Define a bridging wrapper that wraps an underlying C++ pointer type. When
+// importing into Swift, we expose an initializer and accessor that works with
+// `void *`, which is imported as UnsafeMutableRawPointer. Note we can't rely on
+// Swift importing the underlying C++ pointer as an OpaquePointer since that is
+// liable to change with PURE_BRIDGING_MODE, since that changes what we include,
+// and Swift could import the underlying pointee type instead. We need to be
+// careful that the interface we expose remains consistent regardless of
+// PURE_BRIDGING_MODE.
+#define BRIDGING_WRAPPER_IMPL(Node, Name, Nullability)                         \
+  class Bridged##Name {                                                        \
+    swift::Node * Nullability Ptr;                                             \
+                                                                               \
+  public:                                                                      \
+    SWIFT_UNAVAILABLE("Use init(raw:) instead")                                \
+    Bridged##Name(swift::Node * Nullability ptr) : Ptr(ptr) {}                 \
+                                                                               \
+    SWIFT_UNAVAILABLE("Use '.raw' instead")                                    \
+    swift::Node * Nullability get() const { return Ptr; }                      \
+  };                                                                           \
+                                                                               \
+  SWIFT_NAME("getter:Bridged" #Name ".raw(self:)")                             \
+  inline void * Nullability Bridged##Name##_getRaw(Bridged##Name bridged) {    \
+    return bridged.get();                                                      \
+  }                                                                            \
+                                                                               \
+  SWIFT_NAME("Bridged" #Name ".init(raw:)")                                    \
+  inline Bridged##Name Bridged##Name##_fromRaw(void * Nullability ptr) {       \
+    return static_cast<swift::Node *>(ptr);                                    \
+  }
+
+// Bridging wrapper macros for convenience.
+#define BRIDGING_WRAPPER_NONNULL(Name) \
+  BRIDGING_WRAPPER_IMPL(Name, Name, _Nonnull)
+
+#define BRIDGING_WRAPPER_NULLABLE(Name) \
+  BRIDGING_WRAPPER_IMPL(Name, Nullable##Name, _Nullable)
+
 struct BridgedOStream {
   void * _Nonnull streamAddr;
 };
diff --git a/include/swift/Parse/RegexParserBridging.h b/include/swift/Parse/RegexParserBridging.h
@@ -28,7 +28,7 @@
 ///              past.
 /// - MustBeRegex: whether an error during lexing should be considered a regex
 ///                literal, or some thing else.
-/// - BridgedOptionalDiagnosticEngine: RegexLiteralLexingFn should diagnose the
+/// - BridgedNullableDiagnosticEngine: RegexLiteralLexingFn should diagnose the
 ///                                    token using this engine.
 ///
 /// Returns: A bool indicating whether lexing was completely erroneous, and
@@ -37,7 +37,7 @@
 typedef bool (*RegexLiteralLexingFn)(
     /*CurPtrPtr*/ const char *_Nonnull *_Nonnull,
     /*BufferEnd*/ const char *_Nonnull,
-    /*MustBeRegex*/ bool, BridgedOptionalDiagnosticEngine);
+    /*MustBeRegex*/ bool, BridgedNullableDiagnosticEngine);
 void Parser_registerRegexLiteralLexingFn(RegexLiteralLexingFn _Nullable fn);
 
 /// Parse a regex literal string. Takes the following arguments:
@@ -58,7 +58,7 @@ typedef bool (*RegexLiteralParsingFn)(/*InputPtr*/ const char *_Nonnull,
                                       /*CaptureStructureOut*/ void *_Nonnull,
                                       /*CaptureStructureSize*/ unsigned,
                                       /*DiagnosticBaseLoc*/ BridgedSourceLoc,
-                                      BridgedDiagEngine);
+                                      BridgedDiagnosticEngine);
 void Parser_registerRegexLiteralParsingFn(RegexLiteralParsingFn _Nullable fn);
 
 #endif // REGEX_PARSER_BRIDGING
diff --git a/include/swift/SILOptimizer/OptimizerBridging.h b/include/swift/SILOptimizer/OptimizerBridging.h
@@ -181,7 +181,7 @@ struct BridgedPassContext {
   // AST
 
   SWIFT_IMPORT_UNSAFE BRIDGED_INLINE
-  BridgedDiagEngine getDiagnosticEngine() const;
+  BridgedDiagnosticEngine getDiagnosticEngine() const;
 
   // SIL modifications
 
diff --git a/include/swift/SILOptimizer/OptimizerBridgingImpl.h b/include/swift/SILOptimizer/OptimizerBridgingImpl.h
@@ -175,7 +175,7 @@ BridgedNominalTypeDecl BridgedPassContext::getSwiftArrayDecl() const {
 // AST
 
 SWIFT_IMPORT_UNSAFE BRIDGED_INLINE
-BridgedDiagEngine BridgedPassContext::getDiagnosticEngine() const {
+BridgedDiagnosticEngine BridgedPassContext::getDiagnosticEngine() const {
   swift::SILModule *mod = invocation->getPassManager()->getModule();
   return {&mod->getASTContext().Diags};
 }
diff --git a/lib/AST/ASTBridging.cpp b/lib/AST/ASTBridging.cpp
@@ -17,14 +17,6 @@
 
 using namespace swift;
 
-namespace {
-/// BridgedDiagEngine -> DiagnosticEngine *.
-DiagnosticEngine *getDiagnosticEngine(const BridgedDiagEngine &bridged) {
-  return static_cast<DiagnosticEngine *>(bridged.object);
-}
-
-} // namespace
-
 static_assert(sizeof(BridgedDiagnosticArgument) >= sizeof(DiagnosticArgument),
               "BridgedDiagnosticArgument has wrong size");
 
@@ -45,12 +37,12 @@ BridgedDiagnosticFixIt::BridgedDiagnosticFixIt(BridgedSourceLoc start,
           llvm::ArrayRef<DiagnosticArgument>())) {}
 
 void DiagnosticEngine_diagnose(
-    BridgedDiagEngine bridgedEngine, BridgedSourceLoc loc,
+    BridgedDiagnosticEngine bridgedEngine, BridgedSourceLoc loc,
     BridgedDiagID bridgedDiagID,
     BridgedArrayRef /*BridgedDiagnosticArgument*/ bridgedArguments,
     BridgedSourceLoc highlightStart, uint32_t hightlightLength,
     BridgedArrayRef /*BridgedDiagnosticFixIt*/ bridgedFixIts) {
-  auto *D = getDiagnosticEngine(bridgedEngine);
+  auto *D = bridgedEngine.get();
 
   auto diagID = static_cast<DiagID>(bridgedDiagID);
   SmallVector<DiagnosticArgument, 2> arguments;
@@ -73,7 +65,6 @@ void DiagnosticEngine_diagnose(
   }
 }
 
-bool DiagnosticEngine_hadAnyError(BridgedDiagEngine bridgedEngine) {
-  auto *D = getDiagnosticEngine(bridgedEngine);
-  return D->hadAnyError();
+bool DiagnosticEngine_hadAnyError(BridgedDiagnosticEngine bridgedEngine) {
+  return bridgedEngine.get()->hadAnyError();
 }
diff --git a/lib/AST/CASTBridging.cpp b/lib/AST/CASTBridging.cpp
@@ -143,7 +143,7 @@ static inline BridgedDiagnosticImpl *unbridged(BridgedDiagnostic cDiag) {
 }
 
 static inline DiagnosticEngine &unbridged(BridgedDiagnosticEngine cEngine) {
-  return *static_cast<DiagnosticEngine *>(cEngine.raw);
+  return *cEngine.get();
 }
 
 static inline TypeAttributes *unbridged(BridgedTypeAttributes cAttributes) {
diff --git a/lib/ASTGen/Sources/ASTGen/ASTGen.swift b/lib/ASTGen/Sources/ASTGen/ASTGen.swift
@@ -1,3 +1,4 @@
+import ASTBridging
 import BasicBridging
 import CASTBridging
 import CBasicBridging
diff --git a/lib/ASTGen/Sources/ASTGen/DiagnosticsBridge.swift b/lib/ASTGen/Sources/ASTGen/DiagnosticsBridge.swift
@@ -1,3 +1,4 @@
+import ASTBridging
 import BasicBridging
 import CASTBridging
 import SwiftDiagnostics
diff --git a/lib/ASTGen/Sources/ASTGen/PluginHost.swift b/lib/ASTGen/Sources/ASTGen/PluginHost.swift
@@ -10,6 +10,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+import ASTBridging
 import BasicBridging
 import CASTBridging
 import CBasicBridging
diff --git a/lib/ASTGen/Sources/ASTGen/SourceFile.swift b/lib/ASTGen/Sources/ASTGen/SourceFile.swift
@@ -1,3 +1,4 @@
+import ASTBridging
 import CASTBridging
 import CBasicBridging
 import SwiftDiagnostics
diff --git a/lib/ASTGen/Sources/ASTGen/SourceManager.swift b/lib/ASTGen/Sources/ASTGen/SourceManager.swift
@@ -1,3 +1,4 @@
+import ASTBridging
 import BasicBridging
 import CASTBridging
 import SwiftOperators
diff --git a/lib/Parse/Lexer.cpp b/lib/Parse/Lexer.cpp
@@ -2090,8 +2090,8 @@ const char *Lexer::tryScanRegexLiteral(const char *TokStart, bool MustBeRegex,
   // - CompletelyErroneous will be set if there was an error that cannot be
   //   recovered from.
   auto *Ptr = TokStart;
-  CompletelyErroneous = regexLiteralLexingFn(
-      &Ptr, BufferEnd, MustBeRegex, getBridgedOptionalDiagnosticEngine(Diags));
+  CompletelyErroneous =
+      regexLiteralLexingFn(&Ptr, BufferEnd, MustBeRegex, Diags);
 
   // If we didn't make any lexing progress, this isn't a regex literal and we
   // should fallback to lexing as something else.
diff --git a/lib/Parse/ParseRegex.cpp b/lib/Parse/ParseRegex.cpp
@@ -40,12 +40,12 @@ ParserResult<Expr> Parser::parseExprRegexLiteral() {
   auto capturesBuf = Context.AllocateUninitialized<uint8_t>(
       RegexLiteralExpr::getCaptureStructureSerializationAllocationSize(
           regexText.size()));
-  bool hadError =
-      regexLiteralParsingFn(regexText.str().c_str(), &version,
-                            /*captureStructureOut*/ capturesBuf.data(),
-                            /*captureStructureSize*/ capturesBuf.size(),
-                            /*diagBaseLoc*/ {(const uint8_t *)(Tok.getLoc().getOpaquePointerValue())},
-                            getBridgedDiagnosticEngine(&Diags));
+  bool hadError = regexLiteralParsingFn(
+      regexText.str().c_str(), &version,
+      /*captureStructureOut*/ capturesBuf.data(),
+      /*captureStructureSize*/ capturesBuf.size(),
+      /*diagBaseLoc*/ {(const uint8_t *)(Tok.getLoc().getOpaquePointerValue())},
+      &Diags);
   auto loc = consumeToken();
   SourceMgr.recordRegexLiteralStartLoc(loc);
 

Original file line number	Diff line number	Diff line change
`@@ -52,16 +52,16 @@ public struct DiagnosticFixIt {`
`52`	`52`	`}`
`53`	`53`
`54`	`54`	`public struct DiagnosticEngine {`
`55`		`- private let bridged: BridgedDiagEngine`
	`55`	`+ private let bridged: BridgedDiagnosticEngine`
`56`	`56`
`57`		`- public init(bridged: BridgedDiagEngine) {`
	`57`	`+ public init(bridged: BridgedDiagnosticEngine) {`
`58`	`58`	`self.bridged = bridged`
`59`	`59`	`}`
`60`		`- public init?(bridged: BridgedOptionalDiagnosticEngine) {`
`61`		`- guard let object = bridged.object else {`
	`60`	`+ public init?(bridged: BridgedNullableDiagnosticEngine) {`
	`61`	`+ guard let raw = bridged.raw else {`
`62`	`62`	`return nil`
`63`	`63`	`}`
`64`		`- self.bridged = BridgedDiagEngine(object: object)`
	`64`	`+ self.bridged = BridgedDiagnosticEngine(raw: raw)`
`65`	`65`	`}`
`66`	`66`
`67`	`67`	`public func diagnose(_ position: SourceLoc?,`
Original file line number	Diff line number	Diff line change
`@@ -175,7 +175,7 @@ BridgedNominalTypeDecl BridgedPassContext::getSwiftArrayDecl() const {`
`175`	`175`	`// AST`
`176`	`176`
`177`	`177`	`SWIFT_IMPORT_UNSAFE BRIDGED_INLINE`
`178`		`-BridgedDiagEngine BridgedPassContext::getDiagnosticEngine() const {`
	`178`	`+BridgedDiagnosticEngine BridgedPassContext::getDiagnosticEngine() const {`
`179`	`179`	`swift::SILModule *mod = invocation->getPassManager()->getModule();`
`180`	`180`	`return {&mod->getASTContext().Diags};`
`181`	`181`	`}`