From b38204eab50e596414e0e5a751f9c78c629fbde1 Mon Sep 17 00:00:00 2001 From: Kyryl R Date: Fri, 4 Sep 2026 16:17:58 +0300 Subject: [PATCH 1/2] errors: dedup same compiler errors, present them in deterministic order --- src/driver/mod.rs | 3 + src/driver/resolve_order.rs | 39 ++++++- src/error.rs | 205 +++++++++++++++++++++++++++++++++++- 3 files changed, 243 insertions(+), 4 deletions(-) diff --git a/src/driver/mod.rs b/src/driver/mod.rs index 50a94d48..ee9dd091 100644 --- a/src/driver/mod.rs +++ b/src/driver/mod.rs @@ -252,6 +252,9 @@ impl DependencyGraph { graph.discover_dependencies(&mut diagnostics, unstable_features); if diagnostics.has_errors() { + if let Ok(order) = graph.linearize() { + diagnostics.with_source_order(&order); + } diagnostics.with_sources(graph.sources); (None, diagnostics) } else { diff --git a/src/driver/resolve_order.rs b/src/driver/resolve_order.rs index 31228798..04fbd39d 100644 --- a/src/driver/resolve_order.rs +++ b/src/driver/resolve_order.rs @@ -63,7 +63,10 @@ impl DependencyGraph { diagnostics: &mut DiagnosticManager, ) -> Option { match self.linearize() { - Ok(order) => self.assemble_program(&order, diagnostics), + Ok(order) => { + diagnostics.with_source_order(&order); + self.assemble_program(&order, diagnostics) + } Err(err) => { diagnostics.push(err); None @@ -199,6 +202,7 @@ impl DependencyGraph { mod flattening_tests { use crate::driver::tests::setup_graph; use crate::driver::CRATE_STR; + use crate::error::{Diagnostic, Error, Location, Span}; use crate::parse::{self, Visibility}; use std::collections::HashMap; @@ -327,6 +331,39 @@ mod flattening_tests { "a dependency `fn main` must not satisfy a missing entrypoint `fn main`" ); } + + #[test] + fn driver_supplies_dependency_order_for_diagnostic_presentation() { + let (graph, ids, _dir, mut diagnostics) = setup_graph(vec![ + ("libs/lib/A.simf", "pub fn helper() {}"), + ("main.simf", "use lib::A::helper; fn main() {}"), + ]); + let main_id = ids["main"]; + let dependency_id = ids["A"]; + + diagnostics.push(Diagnostic::new( + Error::CannotParse { + msg: "entry".to_owned(), + }, + Span::new(main_id, 0..1), + )); + diagnostics.push(Diagnostic::new( + Error::CannotParse { + msg: "dependency".to_owned(), + }, + Span::new(dependency_id, 0..1), + )); + + let _ = graph.linearize_and_assemble(&mut diagnostics); + assert!(matches!( + diagnostics.diagnostics()[0].location(), + Location::Code(span) if span.file_id == main_id + )); + assert!(matches!( + diagnostics.presentation_order()[0].location(), + Location::Code(span) if span.file_id == dependency_id + )); + } } #[cfg(test)] diff --git a/src/error.rs b/src/error.rs index b5b088ff..0f2f49d1 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,5 +1,5 @@ use std::collections::hash_map::Entry; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::ffi::OsStr; use std::fmt; use std::io::{self, IsTerminal, Write}; @@ -397,9 +397,16 @@ where pub struct DiagnosticManager { diags: Vec, error_count: usize, + index: Box, sources: Option, } +#[derive(Debug, Clone, Default)] +struct DiagnosticIndex { + identities: HashSet, + source_order: HashMap, +} + impl DiagnosticManager { pub fn new() -> Self { Self::default() @@ -409,8 +416,23 @@ impl DiagnosticManager { self.sources = Some(sources); } + pub(crate) fn with_source_order(&mut self, order: &[usize]) { + self.index.source_order = order + .iter() + .copied() + .enumerate() + .map(|(rank, file_id)| (file_id, rank)) + .collect(); + } + /// Extend existing errors with specific `Diagnostic`. pub fn push(&mut self, diag: Diagnostic) { + // Debug is used because `Error` contains foreign error types that do not implement `Eq`/`Hash`. + // `Display` is not used because it coalesces some variants. + if !self.index.identities.insert(format!("{diag:?}")) { + return; + } + if matches!(diag.severity, Severity::Error) { self.error_count += 1; } @@ -434,10 +456,59 @@ impl DiagnosticManager { self.error_count } + /// Diagnostics in the order they were first accepted. pub fn diagnostics(&self) -> &[Diagnostic] { &self.diags } + /// Diagnostics in deterministic dependency-and-source presentation order. + /// + /// The order is influenced by the linearized graph, where `file_rank` and + /// `location_rank` have the following meanings: + /// + /// | Rank | Value | Meaning | + /// |---|---|---| + /// | `file_rank` | `(0, file_id)` | Unranked file | + /// | `file_rank` | `(1, graph_rank)` | Dependency-graph file | + /// | `file_rank` | `(2, 0)` | Global diagnostic | + /// | `location_rank` | `0` | Whole-file diagnostic | + /// | `location_rank` | `1` | Code-span diagnostic | + /// | `location_rank` | `2` | Global diagnostic | + /// + /// Span (start and end variables), severity rank (see [`Severity`]), and `insertion_index` are self-documented. + pub fn presentation_order(&self) -> Vec<&Diagnostic> { + let mut indexed: Vec<_> = self.diags.iter().enumerate().collect(); + indexed.sort_by_key(|(insertion_index, diag)| { + let (file_rank, location_rank, start, end) = match diag.location { + Location::Code(span) => (self.source_rank(span.file_id), 1, span.start, span.end), + Location::File(file_id) => (self.source_rank(file_id), 0, 0, 0), + Location::Global => ((2, 0), 2, usize::MAX, usize::MAX), + }; + let severity_rank = match diag.severity { + Severity::Error => 0, + Severity::Warning => 1, + }; + + ( + file_rank, + location_rank, + start, + end, + severity_rank, + *insertion_index, + ) + }); + indexed.into_iter().map(|(_, diag)| diag).collect() + } + + fn source_rank(&self, file_id: usize) -> (u8, usize) { + self.index + .source_order + .get(&file_id) + .copied() + .map_or((0, file_id), |rank| (1, rank)) + } + pub fn sources(&self) -> Option<&SourceMap> { self.sources.as_ref() } @@ -462,7 +533,7 @@ impl DiagnosticManager { let mut cache = RenderCache::new(sources); - for diag in &self.diags { + for diag in self.presentation_order() { render_one(diag, &mut cache, with_color, &mut w)?; } @@ -483,7 +554,7 @@ impl fmt::Display for DiagnosticManager { // with span highlighting must call `render` or `render_to_string` // explicitly, because `fmt::Formatter` can't carry the color flag // or surface I/O errors. - for diag in &self.diags { + for diag in self.presentation_order() { writeln!(f, "{diag}")?; } Ok(()) @@ -1228,6 +1299,134 @@ mod tests { let diag = result.with_span(Span::new(0, 5..10)).unwrap_err(); assert!(matches!(diag.location(), Location::Code(s) if s.start == 5 && s.end == 10)); } + + #[test] + fn exact_identity_deduplicates_diagnostics() { + let mut manager = DiagnosticManager::new(); + let span = Span::new(0, 4..8); + let duplicate = Diagnostic::new( + Error::CannotParse { + msg: "same".to_owned(), + }, + span, + ) + .with_secondary(Span::new(0, 0..2), "origin") + .with_note("note") + .with_help("help"); + + manager.push(duplicate.clone()); + manager.push(duplicate); + assert_eq!(manager.error_count(), 1); + + // The same text at a different primary span is a distinct structured identity. + manager.push(Diagnostic::new( + Error::CannotParse { + msg: "same".to_owned(), + }, + Span::new(0, 9..13), + )); + + assert_eq!(manager.error_count(), 2); + + manager.push(Diagnostic::new( + Error::CannotParse { + msg: "also collected".to_owned(), + }, + Span::new(0, 14..18), + )); + + assert_eq!(manager.error_count(), 3); + assert_eq!(manager.diagnostics().len(), 3); + } + + #[test] + fn matching_rendered_text_does_not_deduplicate_structured_errors() { + let mut manager = DiagnosticManager::new(); + let invalid_digit = "x".parse::().unwrap_err(); + let positive_overflow = "999".parse::().unwrap_err(); + + manager.push(Diagnostic::new( + Error::ParseInt { + source: invalid_digit, + }, + Span::new(0, 0..1), + )); + manager.push(Diagnostic::new( + Error::ParseInt { + source: positive_overflow, + }, + Span::new(0, 0..1), + )); + + assert_eq!(manager.error_count(), 2); + assert_eq!( + manager.diagnostics()[0].to_string(), + "Integer parsing error" + ); + assert_eq!( + manager.diagnostics()[1].to_string(), + "Integer parsing error" + ); + } + + #[test] + fn labels_notes_and_help_participate_in_identity() { + let mut manager = DiagnosticManager::new(); + let base = || Diagnostic::new(Error::MainRequired, Span::new(0, 0..1)); + + manager.push(base().with_note("first note")); + manager.push(base().with_note("second note")); + manager.push(base().with_secondary(Span::new(0, 2..3), "secondary")); + manager.push(base().with_help("help")); + manager.push(Diagnostic::warning(Error::MainRequired, Span::new(0, 0..1))); + + assert_eq!(manager.error_count(), 4); + assert_eq!(manager.diagnostics().len(), 5); + } + + #[test] + fn insertion_and_presentation_orders_are_explicit_and_distinct() { + let mut manager = DiagnosticManager::new(); + manager.with_source_order(&[2, 1, 0]); + + manager.push(Diagnostic::global(Error::MainRequired)); + manager.push(Diagnostic::warning( + Error::MainNoInputs, + Span::new(1, 10..12), + )); + manager.push(Diagnostic::new(Error::MainNoOutput, Span::new(2, 20..21))); + manager.push(Diagnostic::new(Error::MainRequired, Span::new(1, 10..12))); + + assert!(matches!( + manager.diagnostics()[0].location(), + Location::Global + )); + + let presented = manager.presentation_order(); + assert!(matches!(presented[0].location(), Location::Code(span) if span.file_id == 2)); + assert!(matches!(presented[1].error(), Error::MainRequired)); + assert!(matches!(presented[2].error(), Error::MainNoInputs)); + assert!(matches!(presented[3].location(), Location::Global)); + } + + #[test] + fn unranked_sources_precede_ranked_sources_without_key_collisions() { + let mut manager = DiagnosticManager::new(); + manager.with_source_order(&[42]); + + manager.push(Diagnostic::new(Error::MainRequired, Span::new(0, 0..1))); + manager.push(Diagnostic::new(Error::MainNoOutput, Span::new(42, 0..1))); + + let presented = manager.presentation_order(); + assert!(matches!( + presented[0].location(), + Location::Code(span) if span.file_id == 0 + )); + assert!(matches!( + presented[1].location(), + Location::Code(span) if span.file_id == 42 + )); + } } #[cfg(test)] From 5c8801015e40207cf08dbe1ff267e2db6edc6c08 Mon Sep 17 00:00:00 2001 From: Kyryl R Date: Fri, 4 Sep 2026 16:20:12 +0300 Subject: [PATCH 2/2] general: update CHANGELOG.md --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 319e3ab8..2fc06102 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +# Unreleased + +## Changed + +* Deduplicate identical compiler diagnostics and present multi-file diagnostics in deterministic dependency and source order. [#413](https://github.com/BlockstreamResearch/SimplicityHL/pull/413) + # 0.7.2 - 2026-08-25 ## Added