Repository: hylo-lang/hylo (swift)

Issue:
Copying standard library sources into a resources directory is bad for standard library development.
You get diagnostics and then you're sent to the copy of the source file, whence you make fixes that don't stick.

- If there's a way to avoid doing that copy, we should do so!
- If we absolutely must do that copy:
    - the files should be made read-only so we don't try to edit them
    - some facility for remapping source locations should be introduced so that diagnostics point to the real source.

Pull request:
In an attempt to speed up testing, instead of parsing the library each time we need an AST for it as a starting point for testing, store hosted and/or freestanding ASTs in global constants, which are initialized once, lazily and thread-safely in Swift.

Also fixes #932.

## Results

- Debug:
   - Parallel testing (`time swift test --parallel --skip-build`):
      - Before: `440.54s user 23.45s system 834% cpu 55.578 total`
      - After:    `441.36s user 23.85s system 834% cpu 55.723 total`
   - Serial testing  (`time swift test --skip-build`):
      - Before: `325.20s user 11.28s system 96% cpu 5:49.06 total`
      - After:    `289.28s user 10.43s system 95% cpu 5:13.35 total`
- Release:
   - Parallel testing (`time swift test -c release --parallel --skip-build`):
      - Before: `263.19s user 29.15s system 618% cpu 47.302 total`
      - After:    `264.44s user 28.92s system 624% cpu 46.998 total`
   - Serial testing  (`time swift test -c release --skip-build`):
      - Before: `211.38s user 15.21s system 144% cpu 2:36.36 total`
      - After:   `186.19s user 14.69s system 155% cpu 2:09.02 total`

The `time` man page is unhelpful but from my experiment (`time sleep 5` yields `0.00s user 0.00s system 0% cpu 5.014 total`), the number we mostly care about is the last one, total.

This change seems to make zero difference if parallel testing is enabled, for reasons I don't understand, and the wins from parallel testing for a developer far outweigh any improvement we get from this change when testing serially.  I think it's still a change worth making.

Patch:
diff --git a/Package.swift b/Package.swift
index c4c8a31c3..8d9f29d96 100644
--- a/Package.swift
+++ b/Package.swift
@@ -88,7 +88,6 @@ let package = Package(
       dependencies: [
         "Utils",
         "Core",
-        "StandardLibrary",
         .product(name: "Collections", package: "swift-collections"),
         .product(name: "Durian", package: "Durian"),
         .product(name: "BigInt", package: "BigInt"),
@@ -135,8 +134,9 @@ let package = Package(
 
     .target(
       name: "StandardLibrary",
+      dependencies: ["FrontEnd", "Utils"],
       path: "StandardLibrary",
-      resources: [.copy("Sources")],
+      exclude: ["Sources"],
       swiftSettings: allTargetsSwiftSettings),
 
     .plugin(
diff --git a/Sources/Driver/Driver.swift b/Sources/Driver/Driver.swift
index 21145fab9..5dde40d1d 100644
--- a/Sources/Driver/Driver.swift
+++ b/Sources/Driver/Driver.swift
@@ -44,14 +44,9 @@ public struct Driver: ParsableCommand {
     help: "Import the built-in module.")
   private var importBuiltinModule: Bool = false
 
-  @Flag(
-    name: [.customLong("unhosted")],
-    help: "Load only the core library, omitting any definitions that depend on OS support.")
-  private var unhosted: Bool = false
-
   @Flag(
     name: [.customLong("freestanding")],
-    help: "Compile in freestanding mode (no libc).")
+    help: "Import only the freestanding core of the standard library, omitting any definitions that depend on having an operating system.")
   private var freestanding: Bool = false
 
   @Flag(
@@ -160,10 +155,9 @@ public struct Driver: ParsableCommand {
     }
 
     let productName = makeProductName(inputs)
+
     /// An instance that includes just the standard library.
-    var ast = AST(
-      libraryRoot: unhosted ? coreLibrarySourceRoot : standardLibrarySourceRoot,
-      for: CompilerConfiguration(freestanding ? ["freestanding"] : []))
+    var ast = freestanding ? Host.freestandingLibraryAST : Host.hostedLibraryAST
 
     // The module whose Hylo files were given on the command-line
     let sourceModule = try ast.makeModule(
diff --git a/Sources/FrontEnd/AST+Extensions.swift b/StandardLibrary/AST+Extensions.swift
similarity index 86%
rename from Sources/FrontEnd/AST+Extensions.swift
rename to StandardLibrary/AST+Extensions.swift
index 55d8a9713..1145587a6 100644
--- a/Sources/FrontEnd/AST+Extensions.swift
+++ b/StandardLibrary/AST+Extensions.swift
@@ -1,12 +1,12 @@
 import Core
 import Foundation
-import StandardLibrary
+import FrontEnd
 import Utils
 
 extension AST {
 
   /// Creates an instance that includes the Hylo library whose sources are rooted at `libraryRoot`.
-  public init(libraryRoot: URL, for compiler: CompilerConfiguration) {
+  init(libraryRoot: URL, for compiler: CompilerConfiguration) {
     self.init(for: compiler)
     do {
       var diagnostics = DiagnosticSet()
diff --git a/StandardLibrary/StandardLibrary.swift b/StandardLibrary/StandardLibrary.swift
index bcf85ba7f..3ccd5849c 100644
--- a/StandardLibrary/StandardLibrary.swift
+++ b/StandardLibrary/StandardLibrary.swift
@@ -1,10 +1,33 @@
+import Core
 import Foundation
+import Utils
 
-fileprivate let libraryRoot = Bundle.module.resourceURL!
+// This path points into the source tree rather than to some copy so that when diagnostics are
+// issued for the standard library, they point to the original source files and edits to those
+// source files actually fix the problems (see https://github.com/hylo-lang/hylo/issues/932).  If we
+// ever change that, some other mechanism is needed to ensure that diagnostics refer to the original
+// source files.
+//
+/// The parent directory of the standard library sources directory.
+private let libraryRoot = URL(fileURLWithPath: #filePath).deletingLastPathComponent()
 
-/// The root URL of Hylo's standard library.
-public let standardLibrarySourceRoot = libraryRoot.appendingPathComponent("Sources")
+/// The root of a directory hierarchy containing all the standard library sources.
+private let hostedLibrarySourceRoot = libraryRoot.appendingPathComponent("Sources")
 
-/// The root URL of Hylo's core library.
-public let coreLibrarySourceRoot = standardLibrarySourceRoot.appendingPathComponent("Core")
+/// The root of a directory hierarchy containing the sources for the standard library's freestanding
+/// core.
+private let freestandingLibrarySourceRoot = hostedLibrarySourceRoot.appendingPathComponent("Core")
 
+extension Utils.Host {
+
+  /// An AST representing the whole standard library, conditionally compiled for targeting the host
+  /// platform.
+  public static let hostedLibraryAST
+    = AST(libraryRoot: hostedLibrarySourceRoot, for: CompilerConfiguration([]))
+
+  /// An AST representing the freestanding core of standard library, conditionally compiled for
+  /// targeting the host platform.
+  public static let freestandingLibraryAST
+    = AST(libraryRoot: freestandingLibrarySourceRoot, for: CompilerConfiguration(["freestanding"]))
+
+}


Test patch:
diff --git a/Tests/HyloTests/LoweringTests.swift b/Tests/HyloTests/LoweringTests.swift
index 6d3081e2e..c4517910f 100644
--- a/Tests/HyloTests/LoweringTests.swift
+++ b/Tests/HyloTests/LoweringTests.swift
@@ -14,7 +14,8 @@ extension XCTestCase {
     try checkAnnotatedHyloFileDiagnostics(inFileAt: hyloFilePath, expectSuccess: expectSuccess) {
       (valSource, diagnostics) in
       // Note: built-in module is visible so that we can test built-in function calls.
-      var ast = AST(libraryRoot: coreLibrarySourceRoot, for: CompilerConfiguration())
+      var ast = Host.freestandingLibraryAST
+
       let module = try ast.makeModule(
         valSource.baseName, sourceCode: [valSource], builtinModuleAccess: true,
         diagnostics: &diagnostics)
diff --git a/Tests/HyloTests/TypeCheckerTests.swift b/Tests/HyloTests/TypeCheckerTests.swift
index a5fb59edb..7790ff42b 100644
--- a/Tests/HyloTests/TypeCheckerTests.swift
+++ b/Tests/HyloTests/TypeCheckerTests.swift
@@ -2,6 +2,7 @@ import Core
 import FrontEnd
 import StandardLibrary
 import TestUtils
+import Utils
 import XCTest
 
 extension XCTestCase {
@@ -13,7 +14,7 @@ extension XCTestCase {
     try checkAnnotatedHyloFileDiagnostics(inFileAt: hyloFilePath, expectSuccess: expectSuccess) {
       (source, diagnostics) in
 
-      var ast = AST(libraryRoot: coreLibrarySourceRoot, for: CompilerConfiguration())
+      var ast = Host.freestandingLibraryAST
 
       _ = try ast.makeModule(
         source.baseName, sourceCode: [source], builtinModuleAccess: true,
diff --git a/Tests/ManglingTests/ManglingTests.swift b/Tests/ManglingTests/ManglingTests.swift
index 4e54f55f4..6a716e083 100644
--- a/Tests/ManglingTests/ManglingTests.swift
+++ b/Tests/ManglingTests/ManglingTests.swift
@@ -3,6 +3,7 @@ import FrontEnd
 import IR
 import StandardLibrary
 import TestUtils
+import Utils
 import XCTest
 
 final class ManglingTests: XCTestCase {
@@ -67,7 +68,7 @@ final class ManglingTests: XCTestCase {
 
     let input = SourceFile(synthesizedText: text, named: "main")
     let (p, m) = try checkNoDiagnostic { (d) in
-      var ast = AST(libraryRoot: standardLibrarySourceRoot, for: CompilerConfiguration())
+      var ast = Utils.Host.hostedLibraryAST
       let main = try ast.makeModule("Main", sourceCode: [input], diagnostics: &d)
       let base = ScopedProgram(ast)
       return (try TypedProgram(annotating: base, reportingDiagnosticsTo: &d), main)
@@ -94,7 +95,7 @@ final class ManglingTests: XCTestCase {
 
   func testTypes() throws {
     let p = try checkNoDiagnostic { (d) in
-      let ast = AST(libraryRoot: standardLibrarySourceRoot, for: CompilerConfiguration())
+      let ast = Host.hostedLibraryAST
       let base = ScopedProgram(ast)
       return try TypedProgram(annotating: base, reportingDiagnosticsTo: &d)
     }