Skip to content

Commit 29c5ba8

Browse files
committed
dont cache modules with errors
1 parent 37f6eb7 commit 29c5ba8

3 files changed

Lines changed: 64 additions & 19 deletions

File tree

src/build/cache.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,13 @@ impl ModuleCache {
267267
exports_changed
268268
}
269269

270+
/// Remove a module from the cache (e.g. when it fails typechecking).
271+
pub fn remove(&mut self, module_name: &str) {
272+
if self.entries.remove(module_name).is_some() {
273+
self.index_dirty = true;
274+
}
275+
}
276+
270277
/// Build the reverse dependency graph from cached import data.
271278
pub fn build_reverse_deps(&mut self) {
272279
self.dependents.clear();

src/build/mod.rs

Lines changed: 37 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -706,17 +706,26 @@ fn build_from_sources_impl(
706706
" [{}/{}] ok: {} ({:.2?})",
707707
done, total_modules, pm.module_name, elapsed
708708
);
709-
let import_names: Vec<String> = pm.import_parts.iter()
710-
.map(|parts| interner::resolve_module_name(parts))
711-
.collect();
712-
let exports_changed = if let Some(ref mut c) = cache {
713-
c.update(pm.module_name.clone(), pm.source_hash, result.exports.clone(), import_names)
714-
} else {
715-
true
716-
};
717-
// Only add to rebuilt_set if exports actually changed
718-
if exports_changed {
709+
let has_errors = !result.errors.is_empty();
710+
if has_errors {
711+
// Don't cache modules with type errors
712+
if let Some(ref mut c) = cache {
713+
c.remove(&pm.module_name);
714+
}
719715
rebuilt_set.insert(pm.module_name.clone());
716+
} else {
717+
let import_names: Vec<String> = pm.import_parts.iter()
718+
.map(|parts| interner::resolve_module_name(parts))
719+
.collect();
720+
let exports_changed = if let Some(ref mut c) = cache {
721+
c.update(pm.module_name.clone(), pm.source_hash, result.exports.clone(), import_names)
722+
} else {
723+
true
724+
};
725+
// Only add to rebuilt_set if exports actually changed
726+
if exports_changed {
727+
rebuilt_set.insert(pm.module_name.clone());
728+
}
720729
}
721730
// Register exports immediately — result.exports is moved,
722731
// then result (with its types HashMap) is dropped.
@@ -834,16 +843,25 @@ fn build_from_sources_impl(
834843
" [{}/{}] ok: {} ({:.2?})",
835844
done, total_modules, pm.module_name, elapsed
836845
);
837-
let import_names: Vec<String> = pm.import_parts.iter()
838-
.map(|parts| interner::resolve_module_name(parts))
839-
.collect();
840-
let exports_changed = if let Some(ref mut c) = cache {
841-
c.update(pm.module_name.clone(), pm.source_hash, result.exports.clone(), import_names)
842-
} else {
843-
true
844-
};
845-
if exports_changed {
846+
let has_errors = !result.errors.is_empty();
847+
if has_errors {
848+
// Don't cache modules with type errors
849+
if let Some(ref mut c) = cache {
850+
c.remove(&pm.module_name);
851+
}
846852
rebuilt_set.insert(pm.module_name.clone());
853+
} else {
854+
let import_names: Vec<String> = pm.import_parts.iter()
855+
.map(|parts| interner::resolve_module_name(parts))
856+
.collect();
857+
let exports_changed = if let Some(ref mut c) = cache {
858+
c.update(pm.module_name.clone(), pm.source_hash, result.exports.clone(), import_names)
859+
} else {
860+
true
861+
};
862+
if exports_changed {
863+
rebuilt_set.insert(pm.module_name.clone());
864+
}
847865
}
848866
registry.register(&pm.module_parts, result.exports);
849867
module_results.push(ModuleResult {

tests/build.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1320,3 +1320,23 @@ fn incremental_build_disk_roundtrip() {
13201320
// Cleanup
13211321
let _ = std::fs::remove_dir_all(&tmp_dir);
13221322
}
1323+
1324+
#[test]
1325+
fn incremental_build_does_not_cache_errors() {
1326+
let sources: Vec<(&str, &str)> = vec![
1327+
("ModA.purs", "module ModA where\n\nvalA :: Int\nvalA = undefinedVar\n"),
1328+
];
1329+
1330+
let options = BuildOptions::default();
1331+
let mut cache = ModuleCache::new();
1332+
1333+
// First build: should report type error (undefinedVar is not defined)
1334+
let (result1, _, _) = build_from_sources_incremental(&sources, &None, None, &options, &mut cache);
1335+
let has_type_errors_1 = result1.modules.iter().any(|m| !m.type_errors.is_empty());
1336+
assert!(has_type_errors_1, "First build should have type errors");
1337+
1338+
// Second build with same sources: error should NOT be cached away
1339+
let (result2, _, _) = build_from_sources_incremental(&sources, &None, None, &options, &mut cache);
1340+
let has_type_errors_2 = result2.modules.iter().any(|m| !m.type_errors.is_empty());
1341+
assert!(has_type_errors_2, "Second build should still have type errors (not cached)");
1342+
}

0 commit comments

Comments
 (0)