embedded: fix source location of diagnostics which appear in imported modules

SwiftCompilerSources/Sources/AST/DiagnosticEngine.swift

@@ -138,24 +138,40 @@ public struct DiagnosticEngine {
     diagnose(id, args, at: position, highlight: highlight, fixIts: fixIts)
   }
 
-  public func diagnose(_ diagnostic: Diagnostic) {
-    diagnose(diagnostic.id, diagnostic.arguments, at: diagnostic.position)
+  public func diagnose<SourceLocation: ProvidingSourceLocation>(_ diagnostic: Diagnostic<SourceLocation>) {
+    let loc = diagnostic.location.getSourceLocation(diagnosticEngine: self)
+    diagnose(diagnostic.id, diagnostic.arguments, at: loc)
   }
+
+  /// Loads the file at `path` and returns a `SourceLoc` pointing to `line` and `column` in the file.
+  /// Returns nil if the file cannot be loaded.
+  public func getLocationFromExternalSource(path: StringRef, line: Int, column: Int) -> SourceLoc? {
+    return SourceLoc(bridged: bridged.getLocationFromExternalSource(path: path._bridged, line: line, column: column))
+  }
+}
+
+/// Something which can provide a `SourceLoc` for diagnostics.
+public protocol ProvidingSourceLocation {
+  func getSourceLocation(diagnosticEngine: DiagnosticEngine) -> SourceLoc?
+}
+
+extension SourceLoc: ProvidingSourceLocation {
+  public func getSourceLocation(diagnosticEngine: DiagnosticEngine) -> SourceLoc? { self }
 }
 
 /// A utility struct which allows throwing a Diagnostic.
-public struct Diagnostic : Error {
+public struct Diagnostic<SourceLocation: ProvidingSourceLocation> : Error {
   public let id: DiagID
   public let arguments: [DiagnosticArgument]
-  public let position: SourceLoc?
+  public let location: SourceLocation
 
-  public init(_ id: DiagID, _ arguments: DiagnosticArgument..., at position: SourceLoc?) {
-    self.init(id, arguments, at: position)
+  public init(_ id: DiagID, _ arguments: DiagnosticArgument..., at location: SourceLocation) {
+    self.init(id, arguments, at: location)
   }
 
-  public init(_ id: DiagID, _ arguments: [DiagnosticArgument], at position: SourceLoc?) {
+  public init(_ id: DiagID, _ arguments: [DiagnosticArgument], at location: SourceLocation) {
     self.id = id
     self.arguments = arguments
-    self.position = position
+    self.location = location
   }
 }

SwiftCompilerSources/Sources/Basic/SourceLoc.swift

@@ -12,8 +12,11 @@
 
 import BasicBridging
 
+/// Represents a location in source code.
+/// It is basically a pointer into a buffer of the loaded source file (managed by `DiagnosticEngine`).
+/// In contrast to just having a filename+line+column, this allows displaying the context around
+/// the location when printing diagnostics.
 public struct SourceLoc {
-  /// Points into a source file.
   public let bridged: BridgedSourceLoc
 
   public init?(bridged: BridgedSourceLoc) {

SwiftCompilerSources/Sources/Optimizer/ModulePasses/EmbeddedSwiftDiagnostics.swift

@@ -37,7 +37,7 @@ let embeddedSwiftDiagnostics = ModulePass(name: "embedded-swift-diagnostics") {
     do {
       assert(checker.callStack.isEmpty)
       try checker.checkFunction(function)
-    } catch let error as Diagnostic {
+    } catch let error as Diagnostic<Location> {
       checker.diagnose(error)
     } catch {
       fatalError("unknown error thrown")
@@ -248,9 +248,9 @@ private struct FunctionChecker {
     }
   }
 
-  mutating func diagnose(_ error: Diagnostic) {
+  mutating func diagnose(_ error: Diagnostic<Location>) {
     var diagPrinted = false
-    if error.position != nil {
+    if error.location.hasValidLineNumber {
       context.diagnosticEngine.diagnose(error)
       diagPrinted = true
     }

SwiftCompilerSources/Sources/SIL/ASTExtensions.swift

@@ -76,16 +76,16 @@ extension Conformance {
 
 extension DiagnosticEngine {
   public func diagnose(_ id: DiagID, _ args: DiagnosticArgument..., at location: Location) {
-    diagnose(id, args, at: location.sourceLoc)
+    diagnose(id, args, at: location.getSourceLocation(diagnosticEngine: self))
   }
 
   public func diagnose(_ id: DiagID, _ args: [DiagnosticArgument], at location: Location) {
-    diagnose(id, args, at: location.sourceLoc)
+    diagnose(id, args, at: location.getSourceLocation(diagnosticEngine: self))
   }
 }
 
-extension Diagnostic {
+extension Diagnostic where SourceLocation == Location {
   public init(_ id: DiagID, _ arguments: DiagnosticArgument..., at location: Location) {
-    self.init(id, arguments, at: location.sourceLoc)
+    self.init(id, arguments, at: location)
   }
 }

SwiftCompilerSources/Sources/SIL/Location.swift

@@ -13,7 +13,19 @@
 import SILBridging
 import AST
 
-public struct Location: Equatable, CustomStringConvertible {
+/// Represents a location in source code.
+/// `Location` is used in SIL and by the Optimizer.
+///
+/// When compiling a Swift file, `Location` is basically a `SourceLoc` + information about the
+/// containing debug scope. In this case the `SourceLoc` is directly retrieved from the AST nodes.
+///
+/// However, for debug info which is de-serialized from a swiftmodule file, the location consists of
+/// a filename + line and column indices. From such a location, a `SourceLoc` can only be created by
+/// loading the file with `DiagnosticEngine.getLocationFromExternalSource`.
+///
+/// In case of parsing textual SIL (e.g. with `sil-opt`), which does _not_ contain debug line
+/// information, the location is also a `SourceLoc` which points to the textual SIL.
+public struct Location: ProvidingSourceLocation, Equatable, CustomStringConvertible {
   let bridged: BridgedLocation
 
   public var description: String {
@@ -27,6 +39,24 @@ public struct Location: Equatable, CustomStringConvertible {
     return nil
   }
 
+  public var fileNameAndPosition: (path: StringRef, line: Int, column: Int)? {
+    if bridged.isFilenameAndLocation() {
+      let loc = bridged.getFilenameAndLocation()
+      return (StringRef(bridged: loc.path), loc.line, loc.column)
+    }
+    return nil
+  }
+
+  public func getSourceLocation(diagnosticEngine: DiagnosticEngine) -> SourceLoc? {
+    if let sourceLoc = sourceLoc {
+      return sourceLoc
+    }
+    if let (path, line, column) = fileNameAndPosition {
+      return diagnosticEngine.getLocationFromExternalSource(path: path, line: line, column: column)
+    }
+    return nil
+  }
+
   /// Keeps the debug scope but marks it as auto-generated.
   public var autoGenerated: Location {
     Location(bridged: bridged.getAutogeneratedLocation())

include/swift/AST/ASTBridging.h

@@ -592,6 +592,11 @@ void BridgedDiagnosticEngine_diagnose(
     BridgedArrayRef arguments, BridgedSourceLoc highlightStart,
     uint32_t hightlightLength, BridgedArrayRef fixIts);
 
+SWIFT_NAME("BridgedDiagnosticEngine.getLocationFromExternalSource(self:path:line:column:)")
+BridgedSourceLoc BridgedDiagnostic_getLocationFromExternalSource(
+    BridgedDiagnosticEngine bridgedEngine, BridgedStringRef path,
+    SwiftInt line, SwiftInt column);
+
 SWIFT_NAME("getter:BridgedDiagnosticEngine.hadAnyError(self:)")
 bool BridgedDiagnosticEngine_hadAnyError(BridgedDiagnosticEngine);
 

include/swift/SIL/SILBridging.h

@@ -392,6 +392,12 @@ enum class BridgedMemoryBehavior {
 struct BridgedLocation {
   uint64_t storage[3];
 
+  struct FilenameAndLocation {
+    BridgedStringRef path;
+    SwiftInt line;
+    SwiftInt column;
+  };
+
   BRIDGED_INLINE BridgedLocation(const swift::SILDebugLocation &loc);
   BRIDGED_INLINE const swift::SILDebugLocation &getLoc() const;
 
@@ -402,6 +408,8 @@ struct BridgedLocation {
   BRIDGED_INLINE bool isInlined() const;
   BRIDGED_INLINE bool isEqualTo(BridgedLocation rhs) const;
   SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedSourceLoc getSourceLocation() const;
+  BRIDGED_INLINE bool isFilenameAndLocation() const;
+  SWIFT_IMPORT_UNSAFE BRIDGED_INLINE FilenameAndLocation getFilenameAndLocation() const;
   BRIDGED_INLINE bool hasSameSourceLocation(BridgedLocation rhs) const;
   SWIFT_IMPORT_UNSAFE BRIDGED_INLINE OptionalBridgedDeclObj getDecl() const;
   static BRIDGED_INLINE BridgedLocation fromNominalTypeDecl(BridgedDeclObj decl);

include/swift/SIL/SILBridgingImpl.h

@@ -639,6 +639,13 @@ BridgedSourceLoc BridgedLocation::getSourceLocation() const {
   swift::SourceLoc sourceLoc = silLoc.getSourceLoc();
   return BridgedSourceLoc(sourceLoc.getOpaquePointerValue());
 }
+bool BridgedLocation::isFilenameAndLocation() const {
+  return getLoc().getLocation().isFilenameAndLocation();
+}
+BridgedLocation::FilenameAndLocation BridgedLocation::getFilenameAndLocation() const {
+  auto fnal = getLoc().getLocation().getFilenameAndLocation();
+  return {BridgedStringRef(fnal->filename), (SwiftInt)fnal->line, (SwiftInt)fnal->column};
+}
 bool BridgedLocation::hasSameSourceLocation(BridgedLocation rhs) const {
   return getLoc().hasSameSourceLocation(rhs.getLoc());
 }

lib/AST/Bridging/DiagnosticsBridging.cpp

@@ -15,6 +15,7 @@
 #include "swift/AST/DiagnosticEngine.h"
 #include "swift/AST/DiagnosticsCommon.h"
 #include "swift/Basic/Assertions.h"
+#include "swift/Basic/SourceManager.h"
 
 using namespace swift;
 
@@ -78,6 +79,14 @@ void BridgedDiagnosticEngine_diagnose(
   }
 }
 
+BridgedSourceLoc BridgedDiagnostic_getLocationFromExternalSource(
+    BridgedDiagnosticEngine bridgedEngine, BridgedStringRef path,
+    SwiftInt line, SwiftInt column) {
+  auto *d = bridgedEngine.unbridged();
+  auto loc = d->SourceMgr.getLocFromExternalSource(path.unbridged(), line, column);
+  return BridgedSourceLoc(loc.getOpaquePointerValue());
+}
+
 bool BridgedDiagnosticEngine_hadAnyError(
     BridgedDiagnosticEngine bridgedEngine) {
   return bridgedEngine.unbridged()->hadAnyError();

lib/Frontend/Frontend.cpp

@@ -207,7 +207,6 @@ SerializationOptions CompilerInvocation::computeSerializationOptions(
   serializationOpts.ModuleLinkName = opts.ModuleLinkName;
   serializationOpts.UserModuleVersion = opts.UserModuleVersion;
   serializationOpts.AllowableClients = opts.AllowableClients;
-  serializationOpts.SerializeDebugInfoSIL = opts.SerializeDebugInfoSIL;
 
   serializationOpts.PublicDependentLibraries =
       getIRGenOptions().PublicLinkLibraries;
@@ -268,6 +267,18 @@ SerializationOptions CompilerInvocation::computeSerializationOptions(
   serializationOpts.EmbeddedSwiftModule =
       LangOpts.hasFeature(Feature::Embedded);
 
+  serializationOpts.SerializeDebugInfoSIL = opts.SerializeDebugInfoSIL;
+
+  // Enable serialization of debug info in embedded mode.
+  // This is important to get diagnostics for errors which are located in imported modules.
+  // Such errors can sometimes only be detected when building the client module, because
+  // the error can be in a generic function which is specialized in the client module.
+  if (serializationOpts.EmbeddedSwiftModule &&
+      // Except for the stdlib core. We don't want to get error locations inside stdlib internals.
+      !getParseStdlib()) {
+    serializationOpts.SerializeDebugInfoSIL = true;
+  }
+
   serializationOpts.IsOSSA = getSILOptions().EnableOSSAModules;
 
   serializationOpts.SkipNonExportableDecls =

test/embedded/multi-module-debug-info.swift

@@ -0,0 +1,35 @@
+// RUN: %empty-directory(%t)
+// RUN: %{python} %utils/split_file.py -o %t %s
+
+// RUN: %target-swift-frontend -emit-module -o %t/MyModule.swiftmodule %t/MyModule.swift -enable-experimental-feature Embedded -parse-as-library
+// RUN: %target-swift-frontend -c -I %t %t/Main.swift -enable-experimental-feature Embedded -verify -o /dev/null
+
+// REQUIRES: OS=macosx || OS=linux-gnu
+// REQUIRES: swift_feature_Embedded
+
+// BEGIN MyModule.swift
+
+public struct MyError: Error {
+}
+
+@inline(never)
+public func foo<T>(_ t: T) throws {
+  throw MyError() // expected-error {{cannot use a value of protocol type 'any Error' in embedded Swift}}
+}
+
+@inline(never)
+public func callit<T>(_ t: T) {
+  do {
+    try foo(t)
+  } catch {
+  }
+}
+
+// BEGIN Main.swift
+
+import MyModule
+
+public func testit() {
+  callit(27) // expected-note {{generic specialization called here}}
+}
+