SwiftCompilerSources: refactor DiagnosticEngine

* move it from the SIL to the AST module (where it belongs)
* change the signature of `diagnose` from `diagnose(location, .some_error)` to `diagnose(.some_error, at: location)`
* add an overload to allow passing a `SIL.Location` directly to `diagnose`
* add a `Diagnostic : Error` utility struct which allows throwing a `Diagnostic`

Author: eeckstein

SwiftCompilerSources/Sources/AST/CMakeLists.txt

@@ -12,6 +12,7 @@ add_swift_compiler_module(AST
   SOURCES
     Declarations.swift
     Conformance.swift
+    DiagnosticEngine.swift
     GenericSignature.swift
     Registration.swift
     SubstitutionMap.swift

SwiftCompilerSources/Sources/Optimizer/Utilities/DiagnosticEngine.swift renamed to SwiftCompilerSources/Sources/AST/DiagnosticEngine.swift

@@ -13,40 +13,34 @@
 import ASTBridging
 
 import Basic
-import SIL
 
-typealias DiagID = BridgedDiagID
+public typealias DiagID = BridgedDiagID
 
-protocol DiagnosticArgument {
+public protocol DiagnosticArgument {
   func _withBridgedDiagnosticArgument(_ fn: (BridgedDiagnosticArgument) -> Void)
 }
 extension String: DiagnosticArgument {
-  func _withBridgedDiagnosticArgument(_ fn: (BridgedDiagnosticArgument) -> Void) {
+  public func _withBridgedDiagnosticArgument(_ fn: (BridgedDiagnosticArgument) -> Void) {
     _withBridgedStringRef { fn(BridgedDiagnosticArgument($0)) }
   }
 }
 extension StringRef: DiagnosticArgument {
-  func _withBridgedDiagnosticArgument(_ fn: (BridgedDiagnosticArgument) -> Void) {
+  public func _withBridgedDiagnosticArgument(_ fn: (BridgedDiagnosticArgument) -> Void) {
     fn(BridgedDiagnosticArgument(_bridged))
   }
 }
 extension Int: DiagnosticArgument {
-  func _withBridgedDiagnosticArgument(_ fn: (BridgedDiagnosticArgument) -> Void) {
+  public func _withBridgedDiagnosticArgument(_ fn: (BridgedDiagnosticArgument) -> Void) {
     fn(BridgedDiagnosticArgument(self))
   }
 }
 extension Type: DiagnosticArgument {
-  func _withBridgedDiagnosticArgument(_ fn: (BridgedDiagnosticArgument) -> Void) {
-    fn(bridged.asDiagnosticArgument())
-  }
-}
-extension DeclRef: DiagnosticArgument {
-  func _withBridgedDiagnosticArgument(_ fn: (BridgedDiagnosticArgument) -> Void) {
+  public func _withBridgedDiagnosticArgument(_ fn: (BridgedDiagnosticArgument) -> Void) {
     fn(bridged.asDiagnosticArgument())
   }
 }
 
-struct DiagnosticFixIt {
+public struct DiagnosticFixIt {
   let start: SourceLoc
   let byteLength: Int
   let text: String
@@ -67,10 +61,10 @@ struct DiagnosticFixIt {
   }
 }
 
-struct DiagnosticEngine {
+public struct DiagnosticEngine {
   private let bridged: BridgedDiagnosticEngine
 
-  init(bridged: BridgedDiagnosticEngine) {
+  public init(bridged: BridgedDiagnosticEngine) {
     self.bridged = bridged
   }
   init?(bridged: BridgedNullableDiagnosticEngine) {
@@ -80,9 +74,9 @@ struct DiagnosticEngine {
     self.bridged = BridgedDiagnosticEngine(raw: raw)
   }
 
-  func diagnose(_ position: SourceLoc?,
-                       _ id: DiagID,
+  public func diagnose(_ id: DiagID,
                        _ args: [DiagnosticArgument],
+                       at position: SourceLoc?,
                        highlight: CharSourceRange? = nil,
                        fixIts: [DiagnosticFixIt] = []) {
 
@@ -136,11 +130,32 @@ struct DiagnosticEngine {
     closure()
   }
 
-  func diagnose(_ position: SourceLoc?,
-                       _ id: DiagID,
+  public func diagnose(_ id: DiagID,
                        _ args: DiagnosticArgument...,
+                       at position: SourceLoc?,
                        highlight: CharSourceRange? = nil,
                        fixIts: DiagnosticFixIt...) {
-    diagnose(position, id, args, highlight: highlight, fixIts: fixIts)
+    diagnose(id, args, at: position, highlight: highlight, fixIts: fixIts)
+  }
+
+  public func diagnose(_ diagnostic: Diagnostic) {
+    diagnose(diagnostic.id, diagnostic.arguments, at: diagnostic.position)
+  }
+}
+
+/// A utility struct which allows throwing a Diagnostic.
+public struct Diagnostic : Error {
+  public let id: DiagID
+  public let arguments: [DiagnosticArgument]
+  public let position: SourceLoc?
+
+  public init(_ id: DiagID, _ arguments: DiagnosticArgument..., at position: SourceLoc?) {
+    self.init(id, arguments, at: position)
+  }
+
+  public init(_ id: DiagID, _ arguments: [DiagnosticArgument], at position: SourceLoc?) {
+    self.id = id
+    self.arguments = arguments
+    self.position = position
   }
 }

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/DiagnoseInfiniteRecursion.swift

@@ -289,7 +289,7 @@ private struct Analysis {
     worklist.pushIfNotVisited(function.entryBlock)
     while let block = worklist.pop() {
       if case .recursive(let apply) = block.getKind(for: invariants, context) {
-        context.diagnosticEngine.diagnose(apply.location.sourceLoc, .warn_infinite_recursive_call)
+        context.diagnosticEngine.diagnose(.warn_infinite_recursive_call, at: apply.location)
       } else {
         for succ in block.successors where isInInfiniteRecursionLoop(succ) {
           worklist.pushIfNotVisited(succ)

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceDiagnostics.swift

@@ -135,7 +135,7 @@ private struct DiagnoseDependence {
 
   func diagnose(_ position: SourceLoc?, _ id: DiagID,
                 _ args: DiagnosticArgument...) {
-    context.diagnosticEngine.diagnose(position, id, args)
+    context.diagnosticEngine.diagnose(id, args, at: position)
   }
 
   /// Check that this use is inside the dependence scope.

SwiftCompilerSources/Sources/Optimizer/ModulePasses/DiagnoseUnknownConstValues.swift

@@ -39,8 +39,8 @@ let diagnoseUnknownConstValues = ModulePass(name: "diagnose-unknown-const-values
 private func verifyGlobals(_ context: ModulePassContext) {
   for gv in context.globalVariables where gv.isConst {
     if gv.staticInitValue == nil {
-      context.diagnosticEngine.diagnose(gv.varDecl?.location.sourceLoc,
-                                        .require_const_initializer_for_const)
+      context.diagnosticEngine.diagnose(.require_const_initializer_for_const,
+                                        at: gv.varDecl?.location.sourceLoc)
     }
   }
 }
@@ -66,8 +66,8 @@ private func verifyLocal(debugValueInst: DebugValueInst,
   }
 
   if !constExprState.isConstantValue(debugValueInst.operand.value) {
-    context.diagnosticEngine.diagnose(debugValueInst.location.sourceLoc,
-                                      .require_const_initializer_for_const)
+    context.diagnosticEngine.diagnose(.require_const_initializer_for_const,
+                                      at: debugValueInst.location)
   }
 }
 
@@ -92,8 +92,8 @@ private func verifyCallArguments(apply: FullApplySite,
   for (paramIdx, param) in calleeFn.convention.parameters.enumerated() where param.hasOption(.const) {
     let matchingOperand = apply.parameterOperands[paramIdx]
     if !constExprState.isConstantValue(matchingOperand.value) {
-      context.diagnosticEngine.diagnose(apply.location.sourceLoc,
-                                        .require_const_arg_for_parameter)
+      context.diagnosticEngine.diagnose(.require_const_arg_for_parameter,
+                                        at: apply.location)
     }
   }
 }

SwiftCompilerSources/Sources/Optimizer/ModulePasses/MandatoryPerformanceOptimizations.swift

@@ -137,11 +137,11 @@ private func optimize(function: Function, _ context: FunctionPassContext, _ modu
       // We need to de-virtualize deinits of non-copyable types to be able to specialize the deinitializers.
       case let destroyValue as DestroyValueInst:
         if !devirtualizeDeinits(of: destroyValue, simplifyCtxt) {
-          context.diagnosticEngine.diagnose(destroyValue.location.sourceLoc, .deinit_not_visible)
+          context.diagnosticEngine.diagnose(.deinit_not_visible, at: destroyValue.location)
         }
       case let destroyAddr as DestroyAddrInst:
         if !devirtualizeDeinits(of: destroyAddr, simplifyCtxt) {
-          context.diagnosticEngine.diagnose(destroyAddr.location.sourceLoc, .deinit_not_visible)
+          context.diagnosticEngine.diagnose(.deinit_not_visible, at: destroyAddr.location)
         }
 
       case let iem as InitExistentialMetatypeInst:
@@ -309,7 +309,7 @@ private func checkForGenericMethods(in witnessTable: WitnessTable,
        let witness,
        witness.isGeneric
     {
-      context.diagnosticEngine.diagnose(errorLocation.sourceLoc, .cannot_specialize_witness_method, requirement)
+      context.diagnosticEngine.diagnose(.cannot_specialize_witness_method, requirement, at: errorLocation)
       return
     }
   }
@@ -318,7 +318,7 @@ private func checkForGenericMethods(in witnessTable: WitnessTable,
 private func checkVTablesForGenericFunctions(_ context: ModulePassContext) {
   for vTable in context.vTables where !vTable.class.isGenericAtAnyLevel {
     for entry in vTable.entries where entry.implementation.isGeneric {
-      context.diagnosticEngine.diagnose(entry.methodDecl.location.sourceLoc, .non_final_generic_class_function)
+      context.diagnosticEngine.diagnose(.non_final_generic_class_function, at: entry.methodDecl.location)
     }
   }
 }

SwiftCompilerSources/Sources/Optimizer/Utilities/CMakeLists.txt

@@ -11,7 +11,6 @@ swift_compiler_sources(Optimizer
   AddressUtils.swift
   BorrowUtils.swift
   SpecializationCloner.swift
-  DiagnosticEngine.swift
   Devirtualization.swift
   EscapeUtils.swift
   ForwardingUtils.swift

SwiftCompilerSources/Sources/Optimizer/Utilities/GenericSpecialization.swift

@@ -50,7 +50,7 @@ private struct VTableSpecializer {
     let classDecl = classType.nominal! as! ClassDecl
     guard let origVTable = context.lookupVTable(for: classDecl) else {
       if context.enableWMORequiredDiagnostics {
-        context.diagnosticEngine.diagnose(errorLocation.sourceLoc, .cannot_specialize_class, classType)
+        context.diagnosticEngine.diagnose(.cannot_specialize_class, classType, at: errorLocation)
       }
       return
     }
@@ -91,7 +91,7 @@ private struct VTableSpecializer {
             context.loadFunction(function: entry.implementation, loadCalleesRecursively: true),
             let specializedMethod = context.specialize(function: entry.implementation, for: methodSubs) else
       {
-        context.diagnosticEngine.diagnose(entry.methodDecl.location.sourceLoc, .non_final_generic_class_function)
+        context.diagnosticEngine.diagnose(.non_final_generic_class_function, at: entry.methodDecl.location)
         return nil
       }
       notifyNewFunction(specializedMethod)
@@ -131,7 +131,7 @@ func specializeWitnessTable(forConformance conformance: Conformance,
             context.loadFunction(function: origMethod, loadCalleesRecursively: true),
             let specializedMethod = context.specialize(function: origMethod, for: methodSubs) else
       {
-        context.diagnosticEngine.diagnose(errorLocation.sourceLoc, .cannot_specialize_witness_method, requirement)
+        context.diagnosticEngine.diagnose(.cannot_specialize_witness_method, requirement, at: errorLocation)
         return origEntry
       }
       return .method(requirement: requirement, witness: specializedMethod)

SwiftCompilerSources/Sources/SIL/ASTExtensions.swift

@@ -62,3 +62,19 @@ extension Conformance {
     return function.bridged.conformanceMatchesActorIsolation(bridged)
   }
 }
+
+extension DiagnosticEngine {
+  public func diagnose(_ id: DiagID, _ args: DiagnosticArgument..., at location: Location) {
+    diagnose(id, args, at: location.sourceLoc)
+  }
+
+  public func diagnose(_ id: DiagID, _ args: [DiagnosticArgument], at location: Location) {
+    diagnose(id, args, at: location.sourceLoc)
+  }
+}
+
+extension Diagnostic {
+  public init(_ id: DiagID, _ arguments: DiagnosticArgument..., at location: Location) {
+    self.init(id, arguments, at: location.sourceLoc)
+  }
+}

SwiftCompilerSources/Sources/SIL/DeclRef.swift

@@ -31,3 +31,9 @@ public struct DeclRef: CustomStringConvertible, NoReflectionChildren {
     lhs.bridged.isEqualTo(rhs.bridged)
   }
 }
+
+extension DeclRef: DiagnosticArgument {
+  public func _withBridgedDiagnosticArgument(_ fn: (BridgedDiagnosticArgument) -> Void) {
+    fn(bridged.asDiagnosticArgument())
+  }
+}