Skip to content

Better errors and caching #12

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
roryc89 merged 7 commits into from
Mar 11, 2026
Merged

Better errors and caching #12

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c61064c
better error display
roryc89 Mar 11, 2026
37f6eb7
location on other line
roryc89 Mar 11, 2026
29c5ba8
dont cache modules with errors
roryc89 Mar 11, 2026
c9817d4
prettier errors
roryc89 Mar 11, 2026
bcdc25a
adds newline to lsp diagnostics before code
roryc89 Mar 11, 2026
b7d6244
typecheck on open/init
roryc89 Mar 11, 2026
9f8ea8a
more agressive caching via build plan
roryc89 Mar 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
485 changes: 481 additions & 4 deletions src/build/cache.rs
View file
Open in desktop

Large diffs are not rendered by default.

134 changes: 112 additions & 22 deletions src/build/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,7 @@ fn build_from_sources_impl(
effective_timeout.map(|t| t.as_secs()).unwrap_or(0));

let mut done = 0usize;
let mut rebuilt_set: HashSet<String> = HashSet::new();
let mut export_diffs: HashMap<String, cache::ExportDiff> = HashMap::new();
let mut cached_count = 0usize;

for level in &levels {
Expand All @@ -635,7 +635,7 @@ fn build_from_sources_impl(
{
let pm = &parsed[idx];
if let Some(ref mut cache) = cache {
if !cache.needs_rebuild(&pm.module_name, pm.source_hash, &rebuilt_set) {
if !cache.needs_rebuild_smart(&pm.module_name, pm.source_hash, &export_diffs) {
if let Some(exports) = cache.get_exports(&pm.module_name) {
done += 1;
cached_count += 1;
Expand All @@ -647,6 +647,7 @@ fn build_from_sources_impl(
module_results.push(ModuleResult {
path: pm.path.clone(),
module_name: pm.module_name.clone(),

type_errors: vec![],
cached: true,
});
Expand Down Expand Up @@ -705,24 +706,71 @@ fn build_from_sources_impl(
" [{}/{}] ok: {} ({:.2?})",
done, total_modules, pm.module_name, elapsed
);
let import_names: Vec<String> = pm.import_parts.iter()
.map(|parts| interner::resolve_module_name(parts))
.collect();
let exports_changed = if let Some(ref mut c) = cache {
c.update(pm.module_name.clone(), pm.source_hash, result.exports.clone(), import_names)
let has_errors = !result.errors.is_empty();
if has_errors {
// Don't cache modules with type errors
// Compute a full diff so downstream modules rebuild
let old_exports = cache.as_mut().and_then(|c| c.get_exports(&pm.module_name).cloned());
if let Some(ref mut c) = cache {
c.remove(&pm.module_name);
}
// Treat error modules as having all exports changed
let diff = if let Some(old) = old_exports {
cache::ExportDiff::compute(&old, &result.exports)
} else {
// New module with errors — force downstream rebuild
let mut d = cache::ExportDiff::default();
d.instances_changed = true;
d
};
if !diff.is_empty() {
log::debug!(
"[build-plan] {} exports changed: values={:?}, types={:?}, classes={:?}, instances={}, operators={}",
pm.module_name, diff.changed_values, diff.changed_types, diff.changed_classes,
diff.instances_changed, diff.operators_changed
);
export_diffs.insert(pm.module_name.clone(), diff);
}
} else {
true
};
// Only add to rebuilt_set if exports actually changed
if exports_changed {
rebuilt_set.insert(pm.module_name.clone());
let import_names: Vec<String> = pm.import_parts.iter()
.map(|parts| interner::resolve_module_name(parts))
.collect();
let module_ref = pm.module.as_ref().unwrap();
let import_items = cache::extract_import_items(&module_ref.imports);
// Get old exports before updating for diff computation
let old_exports = cache.as_mut().and_then(|c| c.get_exports(&pm.module_name).cloned());
let exports_changed = if let Some(ref mut c) = cache {
c.update(pm.module_name.clone(), pm.source_hash, result.exports.clone(), import_names, import_items)
} else {
true
};
// Compute per-symbol diff for smart rebuild
if exports_changed {
let diff = if let Some(old) = old_exports {
cache::ExportDiff::compute(&old, &result.exports)
} else {
// New module — force downstream rebuild
let mut d = cache::ExportDiff::default();
d.instances_changed = true;
d
};
if !diff.is_empty() {
log::debug!(
"[build-plan] {} exports changed: values={:?}, types={:?}, classes={:?}, instances={}, operators={}",
pm.module_name, diff.changed_values, diff.changed_types, diff.changed_classes,
diff.instances_changed, diff.operators_changed
);
export_diffs.insert(pm.module_name.clone(), diff);
}
}
}
// Register exports immediately — result.exports is moved,
// then result (with its types HashMap) is dropped.
registry.register(&pm.module_parts, result.exports);
module_results.push(ModuleResult {
path: pm.path.clone(),
module_name: pm.module_name.clone(),

type_errors: result.errors,
cached: false,
});
Expand All @@ -746,7 +794,7 @@ fn build_from_sources_impl(
for &idx in level.iter() {
let pm = &parsed[idx];
if let Some(ref mut cache) = cache {
if !cache.needs_rebuild(&pm.module_name, pm.source_hash, &rebuilt_set) {
if !cache.needs_rebuild_smart(&pm.module_name, pm.source_hash, &export_diffs) {
if let Some(exports) = cache.get_exports(&pm.module_name) {
done += 1;
cached_count += 1;
Expand Down Expand Up @@ -832,21 +880,63 @@ fn build_from_sources_impl(
" [{}/{}] ok: {} ({:.2?})",
done, total_modules, pm.module_name, elapsed
);
let import_names: Vec<String> = pm.import_parts.iter()
.map(|parts| interner::resolve_module_name(parts))
.collect();
let exports_changed = if let Some(ref mut c) = cache {
c.update(pm.module_name.clone(), pm.source_hash, result.exports.clone(), import_names)
let has_errors = !result.errors.is_empty();
if has_errors {
// Don't cache modules with type errors
let old_exports = cache.as_mut().and_then(|c| c.get_exports(&pm.module_name).cloned());
if let Some(ref mut c) = cache {
c.remove(&pm.module_name);
}
let diff = if let Some(old) = old_exports {
cache::ExportDiff::compute(&old, &result.exports)
} else {
let mut d = cache::ExportDiff::default();
d.instances_changed = true;
d
};
if !diff.is_empty() {
log::debug!(
"[build-plan] {} exports changed: values={:?}, types={:?}, classes={:?}, instances={}, operators={}",
pm.module_name, diff.changed_values, diff.changed_types, diff.changed_classes,
diff.instances_changed, diff.operators_changed
);
export_diffs.insert(pm.module_name.clone(), diff);
}
} else {
true
};
if exports_changed {
rebuilt_set.insert(pm.module_name.clone());
let import_names: Vec<String> = pm.import_parts.iter()
.map(|parts| interner::resolve_module_name(parts))
.collect();
let module_ref = pm.module.as_ref().unwrap();
let import_items = cache::extract_import_items(&module_ref.imports);
let old_exports = cache.as_mut().and_then(|c| c.get_exports(&pm.module_name).cloned());
let exports_changed = if let Some(ref mut c) = cache {
c.update(pm.module_name.clone(), pm.source_hash, result.exports.clone(), import_names, import_items)
} else {
true
};
if exports_changed {
let diff = if let Some(old) = old_exports {
cache::ExportDiff::compute(&old, &result.exports)
} else {
let mut d = cache::ExportDiff::default();
d.instances_changed = true;
d
};
if !diff.is_empty() {
log::debug!(
"[build-plan] {} exports changed: values={:?}, types={:?}, classes={:?}, instances={}, operators={}",
pm.module_name, diff.changed_values, diff.changed_types, diff.changed_classes,
diff.instances_changed, diff.operators_changed
);
export_diffs.insert(pm.module_name.clone(), diff);
}
}
}
registry.register(&pm.module_parts, result.exports);
module_results.push(ModuleResult {
path: pm.path.clone(),
module_name: pm.module_name.clone(),

type_errors: result.errors,
cached: false,
});
Expand Down
60 changes: 30 additions & 30 deletions src/build/portable.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
//! Uses a deduplicated string table so each symbol is stored once.
//! Symbol references are u32 indices into the string table.

use std::collections::{HashMap, HashSet};
use std::collections::{BTreeMap, BTreeSet, HashMap};

use serde::{Deserialize, Serialize};

Expand Down Expand Up @@ -64,7 +64,7 @@ impl StringTableReader {

// ===== Portable QualifiedIdent =====

#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, Hash)]
#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)]
pub struct PQI {
pub module: Option<u32>,
pub name: u32,
Expand Down Expand Up @@ -227,34 +227,34 @@ fn rest_role(p: &PRole) -> Role {

#[derive(Serialize, Deserialize, Clone, Debug)]
pub struct PModuleExports {
pub values: HashMap<PQI, PScheme>,
pub class_methods: HashMap<PQI, (PQI, Vec<PQI>)>,
pub data_constructors: HashMap<PQI, Vec<PQI>>,
pub ctor_details: HashMap<PQI, (PQI, Vec<PQI>, Vec<PType>)>,
pub instances: HashMap<PQI, Vec<(Vec<PType>, Vec<(PQI, Vec<PType>)>)>>,
pub type_operators: HashMap<PQI, PQI>,
pub value_fixities: HashMap<PQI, (PAssociativity, u8)>,
pub type_fixities: HashMap<PQI, (PAssociativity, u8)>,
pub function_op_aliases: HashSet<PQI>,
pub value_operator_targets: HashMap<PQI, PQI>,
pub constrained_class_methods: HashSet<PQI>,
pub type_aliases: HashMap<PQI, (Vec<PQI>, PType)>,
pub class_param_counts: HashMap<PQI, usize>,
pub value_origins: HashMap<u32, u32>,
pub type_origins: HashMap<u32, u32>,
pub class_origins: HashMap<u32, u32>,
pub operator_class_targets: HashMap<u32, u32>,
pub class_fundeps: HashMap<u32, (Vec<u32>, Vec<(Vec<usize>, Vec<usize>)>)>,
pub type_con_arities: HashMap<PQI, usize>,
pub type_roles: HashMap<u32, Vec<PRole>>,
pub newtype_names: HashSet<u32>,
pub signature_constraints: HashMap<PQI, Vec<(PQI, Vec<PType>)>>,
pub type_kinds: HashMap<u32, PType>,
pub class_type_kinds: HashMap<u32, PType>,
pub partial_dischargers: HashSet<u32>,
pub self_referential_aliases: HashSet<u32>,
pub class_superclasses: HashMap<PQI, (Vec<u32>, Vec<(PQI, Vec<PType>)>)>,
pub method_own_constraints: HashMap<PQI, Vec<u32>>,
pub values: BTreeMap<PQI, PScheme>,
pub class_methods: BTreeMap<PQI, (PQI, Vec<PQI>)>,
pub data_constructors: BTreeMap<PQI, Vec<PQI>>,
pub ctor_details: BTreeMap<PQI, (PQI, Vec<PQI>, Vec<PType>)>,
pub instances: BTreeMap<PQI, Vec<(Vec<PType>, Vec<(PQI, Vec<PType>)>)>>,
pub type_operators: BTreeMap<PQI, PQI>,
pub value_fixities: BTreeMap<PQI, (PAssociativity, u8)>,
pub type_fixities: BTreeMap<PQI, (PAssociativity, u8)>,
pub function_op_aliases: BTreeSet<PQI>,
pub value_operator_targets: BTreeMap<PQI, PQI>,
pub constrained_class_methods: BTreeSet<PQI>,
pub type_aliases: BTreeMap<PQI, (Vec<PQI>, PType)>,
pub class_param_counts: BTreeMap<PQI, usize>,
pub value_origins: BTreeMap<u32, u32>,
pub type_origins: BTreeMap<u32, u32>,
pub class_origins: BTreeMap<u32, u32>,
pub operator_class_targets: BTreeMap<u32, u32>,
pub class_fundeps: BTreeMap<u32, (Vec<u32>, Vec<(Vec<usize>, Vec<usize>)>)>,
pub type_con_arities: BTreeMap<PQI, usize>,
pub type_roles: BTreeMap<u32, Vec<PRole>>,
pub newtype_names: BTreeSet<u32>,
pub signature_constraints: BTreeMap<PQI, Vec<(PQI, Vec<PType>)>>,
pub type_kinds: BTreeMap<u32, PType>,
pub class_type_kinds: BTreeMap<u32, PType>,
pub partial_dischargers: BTreeSet<u32>,
pub self_referential_aliases: BTreeSet<u32>,
pub class_superclasses: BTreeMap<PQI, (Vec<u32>, Vec<(PQI, Vec<PType>)>)>,
pub method_own_constraints: BTreeMap<PQI, Vec<u32>>,
}

impl PModuleExports {
Expand Down
29 changes: 24 additions & 5 deletions src/lsp/handlers/diagnostics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use tower_lsp::lsp_types::*;

use crate::cst::Module;
use crate::interner;
use crate::build::cache::ModuleCache;
use crate::build::cache::{ModuleCache, extract_import_items};
use crate::typechecker::registry::ModuleRegistry;

use super::super::{Backend, FileState};
Expand Down Expand Up @@ -112,8 +112,9 @@ impl Backend {
let import_names: Vec<String> = module.imports.iter()
.map(|imp| interner::resolve_module_name(&imp.module.parts))
.collect();
let import_items = extract_import_items(&module.imports);
let mut cache = self.module_cache.write().await;
cache.update(module_name.clone(), source_hash, check_result.exports, import_names);
cache.update(module_name.clone(), source_hash, check_result.exports, import_names, import_items);
drop(cache);

// Publish diagnostics for the changed module
Expand All @@ -124,9 +125,27 @@ impl Backend {

self.info(format!("[on_change] total: {:.2?}", on_change_start.elapsed())).await;
}

/// Re-typecheck all files that are currently open in the editor.
/// Called after initialization completes to process files opened during loading.
pub(crate) async fn typecheck_open_files(&self) {
let open_files: Vec<(String, String)> = {
let files = self.files.read().await;
files.iter().map(|(uri, fs)| (uri.clone(), fs.source.clone())).collect()
};
if open_files.is_empty() {
return;
}
self.info(format!("[lsp] typechecking {} open file(s) after init", open_files.len())).await;
for (uri_str, source) in open_files {
if let Ok(uri) = Url::parse(&uri_str) {
self.on_change(uri, source).await;
}
}
}
}

fn type_errors_to_diagnostics(errors: &[crate::typechecker::error::TypeError], source: &str) -> Vec<Diagnostic> {
pub(crate) fn type_errors_to_diagnostics(errors: &[crate::typechecker::error::TypeError], source: &str) -> Vec<Diagnostic> {
errors
.iter()
.map(|err| {
Expand All @@ -149,14 +168,14 @@ fn type_errors_to_diagnostics(errors: &[crate::typechecker::error::TypeError], s
severity: Some(DiagnosticSeverity::ERROR),
code: Some(NumberOrString::String(format!("TypeError.{}", err.code()))),
source: Some("pfc".to_string()),
message: format!("{err}"),
message: format!("{}\n", err.format_pretty()),
..Default::default()
}
})
.collect()
}

fn error_to_range(err: &crate::diagnostics::CompilerError, source: &str) -> Range {
pub(crate) fn error_to_range(err: &crate::diagnostics::CompilerError, source: &str) -> Range {
match err.get_span() {
Some(span) => match span.to_pos(source) {
Some((start, end)) => Range {
Expand Down
Loading
Loading