diff --git a/src/diagnostics.rs b/src/diagnostics.rs index d93b43665..777cf4fd3 100644 --- a/src/diagnostics.rs +++ b/src/diagnostics.rs @@ -1,7 +1,7 @@ use crate::term::{ check::{ - set_entrypoint::EntryErr, shared_names::TopLevelErr, unbound_pats::UnboundCtrErr, - unbound_vars::UnboundVarErr, + repeated_bind::RepeatedBindWarn, set_entrypoint::EntryErr, shared_names::TopLevelErr, + unbound_pats::UnboundCtrErr, unbound_vars::UnboundVarErr, }, display::DisplayFn, transform::{ @@ -21,20 +21,20 @@ pub const ERR_INDENT_SIZE: usize = 2; #[derive(Debug, Clone, Default)] pub struct Info { err_counter: usize, - errs: Vec, - errs_with_def: BTreeMap>, - pub warns: Vec, + book_errs: Vec, + rule_errs: BTreeMap>, + pub warns: Warnings, } impl Info { pub fn error>(&mut self, e: E) { self.err_counter += 1; - self.errs.push(e.into()) + self.book_errs.push(e.into()) } pub fn def_error>(&mut self, name: Name, e: E) { self.err_counter += 1; - let entry = self.errs_with_def.entry(name).or_default(); + let entry = self.rule_errs.entry(name).or_default(); entry.push(e.into()); } @@ -52,7 +52,7 @@ impl Info { } pub fn has_errors(&self) -> bool { - !(self.errs.is_empty() && self.errs_with_def.is_empty()) + !(self.book_errs.is_empty() && self.rule_errs.is_empty()) } /// Resets the internal counter @@ -67,12 +67,16 @@ impl Info { if self.err_counter == 0 { Ok(t) } else { Err(std::mem::take(self)) } } + pub fn warning>(&mut self, def_name: Name, warning: W) { + self.warns.0.entry(def_name).or_default().push(warning.into()); + } + pub fn display(&self, verbose: bool) -> impl Display + '_ { DisplayFn(move |f| { - writeln!(f, "{}", self.errs.iter().map(|err| err.display(verbose)).join("\n"))?; + writeln!(f, "{}", self.book_errs.iter().map(|err| err.display(verbose)).join("\n"))?; - for (def_name, errs) in &self.errs_with_def { - writeln!(f, "In definition '{def_name}':")?; + for (def_name, errs) in &self.rule_errs { + in_definition(def_name, f)?; for err in errs { writeln!(f, "{:ERR_INDENT_SIZE$}{}", "", err.display(verbose))?; } @@ -83,6 +87,10 @@ impl Info { } } +fn in_definition(def_name: &Name, f: &mut Formatter<'_>) -> Result<(), std::fmt::Error> { + writeln!(f, "In definition '{def_name}':") +} + impl Display for Info { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { write!(f, "{}", self.display(false)) @@ -91,7 +99,7 @@ impl Display for Info { impl From for Info { fn from(value: String) -> Self { - Info { errs: vec![Error::Custom(value)], ..Default::default() } + Info { book_errs: vec![Error::Custom(value)], ..Default::default() } } } @@ -112,6 +120,7 @@ pub enum Error { TopLevel(TopLevelErr), Custom(String), ArgError(ArgError), + RepeatedBind(RepeatedBindWarn), } impl Display for Error { @@ -132,6 +141,7 @@ impl Error { Error::TopLevel(err) => write!(f, "{err}"), Error::Custom(err) => write!(f, "{err}"), Error::ArgError(err) => write!(f, "{err}"), + Error::RepeatedBind(err) => write!(f, "{err}"), }) } } @@ -184,19 +194,41 @@ impl From for Error { } } +#[derive(Debug, Clone, Default)] +pub struct Warnings(pub BTreeMap>); + #[derive(Debug, Clone)] -pub enum Warning { - MatchOnlyVars(Name), - UnusedDefinition(Name), +pub enum WarningType { + MatchOnlyVars, + UnusedDefinition, + RepeatedBind(RepeatedBindWarn), } -impl Display for Warning { +impl Display for WarningType { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { - Warning::MatchOnlyVars(def_name) => { - write!(f, "Match expression at definition '{def_name}' only uses var patterns.") + WarningType::MatchOnlyVars => write!(f, "Match expression at definition only uses var patterns."), + WarningType::UnusedDefinition => write!(f, "Definition is unused."), + WarningType::RepeatedBind(warn) => write!(f, "{warn}"), + } + } +} + +impl Display for Warnings { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + for (def_name, warns) in &self.0 { + in_definition(def_name, f)?; + for warn in warns { + writeln!(f, "{:ERR_INDENT_SIZE$}{}", "", warn)?; } - Warning::UnusedDefinition(def_name) => write!(f, "Unused definition '{def_name}'."), } + + Ok(()) + } +} + +impl From for WarningType { + fn from(value: RepeatedBindWarn) -> Self { + Self::RepeatedBind(value) } } diff --git a/src/lib.rs b/src/lib.rs index 8bb5723c6..650cafed9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,7 +1,7 @@ #![feature(box_patterns)] #![feature(let_chains)] -use diagnostics::{Info, Warning}; +use diagnostics::{Info, WarningType, Warnings}; use hvmc::{ ast::{self, Net}, dispatch_dyn_net, @@ -17,11 +17,8 @@ use std::{ time::Instant, }; use term::{ - book_to_nets, - display::{display_readback_errors, DisplayJoin}, - net_to_term::net_to_term, - term_to_net::Labels, - AdtEncoding, Book, Ctx, ReadbackError, Term, + book_to_nets, display::display_readback_errors, net_to_term::net_to_term, term_to_net::Labels, AdtEncoding, + Book, Ctx, ReadbackError, Term, }; pub mod diagnostics; @@ -107,11 +104,7 @@ pub fn compile_book( Ok(CompileResult { core_book, labels, warns }) } -pub fn desugar_book( - book: &mut Book, - opts: CompileOpts, - args: Option>, -) -> Result, Info> { +pub fn desugar_book(book: &mut Book, opts: CompileOpts, args: Option>) -> Result { let mut ctx = Ctx::new(book); ctx.check_shared_names(); @@ -123,6 +116,7 @@ pub fn desugar_book( ctx.book.resolve_ctrs_in_pats(); ctx.resolve_refs()?; + ctx.check_repeated_binds(); ctx.check_match_arity()?; ctx.check_unbound_pats()?; @@ -194,7 +188,7 @@ pub fn run_book( let book = Arc::new(book); let labels = Arc::new(labels); - display_warnings(&warns, warning_opts)?; + display_warnings(warns, warning_opts)?; // Run let debug_hook = run_opts.debug_hook(&book, &labels); @@ -462,6 +456,7 @@ impl CompileOpts { pub struct WarningOpts { pub match_only_vars: WarnState, pub unused_defs: WarnState, + pub repeated_bind: WarnState, } #[derive(Default, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] @@ -474,65 +469,77 @@ pub enum WarnState { impl WarningOpts { pub fn allow_all() -> Self { - Self { match_only_vars: WarnState::Allow, unused_defs: WarnState::Allow } + Self { match_only_vars: WarnState::Allow, unused_defs: WarnState::Allow, repeated_bind: WarnState::Allow } } pub fn deny_all() -> Self { - Self { match_only_vars: WarnState::Deny, unused_defs: WarnState::Deny } + Self { match_only_vars: WarnState::Deny, unused_defs: WarnState::Deny, repeated_bind: WarnState::Deny } } pub fn warn_all() -> Self { - Self { match_only_vars: WarnState::Warn, unused_defs: WarnState::Warn } + Self { match_only_vars: WarnState::Warn, unused_defs: WarnState::Warn, repeated_bind: WarnState::Warn } } - /// Filters warnings based on the enabled flags. - pub fn filter<'a>(&'a self, warns: &'a [Warning], ws: WarnState) -> Vec<&Warning> { + /// Split warnings into two based on its Warn or Deny WarnState. + pub fn split(&self, warns: Warnings) -> (Warnings, Warnings) { + let mut warn = Warnings::default(); + let mut deny = Warnings::default(); + warns - .iter() - .filter(|w| { - (match w { - Warning::MatchOnlyVars(_) => self.match_only_vars, - Warning::UnusedDefinition(_) => self.unused_defs, - }) == ws - }) - .collect() + .0 + .into_iter() + .flat_map(|(def, warns)| warns.into_iter().map(move |warn| (def.clone(), warn))) + .for_each(|(def, w)| { + let ws = match w { + WarningType::MatchOnlyVars => self.match_only_vars, + WarningType::UnusedDefinition => self.unused_defs, + WarningType::RepeatedBind(_) => self.repeated_bind, + }; + + match ws { + WarnState::Allow => {} + WarnState::Warn => warn.0.entry(def).or_default().push(w), + WarnState::Deny => deny.0.entry(def).or_default().push(w), + }; + }); + + (warn, deny) } } /// Either just prints warnings or returns Err when any denied was produced. -pub fn display_warnings(warnings: &[Warning], warning_opts: WarningOpts) -> Result<(), String> { - let warns = warning_opts.filter(warnings, WarnState::Warn); - if !warns.is_empty() { - eprintln!("Warnings:\n{}", DisplayJoin(|| warns.iter(), "\n")); +pub fn display_warnings(warnings: Warnings, warning_opts: WarningOpts) -> Result<(), String> { + let (warns, denies) = warning_opts.split(warnings); + + if !warns.0.is_empty() { + eprintln!("Warnings:\n{}", warns); } - let denies = warning_opts.filter(warnings, WarnState::Deny); - if !denies.is_empty() { - return Err(format!( - "{}\nCould not run the code because of the previous warnings", - DisplayJoin(|| denies.iter(), "\n") - )); + + if !denies.0.is_empty() { + return Err(format!("{denies}\nCould not run the code because of the previous warnings.")); } + Ok(()) } pub struct CompileResult { - pub warns: Vec, + pub warns: Warnings, pub core_book: hvmc::ast::Book, pub labels: Labels, } impl std::fmt::Debug for CompileResult { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - for warn in &self.warns { - writeln!(f, "// WARNING: {}", warn)?; + if !self.warns.0.is_empty() { + writeln!(f, "// Warnings:\n{}", self.warns)?; } write!(f, "{}", self.core_book) } } impl CompileResult { - pub fn display_with_warns(&self, opts: WarningOpts) -> Result { - display_warnings(&self.warns, opts)?; + pub fn display_with_warns(self, opts: WarningOpts) -> Result { + display_warnings(self.warns, opts)?; Ok(self.core_book.to_string()) } } diff --git a/src/term/check/mod.rs b/src/term/check/mod.rs index 0ea997339..f5913fa44 100644 --- a/src/term/check/mod.rs +++ b/src/term/check/mod.rs @@ -1,5 +1,6 @@ pub mod ctrs_arities; pub mod match_arity; +pub mod repeated_bind; pub mod set_entrypoint; pub mod shared_names; pub mod type_check; diff --git a/src/term/check/repeated_bind.rs b/src/term/check/repeated_bind.rs new file mode 100644 index 000000000..9f23d9af6 --- /dev/null +++ b/src/term/check/repeated_bind.rs @@ -0,0 +1,70 @@ +use crate::term::{Ctx, Name, Term}; +use std::{collections::HashSet, fmt::Display}; + +#[derive(Debug, Clone)] +pub enum RepeatedBindWarn { + Rule(Name), + Match(Name), +} + +impl Display for RepeatedBindWarn { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + RepeatedBindWarn::Rule(bind) => write!(f, "Repeated bind inside rule pattern: '{bind}'."), + RepeatedBindWarn::Match(bind) => write!(f, "Repeated bind inside match arm: '{bind}'."), + } + } +} + +impl Ctx<'_> { + /// Checks that there are no unbound variables in all definitions. + pub fn check_repeated_binds(&mut self) { + for (def_name, def) in &self.book.defs { + for rule in &def.rules { + let mut binds = HashSet::new(); + for pat in &rule.pats { + for nam in pat.named_binds() { + if !binds.insert(nam) { + self.info.warning(def_name.clone(), RepeatedBindWarn::Rule(nam.clone())); + } + } + } + + let mut repeated_in_matches = Vec::new(); + rule.body.check_repeated_binds(&mut repeated_in_matches); + + for repeated in repeated_in_matches { + self.info.warning(def_name.clone(), repeated); + } + } + } + } +} + +impl Term { + pub fn check_repeated_binds(&self, repeated: &mut Vec) { + Term::recursive_call(|| match self { + Term::Mat { args, rules } => { + for arg in args { + arg.check_repeated_binds(repeated); + } + + for rule in rules { + let mut binds = HashSet::new(); + for pat in &rule.pats { + for nam in pat.named_binds() { + if !binds.insert(nam) { + repeated.push(RepeatedBindWarn::Match(nam.clone())); + } + } + } + } + } + _ => { + for child in self.children() { + child.check_repeated_binds(repeated); + } + } + }) + } +} diff --git a/src/term/transform/definition_pruning.rs b/src/term/transform/definition_pruning.rs index 8189315ab..0539db71d 100644 --- a/src/term/transform/definition_pruning.rs +++ b/src/term/transform/definition_pruning.rs @@ -1,7 +1,7 @@ use std::collections::{hash_map::Entry, HashMap}; use crate::{ - diagnostics::Warning, + diagnostics::WarningType, term::{Adt, AdtEncoding, Book, Ctx, Name, Tag, Term, LIST, STRING}, CORE_BUILTINS, }; @@ -72,7 +72,7 @@ impl Ctx<'_> { if prune_all || def.builtin { self.book.defs.swap_remove(&def_name); } else if !def_name.is_generated() { - self.info.warns.push(Warning::UnusedDefinition(def_name.clone())); + self.info.warning(def_name.clone(), WarningType::UnusedDefinition); } } } diff --git a/tests/golden_tests/compile_file/repeated_bind_match.hvm b/tests/golden_tests/compile_file/repeated_bind_match.hvm new file mode 100644 index 000000000..e6e0cb51d --- /dev/null +++ b/tests/golden_tests/compile_file/repeated_bind_match.hvm @@ -0,0 +1,4 @@ +main = @a match a { + (List.cons x x): x + (List.nil) : * +} diff --git a/tests/golden_tests/compile_file/repeated_bind_rule.hvm b/tests/golden_tests/compile_file/repeated_bind_rule.hvm new file mode 100644 index 000000000..06e4d2b25 --- /dev/null +++ b/tests/golden_tests/compile_file/repeated_bind_rule.hvm @@ -0,0 +1,3 @@ +Foo a a = a + +main = (Foo 1 2) \ No newline at end of file diff --git a/tests/snapshots/cli__compile_pre_reduce.hvm.snap b/tests/snapshots/cli__compile_pre_reduce.hvm.snap index e02d96d53..29cdb1d8c 100644 --- a/tests/snapshots/cli__compile_pre_reduce.hvm.snap +++ b/tests/snapshots/cli__compile_pre_reduce.hvm.snap @@ -3,7 +3,9 @@ source: tests/golden_tests.rs input_file: tests/golden_tests/cli/compile_pre_reduce.hvm --- Warnings: -Unused definition 'I'. +In definition 'I': + Definition is unused. + @I = #5 @main = * diff --git a/tests/snapshots/compile_file__crlf.hvm.snap b/tests/snapshots/compile_file__crlf.hvm.snap index 3d8afbc21..3609f7dcd 100644 --- a/tests/snapshots/compile_file__crlf.hvm.snap +++ b/tests/snapshots/compile_file__crlf.hvm.snap @@ -2,6 +2,9 @@ source: tests/golden_tests.rs input_file: tests/golden_tests/compile_file/crlf.hvm --- -// WARNING: Unused definition 'a'. +// Warnings: +In definition 'a': + Definition is unused. + @a = #1 @main = #2 diff --git a/tests/snapshots/compile_file__match_num_all_patterns.hvm.snap b/tests/snapshots/compile_file__match_num_all_patterns.hvm.snap index 9dbbbf592..939d809bb 100644 --- a/tests/snapshots/compile_file__match_num_all_patterns.hvm.snap +++ b/tests/snapshots/compile_file__match_num_all_patterns.hvm.snap @@ -2,17 +2,30 @@ source: tests/golden_tests.rs input_file: tests/golden_tests/compile_file/match_num_all_patterns.hvm --- -// WARNING: Unused definition 'zero_succ'. -// WARNING: Unused definition 'succ_zero'. -// WARNING: Unused definition 'var_zero'. -// WARNING: Unused definition 'var_succ'. -// WARNING: Unused definition 'zero_var'. -// WARNING: Unused definition 'succ_var'. -// WARNING: Unused definition 'zero_var_succ'. -// WARNING: Unused definition 'succ_var_zero'. -// WARNING: Unused definition 'zero_succ_var'. -// WARNING: Unused definition 'succ_zero_var'. -// WARNING: Unused definition 'succ_zero_succ'. +// Warnings: +In definition 'succ_var': + Definition is unused. +In definition 'succ_var_zero': + Definition is unused. +In definition 'succ_zero': + Definition is unused. +In definition 'succ_zero_succ': + Definition is unused. +In definition 'succ_zero_var': + Definition is unused. +In definition 'var_succ': + Definition is unused. +In definition 'var_zero': + Definition is unused. +In definition 'zero_succ': + Definition is unused. +In definition 'zero_succ_var': + Definition is unused. +In definition 'zero_var': + Definition is unused. +In definition 'zero_var_succ': + Definition is unused. + @main = #0 @succ_var = (?<(#0 (a a)) b> b) @succ_var_zero = (?<(a @succ_var_zero$S0) b> b) diff --git a/tests/snapshots/compile_file__match_num_unscoped_lambda.hvm.snap b/tests/snapshots/compile_file__match_num_unscoped_lambda.hvm.snap index 0e5525d6a..1938ef3e6 100644 --- a/tests/snapshots/compile_file__match_num_unscoped_lambda.hvm.snap +++ b/tests/snapshots/compile_file__match_num_unscoped_lambda.hvm.snap @@ -2,9 +2,12 @@ source: tests/golden_tests.rs input_file: tests/golden_tests/compile_file/match_num_unscoped_lambda.hvm --- -// WARNING: Unused definition 'lambda_out'. -// WARNING: Unused definition 'lambda_in'. +// Warnings: +In definition 'lambda_in': + Definition is unused. +In definition 'lambda_out': + Definition is unused. + @lambda_in = (?<((a a) (b (c b))) (c d)> d) @lambda_out = (?<(a (b b)) c> (a c)) @main = * - diff --git a/tests/snapshots/compile_file__repeated_bind_match.hvm.snap b/tests/snapshots/compile_file__repeated_bind_match.hvm.snap new file mode 100644 index 000000000..95c3e681a --- /dev/null +++ b/tests/snapshots/compile_file__repeated_bind_match.hvm.snap @@ -0,0 +1,11 @@ +--- +source: tests/golden_tests.rs +input_file: tests/golden_tests/compile_file/repeated_bind_match.hvm +--- +// Warnings: +In definition 'main': + Repeated bind inside match arm: 'x'. + +@List.cons = (a (b {2 {2 a {2 b c}} {2 * c}})) +@List.nil = {2 * {2 a a}} +@main = ({2 {2 a {2 * a}} {2 * b}} b) diff --git a/tests/snapshots/compile_file__repeated_bind_rule.hvm.snap b/tests/snapshots/compile_file__repeated_bind_rule.hvm.snap new file mode 100644 index 000000000..8c4c83392 --- /dev/null +++ b/tests/snapshots/compile_file__repeated_bind_rule.hvm.snap @@ -0,0 +1,11 @@ +--- +source: tests/golden_tests.rs +input_file: tests/golden_tests/compile_file/repeated_bind_rule.hvm +--- +// Warnings: +In definition 'Foo': + Repeated bind inside rule pattern: 'a'. + +@Foo = (a (* a)) +@main = a +& @Foo ~ (#1 (#2 a))