refactor cli/src/build.rs

crates/cli/Cargo.toml

@@ -69,6 +69,7 @@ roc_wasm_interp = { path = "../wasm_interp", optional = true }
 
 ven_pretty = { path = "../vendor/pretty" }
 
+indoc.workspace = true
 clap.workspace = true
 const_format.workspace = true
 mimalloc.workspace = true

crates/cli/src/build.rs

@@ -58,6 +58,41 @@ pub enum BuildFileError<'a> {
     },
 }
 
+impl<'a> BuildFileError<'a> {
+    fn from_mono_error(error: LoadMonomorphizedError<'a>, compilation_start: Instant) -> Self {
+        match error {
+            LoadMonomorphizedError::LoadingProblem(problem) => {
+                BuildFileError::LoadingProblem(problem)
+            }
+            LoadMonomorphizedError::ErrorModule(module) => BuildFileError::ErrorModule {
+                module,
+                total_time: compilation_start.elapsed(),
+            },
+        }
+    }
+}
+
+pub fn standard_load_config(
+    target: &Triple,
+    order: BuildOrdering,
+    threading: Threading,
+) -> LoadConfig {
+    let target_info = TargetInfo::from(target);
+
+    let exec_mode = match order {
+        BuildOrdering::BuildIfChecks => ExecutionMode::ExecutableIfCheck,
+        BuildOrdering::AlwaysBuild => ExecutionMode::Executable,
+    };
+
+    LoadConfig {
+        target_info,
+        render: RenderTarget::ColorTerminal,
+        palette: DEFAULT_PALETTE,
+        threading,
+        exec_mode,
+    }
+}
+
 #[allow(clippy::too_many_arguments)]
 pub fn build_file<'a>(
     arena: &'a Bump,
@@ -68,132 +103,88 @@ pub fn build_file<'a>(
     link_type: LinkType,
     linking_strategy: LinkingStrategy,
     prebuilt_requested: bool,
-    threading: Threading,
     wasm_dev_stack_bytes: Option<u32>,
     roc_cache_dir: RocCacheDir<'_>,
-    order: BuildOrdering,
+    load_config: LoadConfig,
 ) -> Result<BuiltFile<'a>, BuildFileError<'a>> {
     let compilation_start = Instant::now();
-    let target_info = TargetInfo::from(target);
 
     // Step 1: compile the app and generate the .o file
-    let exec_mode = match order {
+    let loaded =
-        BuildOrdering::BuildIfChecks => ExecutionMode::ExecutableIfCheck,
+        roc_load::load_and_monomorphize(arena, app_module_path.clone(), roc_cache_dir, load_config)
-        BuildOrdering::AlwaysBuild => ExecutionMode::Executable,
+            .map_err(|e| BuildFileError::from_mono_error(e, compilation_start))?;
+
+    build_loaded_file(
+        arena,
+        target,
+        app_module_path,
+        code_gen_options,
+        emit_timings,
+        link_type,
+        linking_strategy,
+        prebuilt_requested,
+        wasm_dev_stack_bytes,
+        loaded,
+        compilation_start,
+    )
+}
+
+#[allow(clippy::too_many_arguments)]
+fn build_loaded_file<'a>(
+    arena: &'a Bump,
+    target: &Triple,
+    app_module_path: PathBuf,
+    code_gen_options: CodeGenOptions,
+    emit_timings: bool,
+    link_type: LinkType,
+    linking_strategy: LinkingStrategy,
+    prebuilt_requested: bool,
+    wasm_dev_stack_bytes: Option<u32>,
+    loaded: roc_load::MonomorphizedModule<'a>,
+    compilation_start: Instant,
+) -> Result<BuiltFile<'a>, BuildFileError<'a>> {
+    let operating_system = roc_target::OperatingSystem::from(target.operating_system);
+
+    let host_input_path = match &loaded.entry_point {
+        EntryPoint::Executable { platform_path, .. } => {
+            use roc_target::OperatingSystem::*;
+
+            let host_filename = match operating_system {
+                Wasi => "host.zig".to_string(),
+                Unix => legacy_host_filename(target, code_gen_options.opt_level).unwrap(),
+                Windows => legacy_host_filename(target, code_gen_options.opt_level).unwrap(),
+            };
+
+            platform_path.with_file_name(host_filename)
+        }
+        _ => unreachable!(),
     };
 
-    let load_config = LoadConfig {
+    let preprocessed_host_path = if linking_strategy == LinkingStrategy::Legacy {
-        target_info,
+        let filename = legacy_host_filename(target, code_gen_options.opt_level).unwrap();
-        // TODO: expose this from CLI?
+        host_input_path.with_file_name(filename)
-        render: RenderTarget::ColorTerminal,
+    } else {
-        palette: DEFAULT_PALETTE,
+        let filename = preprocessed_host_filename(target).unwrap();
-        threading,
+        host_input_path.with_file_name(filename)
-        exec_mode,
-    };
-    let load_result =
-        roc_load::load_and_monomorphize(arena, app_module_path.clone(), roc_cache_dir, load_config);
-    let loaded = match load_result {
-        Ok(loaded) => loaded,
-        Err(LoadMonomorphizedError::LoadingProblem(problem)) => {
-            return Err(BuildFileError::LoadingProblem(problem))
-        }
-        Err(LoadMonomorphizedError::ErrorModule(module)) => {
-            return Err(BuildFileError::ErrorModule {
-                module,
-                total_time: compilation_start.elapsed(),
-            })
-        }
     };
 
     // For example, if we're loading the platform from a URL, it's automatically prebuilt
     // even if the --prebuilt-platform=true CLI flag wasn't set.
-    let is_prebuilt = prebuilt_requested || loaded.uses_prebuilt_platform;
+    let is_platform_prebuilt = prebuilt_requested || loaded.uses_prebuilt_platform;
-    let (app_extension, extension, host_filename) = {
-        use roc_target::OperatingSystem::*;
-
-        match roc_target::OperatingSystem::from(target.operating_system) {
-            Wasi => {
-                if matches!(code_gen_options.opt_level, OptLevel::Development) {
-                    ("wasm", Some("wasm"), "host.zig".to_string())
-                } else {
-                    ("bc", Some("wasm"), "host.zig".to_string())
-                }
-            }
-            Unix => (
-                "o",
-                None,
-                legacy_host_filename(target, code_gen_options.opt_level).unwrap(),
-            ),
-            Windows => (
-                "obj",
-                Some("exe"),
-                legacy_host_filename(target, code_gen_options.opt_level).unwrap(),
-            ),
-        }
-    };
 
     let cwd = app_module_path.parent().unwrap();
-    let mut binary_path = cwd.join(&*loaded.output_path);
+    let mut output_exe_path = cwd.join(&*loaded.output_path);
 
-    if let Some(extension) = extension {
+    if let Some(extension) = operating_system.executable_file_ext() {
-        binary_path.set_extension(extension);
+        output_exe_path.set_extension(extension);
     }
 
-    let host_input_path = if let EntryPoint::Executable { platform_path, .. } = &loaded.entry_point
-    {
-        platform_path.with_file_name(host_filename)
-    } else {
-        unreachable!();
-    };
-
-    // TODO this should probably be moved before load_and_monomorphize.
-    // To do this we will need to preprocess files just for their exported symbols.
-    // Also, we should no longer need to do this once we have platforms on
-    // a package repository, as we can then get prebuilt platforms from there.
-
-    let exposed_values = loaded
-        .exposed_to_host
-        .values
-        .keys()
-        .map(|x| x.as_str(&loaded.interns).to_string())
-        .collect();
-
-    let exposed_closure_types = loaded
-        .exposed_to_host
-        .closure_types
-        .iter()
-        .map(|x| {
-            format!(
-                "{}_{}",
-                x.module_string(&loaded.interns),
-                x.as_str(&loaded.interns)
-            )
-        })
-        .collect();
-
-    let preprocessed_host_path = if linking_strategy == LinkingStrategy::Legacy {
-        host_input_path
-            .with_file_name(legacy_host_filename(target, code_gen_options.opt_level).unwrap())
-    } else {
-        host_input_path.with_file_name(preprocessed_host_filename(target).unwrap())
-    };
-
     // We don't need to spawn a rebuild thread when using a prebuilt host.
     let rebuild_thread = if matches!(link_type, LinkType::Dylib | LinkType::None) {
         None
-    } else if is_prebuilt {
+    } else if is_platform_prebuilt {
         if !preprocessed_host_path.exists() {
-            if prebuilt_requested {
+            invalid_prebuilt_platform(prebuilt_requested, preprocessed_host_path);
-                eprintln!(
-                    "\nBecause I was run with --prebuilt-platform=true, I was expecting this file to exist:\n\n    {}\n\nHowever, it was not there!\n\nIf you have the platform's source code locally, you may be able to generate it by re-running this command with --prebuilt-platform=false\n",
-                    preprocessed_host_path.to_string_lossy()
-                );
-            } else {
-                eprintln!(
-                    "\nI was expecting this file to exist:\n\n    {}\n\nHowever, it was not there!\n\nIf you have the platform's source code locally, you may be able to generate it by re-running this command with --prebuilt-platform=false\n",
-                    preprocessed_host_path.to_string_lossy()
-                );
-            }
 
             std::process::exit(1);
         }
@@ -201,21 +192,48 @@ pub fn build_file<'a>(
         if linking_strategy == LinkingStrategy::Surgical {
             // Copy preprocessed host to executable location.
             // The surgical linker will modify that copy in-place.
-            std::fs::copy(&preprocessed_host_path, binary_path.as_path()).unwrap();
+            std::fs::copy(&preprocessed_host_path, output_exe_path.as_path()).unwrap();
         }
 
         None
     } else {
-        Some(spawn_rebuild_thread(
+        // TODO this should probably be moved before load_and_monomorphize.
+        // To do this we will need to preprocess files just for their exported symbols.
+        // Also, we should no longer need to do this once we have platforms on
+        // a package repository, as we can then get prebuilt platforms from there.
+
+        let exposed_values = loaded
+            .exposed_to_host
+            .values
+            .keys()
+            .map(|x| x.as_str(&loaded.interns).to_string())
+            .collect();
+
+        let exposed_closure_types = loaded
+            .exposed_to_host
+            .closure_types
+            .iter()
+            .map(|x| {
+                format!(
+                    "{}_{}",
+                    x.module_string(&loaded.interns),
+                    x.as_str(&loaded.interns)
+                )
+            })
+            .collect();
+
+        let join_handle = spawn_rebuild_thread(
             code_gen_options.opt_level,
             linking_strategy,
             host_input_path.clone(),
             preprocessed_host_path.clone(),
-            binary_path.clone(),
+            output_exe_path.clone(),
             target,
             exposed_values,
             exposed_closure_types,
-        ))
+        );
+
+        Some(join_handle)
     };
 
     let buf = &mut String::with_capacity(1024);
@@ -235,29 +253,8 @@ pub fn build_file<'a>(
 
         buf.push('\n');
 
-        report_timing(buf, "Read .roc file from disk", module_timing.read_roc_file);
+        use std::fmt::Write;
-        report_timing(buf, "Parse header", module_timing.parse_header);
+        write!(buf, "{}", module_timing).unwrap();
-        report_timing(buf, "Parse body", module_timing.parse_body);
-        report_timing(buf, "Canonicalize", module_timing.canonicalize);
-        report_timing(buf, "Constrain", module_timing.constrain);
-        report_timing(buf, "Solve", module_timing.solve);
-        report_timing(
-            buf,
-            "Find Specializations",
-            module_timing.find_specializations,
-        );
-        let multiple_make_specializations_passes = module_timing.make_specializations.len() > 1;
-        for (i, pass_time) in module_timing.make_specializations.iter().enumerate() {
-            let suffix = if multiple_make_specializations_passes {
-                format!(" (Pass {})", i)
-            } else {
-                String::new()
-            };
-            report_timing(buf, &format!("Make Specializations{}", suffix), *pass_time);
-        }
-        report_timing(buf, "Other", module_timing.other());
-        buf.push('\n');
-        report_timing(buf, "Total", module_timing.total());
 
         if it.peek().is_some() {
             buf.push('\n');
@@ -281,7 +278,7 @@ pub fn build_file<'a>(
                 .join()
                 .expect("Failed to (re)build platform.");
 
-            if emit_timings && !is_prebuilt {
+            if emit_timings && !is_platform_prebuilt {
                 println!(
                     "Finished rebuilding the platform in {} ms\n",
                     rebuild_duration
@@ -336,7 +333,7 @@ pub fn build_file<'a>(
     if let Some(HostRebuildTiming::ConcurrentWithApp(thread)) = opt_rebuild_timing {
         let rebuild_duration = thread.join().expect("Failed to (re)build platform.");
 
-        if emit_timings && !is_prebuilt {
+        if emit_timings && !is_platform_prebuilt {
             println!(
                 "Finished rebuilding the platform in {} ms\n",
                 rebuild_duration
@@ -346,47 +343,45 @@ pub fn build_file<'a>(
 
     // Step 2: link the prebuilt platform and compiled app
     let link_start = Instant::now();
-    let problems = match (linking_strategy, link_type) {
+
+    match (linking_strategy, link_type) {
         (LinkingStrategy::Surgical, _) => {
             roc_linker::link_preprocessed_host(
                 target,
                 &host_input_path,
                 &roc_app_bytes,
-                &binary_path,
+                &output_exe_path,
             );
-
-            problems
         }
         (LinkingStrategy::Additive, _) | (LinkingStrategy::Legacy, LinkType::None) => {
             // Just copy the object file to the output folder.
-            binary_path.set_extension(app_extension);
+            output_exe_path.set_extension(operating_system.object_file_ext());
-            std::fs::write(&binary_path, &*roc_app_bytes).unwrap();
+            std::fs::write(&output_exe_path, &*roc_app_bytes).unwrap();
-            problems
         }
         (LinkingStrategy::Legacy, _) => {
             let app_o_file = tempfile::Builder::new()
                 .prefix("roc_app")
-                .suffix(&format!(".{}", app_extension))
+                .suffix(&format!(".{}", operating_system.object_file_ext()))
                 .tempfile()
                 .map_err(|err| todo!("TODO Gracefully handle tempfile creation error {:?}", err))?;
             let app_o_file = app_o_file.path();
 
             std::fs::write(app_o_file, &*roc_app_bytes).unwrap();
 
+            let builtins_host_tempfile =
+                bitcode::host_tempfile().expect("failed to write host builtins object to tempfile");
+
             let mut inputs = vec![app_o_file.to_str().unwrap()];
 
             if !matches!(link_type, LinkType::Dylib | LinkType::None) {
                 inputs.push(host_input_path.as_path().to_str().unwrap());
             }
 
-            let builtins_host_tempfile =
-                bitcode::host_tempfile().expect("failed to write host builtins object to tempfile");
-
             if matches!(code_gen_options.backend, program::CodeGenBackend::Assembly) {
                 inputs.push(builtins_host_tempfile.path().to_str().unwrap());
             }
 
-            let (mut child, _) = link(target, binary_path.clone(), &inputs, link_type)
+            let (mut child, _) = link(target, output_exe_path.clone(), &inputs, link_type)
                 .map_err(|_| todo!("gracefully handle `ld` failing to spawn."))?;
 
             let exit_status = child
@@ -397,16 +392,14 @@ pub fn build_file<'a>(
             // (and thus deleted) before the child process is done using it!
             let _ = builtins_host_tempfile;
 
-            if exit_status.success() {
+            if !exit_status.success() {
-                problems
-            } else {
                 todo!(
                     "gracefully handle `ld` (or `zig` in the case of wasm with --optimize) returning exit code {:?}",
                     exit_status.code()
                 );
             }
         }
-    };
+    }
 
     let linking_time = link_start.elapsed();
 
@@ -417,20 +410,43 @@ pub fn build_file<'a>(
     let total_time = compilation_start.elapsed();
 
     Ok(BuiltFile {
-        binary_path,
+        binary_path: output_exe_path,
         problems,
         total_time,
         expect_metadata,
     })
 }
 
+fn invalid_prebuilt_platform(prebuilt_requested: bool, preprocessed_host_path: PathBuf) {
+    let prefix = match prebuilt_requested {
+        true => "Because I was run with --prebuilt-platform=true, ",
+        false => "",
+    };
+
+    eprintln!(
+        indoc::indoc!(
+            r#"
+            {}I was expecting this file to exist:
+
+                {}
+
+            However, it was not there!
+
+            If you have the platform's source code locally, you may be able to generate it by re-running this command with --prebuilt-platform=false
+            "#
+        ),
+        prefix,
+        preprocessed_host_path.to_string_lossy(),
+    );
+}
+
 #[allow(clippy::too_many_arguments)]
 fn spawn_rebuild_thread(
     opt_level: OptLevel,
     linking_strategy: LinkingStrategy,
     host_input_path: PathBuf,
     preprocessed_host_path: PathBuf,
-    binary_path: PathBuf,
+    output_exe_path: PathBuf,
     target: &Triple,
     exported_symbols: Vec<String>,
     exported_closure_types: Vec<String>,
@@ -467,7 +483,7 @@ fn spawn_rebuild_thread(
 
                 // Copy preprocessed host to executable location.
                 // The surgical linker will modify that copy in-place.
-                std::fs::copy(&preprocessed_host_path, binary_path.as_path()).unwrap();
+                std::fs::copy(&preprocessed_host_path, output_exe_path.as_path()).unwrap();
             }
             LinkingStrategy::Legacy => {
                 rebuild_host(

crates/cli/src/lib.rs

@@ -34,7 +34,7 @@ pub mod build;
 mod format;
 pub use format::format;
 
-use crate::build::{BuildFileError, BuildOrdering};
+use crate::build::{standard_load_config, BuildFileError, BuildOrdering};
 
 const DEFAULT_ROC_FILENAME: &str = "main.roc";
 
@@ -672,6 +672,8 @@ pub fn build(
         emit_debug_info,
     };
 
+    let load_config = standard_load_config(&triple, build_ordering, threading);
+
     let res_binary_path = build_file(
         &arena,
         &triple,
@@ -681,10 +683,9 @@ pub fn build(
         link_type,
         linking_strategy,
         prebuilt,
-        threading,
         wasm_dev_stack_bytes,
         roc_cache_dir,
-        build_ordering,
+        load_config,
     );
 
     match res_binary_path {

crates/compiler/load_internal/src/file.rs

@@ -1148,6 +1148,51 @@ impl ModuleTiming {
     }
 }
 
+fn report_timing(
+    buf: &mut impl std::fmt::Write,
+    label: &str,
+    duration: Duration,
+) -> std::fmt::Result {
+    writeln!(
+        buf,
+        "        {:9.3} ms   {}",
+        duration.as_secs_f64() * 1000.0,
+        label,
+    )
+}
+
+impl std::fmt::Display for ModuleTiming {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let module_timing = self;
+
+        report_timing(f, "Read .roc file from disk", module_timing.read_roc_file)?;
+        report_timing(f, "Parse header", module_timing.parse_header)?;
+        report_timing(f, "Parse body", module_timing.parse_body)?;
+        report_timing(f, "Canonicalize", module_timing.canonicalize)?;
+        report_timing(f, "Constrain", module_timing.constrain)?;
+        report_timing(f, "Solve", module_timing.solve)?;
+        report_timing(
+            f,
+            "Find Specializations",
+            module_timing.find_specializations,
+        )?;
+        let multiple_make_specializations_passes = module_timing.make_specializations.len() > 1;
+        for (i, pass_time) in module_timing.make_specializations.iter().enumerate() {
+            let suffix = if multiple_make_specializations_passes {
+                format!(" (Pass {})", i)
+            } else {
+                String::new()
+            };
+            report_timing(f, &format!("Make Specializations{}", suffix), *pass_time)?;
+        }
+        report_timing(f, "Other", module_timing.other())?;
+        f.write_str("\n")?;
+        report_timing(f, "Total", module_timing.total())?;
+
+        Ok(())
+    }
+}
+
 /// A message sent _to_ a worker thread, describing the work to be done
 #[derive(Debug)]
 #[allow(dead_code)]

crates/compiler/roc_target/src/lib.rs

@@ -24,6 +24,22 @@ impl OperatingSystem {
             _ => None,
         }
     }
+
+    pub const fn object_file_ext(&self) -> &str {
+        match self {
+            OperatingSystem::Windows => "obj",
+            OperatingSystem::Unix => "o",
+            OperatingSystem::Wasi => "o",
+        }
+    }
+
+    pub const fn executable_file_ext(&self) -> Option<&str> {
+        match self {
+            OperatingSystem::Windows => Some("exe"),
+            OperatingSystem::Unix => None,
+            OperatingSystem::Wasi => None,
+        }
+    }
 }
 
 impl From<target_lexicon::OperatingSystem> for OperatingSystem {