From 3456a447423b193194b4b36b6a07c383bdf9b021 Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Sun, 28 Aug 2022 10:16:55 -0500 Subject: [PATCH 1/2] Only compile and run expects that belong to the same package In particular, don't run expects that come from modules with a different package qualification (including subpackages; we can loosen this restriction later), or builtins when run on userspace apps/interfaces. Closes #3722 --- crates/compiler/load_internal/src/file.rs | 31 +++++++++++++++-------- crates/compiler/module/src/symbol.rs | 14 ++++++++++ 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/crates/compiler/load_internal/src/file.rs b/crates/compiler/load_internal/src/file.rs index b733e0845c..5b6680eed3 100644 --- a/crates/compiler/load_internal/src/file.rs +++ b/crates/compiler/load_internal/src/file.rs @@ -461,6 +461,9 @@ fn start_phase<'a>( let derived_module = SharedDerivedModule::clone(&state.derived_module); + let build_expects = matches!(state.exec_mode, ExecutionMode::Test) + && state.module_cache.expectations.contains_key(&module_id); + BuildTask::BuildPendingSpecializations { layout_cache, execution_mode: state.exec_mode, @@ -475,6 +478,7 @@ fn start_phase<'a>( // TODO: awful, how can we get rid of the clone? exposed_by_module: state.exposed_types.clone(), derived_module, + build_expects, } } Phase::MakeSpecializations => { @@ -1127,6 +1131,7 @@ enum BuildTask<'a> { exposed_by_module: ExposedByModule, abilities_store: AbilitiesStore, derived_module: SharedDerivedModule, + build_expects: bool, }, MakeSpecializations { module_id: ModuleId, @@ -2358,7 +2363,14 @@ fn update<'a>( .type_problems .insert(module_id, solved_module.problems); - if !loc_expects.is_empty() { + let should_include_expects = !loc_expects.is_empty() && { + let modules = state.arc_modules.lock(); + modules + .package_eq(module_id, state.root_id) + .expect("root or this module is not yet known - that's a bug!") + }; + + if should_include_expects { let (path, _) = state.module_cache.sources.get(&module_id).unwrap(); let expectations = Expectations { @@ -4821,6 +4833,7 @@ fn build_pending_specializations<'a>( exposed_by_module: &ExposedByModule, abilities_store: AbilitiesStore, derived_module: SharedDerivedModule, + build_expects: bool, ) -> Msg<'a> { let find_specializations_start = Instant::now(); @@ -5067,11 +5080,8 @@ fn build_pending_specializations<'a>( } Expectation => { // skip expectations if we're not going to run them - match execution_mode { - ExecutionMode::Test => { /* fall through */ } - ExecutionMode::Check - | ExecutionMode::Executable - | ExecutionMode::ExecutableIfCheck => continue, + if !build_expects { + continue; } // mark this symbol as a top-level thunk before any other work on the procs @@ -5143,11 +5153,8 @@ fn build_pending_specializations<'a>( } ExpectationFx => { // skip expectations if we're not going to run them - match execution_mode { - ExecutionMode::Test => { /* fall through */ } - ExecutionMode::Check - | ExecutionMode::Executable - | ExecutionMode::ExecutableIfCheck => continue, + if !build_expects { + continue; } // mark this symbol as a top-level thunk before any other work on the procs @@ -5433,6 +5440,7 @@ fn run_task<'a>( abilities_store, exposed_by_module, derived_module, + build_expects, } => Ok(build_pending_specializations( arena, execution_mode, @@ -5448,6 +5456,7 @@ fn run_task<'a>( &exposed_by_module, abilities_store, derived_module, + build_expects, )), MakeSpecializations { module_id, diff --git a/crates/compiler/module/src/symbol.rs b/crates/compiler/module/src/symbol.rs index fd48b888b2..79cd220c37 100644 --- a/crates/compiler/module/src/symbol.rs +++ b/crates/compiler/module/src/symbol.rs @@ -512,6 +512,20 @@ impl<'a> PackageModuleIds<'a> { pub fn available_modules(&self) -> impl Iterator { self.by_id.iter() } + + /// Returns true iff two modules belong to the same package. + /// Returns [None] if one module is unknown. + pub fn package_eq(&self, left: ModuleId, right: ModuleId) -> Option { + if left.is_builtin() ^ right.is_builtin() { + return Some(false); + } + let result = match (self.get_name(left)?, self.get_name(right)?) { + (PQModuleName::Unqualified(_), PQModuleName::Unqualified(_)) => true, + (PQModuleName::Qualified(pkg1, _), PQModuleName::Qualified(pkg2, _)) => pkg1 == pkg2, + _ => false, + }; + Some(result) + } } /// Stores a mapping between ModuleId and InlinableString. From c6516acdb4b3ecbffd58ccebdaad98c40c350c83 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sun, 28 Aug 2022 16:00:19 -0400 Subject: [PATCH 2/2] Drop unused ExecutionMode --- crates/compiler/load_internal/src/file.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/crates/compiler/load_internal/src/file.rs b/crates/compiler/load_internal/src/file.rs index 5b6680eed3..db5a93daf6 100644 --- a/crates/compiler/load_internal/src/file.rs +++ b/crates/compiler/load_internal/src/file.rs @@ -466,7 +466,6 @@ fn start_phase<'a>( BuildTask::BuildPendingSpecializations { layout_cache, - execution_mode: state.exec_mode, module_id, module_timing, solved_subs, @@ -1120,7 +1119,6 @@ enum BuildTask<'a> { }, BuildPendingSpecializations { module_timing: ModuleTiming, - execution_mode: ExecutionMode, layout_cache: LayoutCache<'a>, solved_subs: Solved, imported_module_thunks: &'a [Symbol], @@ -4820,7 +4818,6 @@ fn make_specializations<'a>( #[allow(clippy::too_many_arguments)] fn build_pending_specializations<'a>( arena: &'a Bump, - execution_mode: ExecutionMode, solved_subs: Solved, imported_module_thunks: &'a [Symbol], home: ModuleId, @@ -5429,7 +5426,6 @@ fn run_task<'a>( )), BuildPendingSpecializations { module_id, - execution_mode, ident_ids, decls, module_timing, @@ -5443,7 +5439,6 @@ fn run_task<'a>( build_expects, } => Ok(build_pending_specializations( arena, - execution_mode, solved_subs, imported_module_thunks, module_id,