From 0d2d2bb01ee32b3d98eb26e20c1cf3c008934d05 Mon Sep 17 00:00:00 2001 From: imaqtkatt Date: Fri, 7 Jun 2024 17:25:23 -0300 Subject: [PATCH 1/2] Improve function redefinition error message --- src/fun/parser.rs | 12 ++++++++++-- src/imp/parser.rs | 4 ++-- .../parse_file/redefinition_builtin.bend | 5 +++++ .../parse_file__redefinition_builtin.bend.snap | 11 +++++++++++ .../parse_file__redefinition_imp_fun.bend.snap | 2 +- ...rse_file__redefinition_with_def_between.bend.snap | 2 +- ..._file__redefinition_with_object_between.bend.snap | 2 +- ...se_file__redefinition_with_type_between.bend.snap | 2 +- 8 files changed, 32 insertions(+), 8 deletions(-) create mode 100644 tests/golden_tests/parse_file/redefinition_builtin.bend create mode 100644 tests/snapshots/parse_file__redefinition_builtin.bend.snap diff --git a/src/fun/parser.rs b/src/fun/parser.rs index 8624d6431..0ba98453d 100644 --- a/src/fun/parser.rs +++ b/src/fun/parser.rs @@ -133,12 +133,12 @@ impl<'a> TermParser<'a> { def.rules.push(rule); } else { // Trying to add a new rule to a previous definition, coming from a different rule. - let msg = format!("Redefinition of function '{name}'"); + let msg = self.redefinition_of_function_msg(builtin, &name); return self.with_ctx(Err(msg), ini_idx, end_idx); } } else { // Trying to add a new rule to a previous definition, coming from another kind of top-level. - let msg = format!("Redefinition of function '{name}'"); + let msg = self.redefinition_of_function_msg(builtin, &name); return self.with_ctx(Err(msg), ini_idx, end_idx); } } else { @@ -1234,4 +1234,12 @@ pub trait ParserCommons<'a>: Parser<'a> { self.consume_exactly("`")?; Ok(result) } + + fn redefinition_of_function_msg(&self, builtin: bool, function_name: &str) -> String { + if builtin { + format!("Redefinition of builtin (function) '{function_name}'.") + } else { + format!("Redefinition of function '{function_name}'.") + } + } } diff --git a/src/imp/parser.rs b/src/imp/parser.rs index 45ee1a7a0..7fb9e3a38 100644 --- a/src/imp/parser.rs +++ b/src/imp/parser.rs @@ -1069,8 +1069,8 @@ impl<'a> PyParser<'a> { end_idx: usize, builtin: bool, ) -> ParseResult<()> { - if book.defs.contains_key(&def.name) { - let msg = format!("Redefinition of function '{}'.", def.name); + if let Some(def) = book.defs.get(&def.name) { + let msg = self.redefinition_of_function_msg(def.builtin, &def.name); return self.with_ctx(Err(msg), ini_idx, end_idx); } if book.ctrs.contains_key(&def.name) { diff --git a/tests/golden_tests/parse_file/redefinition_builtin.bend b/tests/golden_tests/parse_file/redefinition_builtin.bend new file mode 100644 index 000000000..5dae112fc --- /dev/null +++ b/tests/golden_tests/parse_file/redefinition_builtin.bend @@ -0,0 +1,5 @@ +def Map/get(m): + return m + +def main: + return *(*(*(*(*(*))))) diff --git a/tests/snapshots/parse_file__redefinition_builtin.bend.snap b/tests/snapshots/parse_file__redefinition_builtin.bend.snap new file mode 100644 index 000000000..2ea6f62fe --- /dev/null +++ b/tests/snapshots/parse_file__redefinition_builtin.bend.snap @@ -0,0 +1,11 @@ +--- +source: tests/golden_tests.rs +input_file: tests/golden_tests/parse_file/redefinition_builtin.bend +--- +Errors: +In tests/golden_tests/parse_file/redefinition_builtin.bend : +Redefinition of builtin (function) 'Map/get'. + 1 | def Map/get(m): + 2 |  return m + 3 |  + 4 | def main: diff --git a/tests/snapshots/parse_file__redefinition_imp_fun.bend.snap b/tests/snapshots/parse_file__redefinition_imp_fun.bend.snap index e14ae79dd..39d574ed1 100644 --- a/tests/snapshots/parse_file__redefinition_imp_fun.bend.snap +++ b/tests/snapshots/parse_file__redefinition_imp_fun.bend.snap @@ -4,5 +4,5 @@ input_file: tests/golden_tests/parse_file/redefinition_imp_fun.bend --- Errors: In tests/golden_tests/parse_file/redefinition_imp_fun.bend : -Redefinition of function 'A' +Redefinition of function 'A'.  5 | (A) = 1 diff --git a/tests/snapshots/parse_file__redefinition_with_def_between.bend.snap b/tests/snapshots/parse_file__redefinition_with_def_between.bend.snap index aca21a527..25c124613 100644 --- a/tests/snapshots/parse_file__redefinition_with_def_between.bend.snap +++ b/tests/snapshots/parse_file__redefinition_with_def_between.bend.snap @@ -4,5 +4,5 @@ input_file: tests/golden_tests/parse_file/redefinition_with_def_between.bend --- Errors: In tests/golden_tests/parse_file/redefinition_with_def_between.bend : -Redefinition of function 'A' +Redefinition of function 'A'.  4 | (A) = @x x diff --git a/tests/snapshots/parse_file__redefinition_with_object_between.bend.snap b/tests/snapshots/parse_file__redefinition_with_object_between.bend.snap index a95e81462..707c7f137 100644 --- a/tests/snapshots/parse_file__redefinition_with_object_between.bend.snap +++ b/tests/snapshots/parse_file__redefinition_with_object_between.bend.snap @@ -4,5 +4,5 @@ input_file: tests/golden_tests/parse_file/redefinition_with_object_between.bend --- Errors: In tests/golden_tests/parse_file/redefinition_with_object_between.bend : -Redefinition of function 'A' +Redefinition of function 'A'.  3 | A = 1 diff --git a/tests/snapshots/parse_file__redefinition_with_type_between.bend.snap b/tests/snapshots/parse_file__redefinition_with_type_between.bend.snap index 264ad129d..ccdc4328f 100644 --- a/tests/snapshots/parse_file__redefinition_with_type_between.bend.snap +++ b/tests/snapshots/parse_file__redefinition_with_type_between.bend.snap @@ -4,5 +4,5 @@ input_file: tests/golden_tests/parse_file/redefinition_with_type_between.bend --- Errors: In tests/golden_tests/parse_file/redefinition_with_type_between.bend : -Redefinition of function 'A' +Redefinition of function 'A'.  3 | A = 1 From 3a023c692861539746ca28722fb63d272c0a6d27 Mon Sep 17 00:00:00 2001 From: imaqtkatt Date: Fri, 7 Jun 2024 18:57:45 -0300 Subject: [PATCH 2/2] Update builtins.rs and redefinition error messages for types and objects --- src/fun/builtins.rs | 17 +++++++++ src/fun/parser.rs | 38 +++++++++++-------- src/imp/parser.rs | 20 +++++----- .../parse_file/redefinition_builtin.bend | 3 -- .../parse_file/redefinition_ctr_with_fun.bend | 2 + .../redefinition_type_with_object.bend | 1 + .../compile_file_o_all__adt_string.bend.snap | 2 +- ...parse_file__redefinition_builtin.bend.snap | 3 +- ..._file__redefinition_ctr_with_fun.bend.snap | 10 +++++ ...e__redefinition_type_with_object.bend.snap | 9 +++++ .../parse_file__repeated_adt_name.bend.snap | 2 +- 11 files changed, 74 insertions(+), 33 deletions(-) create mode 100644 tests/golden_tests/parse_file/redefinition_ctr_with_fun.bend create mode 100644 tests/golden_tests/parse_file/redefinition_type_with_object.bend create mode 100644 tests/snapshots/parse_file__redefinition_ctr_with_fun.bend.snap create mode 100644 tests/snapshots/parse_file__redefinition_type_with_object.bend.snap diff --git a/src/fun/builtins.rs b/src/fun/builtins.rs index 48bfcbdf3..1ef955ec8 100644 --- a/src/fun/builtins.rs +++ b/src/fun/builtins.rs @@ -25,6 +25,23 @@ pub const NAT_SUCC: &str = "Nat/Succ"; pub const NAT_ZERO: &str = "Nat/Zero"; pub const NAT_SUCC_TAG: u32 = 0; +pub const TREE: &str = "Tree"; +pub const TREE_NODE: &str = "Tree/Node"; +pub const TREE_LEAF: &str = "Tree/Leaf"; + +pub const MAP: &str = "Map"; +pub const MAP_NODE: &str = "Map/Node"; +pub const MAP_LEAF: &str = "Map/Leaf"; + +pub const IO: &str = "IO"; +pub const IO_DONE: &str = "IO/Done"; +pub const IO_CALL: &str = "IO/Call"; + +pub const BUILTIN_CTRS: &[&str] = + &[LCONS, LNIL, SCONS, SNIL, NAT_SUCC, NAT_ZERO, TREE_NODE, TREE_LEAF, MAP_NODE, MAP_LEAF, IO_DONE, IO_CALL]; + +pub const BUILTIN_TYPES: &[&str] = &[LIST, STRING, NAT, TREE, MAP, IO]; + impl Book { pub fn builtins() -> Book { TermParser::new(BUILTINS) diff --git a/src/fun/parser.rs b/src/fun/parser.rs index 0ba98453d..af105b906 100644 --- a/src/fun/parser.rs +++ b/src/fun/parser.rs @@ -133,12 +133,12 @@ impl<'a> TermParser<'a> { def.rules.push(rule); } else { // Trying to add a new rule to a previous definition, coming from a different rule. - let msg = self.redefinition_of_function_msg(builtin, &name); + let msg = Self::redefinition_of_function_msg(builtin, &name); return self.with_ctx(Err(msg), ini_idx, end_idx); } } else { // Trying to add a new rule to a previous definition, coming from another kind of top-level. - let msg = self.redefinition_of_function_msg(builtin, &name); + let msg = Self::redefinition_of_function_msg(builtin, &name); return self.with_ctx(Err(msg), ini_idx, end_idx); } } else { @@ -801,23 +801,13 @@ impl Indent { impl Book { fn add_adt(&mut self, nam: Name, adt: Adt) -> ParseResult<()> { - if let Some(adt) = self.adts.get(&nam) { - if adt.builtin { - return Err(format!("{} is a built-in datatype and should not be overridden.", nam)); - } else { - return Err(format!("Repeated datatype '{}'", nam)); - } + if self.adts.contains_key(&nam) { + Err(TermParser::redefinition_of_type_msg(&nam))? } else { for ctr in adt.ctrs.keys() { match self.ctrs.entry(ctr.clone()) { indexmap::map::Entry::Vacant(e) => _ = e.insert(nam.clone()), - indexmap::map::Entry::Occupied(e) => { - if self.adts.get(e.get()).is_some_and(|adt| adt.builtin) { - return Err(format!("{} is a built-in constructor and should not be overridden.", e.key())); - } else { - return Err(format!("Repeated constructor '{}'", e.key())); - } - } + indexmap::map::Entry::Occupied(e) => Err(TermParser::redefinition_of_constructor_msg(e.key()))?, } } self.adts.insert(nam.clone(), adt); @@ -1235,11 +1225,27 @@ pub trait ParserCommons<'a>: Parser<'a> { Ok(result) } - fn redefinition_of_function_msg(&self, builtin: bool, function_name: &str) -> String { + fn redefinition_of_function_msg(builtin: bool, function_name: &str) -> String { if builtin { format!("Redefinition of builtin (function) '{function_name}'.") } else { format!("Redefinition of function '{function_name}'.") } } + + fn redefinition_of_constructor_msg(constructor_name: &str) -> String { + if crate::fun::builtins::BUILTIN_CTRS.contains(&constructor_name) { + format!("Redefinition of builtin (constructor) '{constructor_name}'.") + } else { + format!("Redefinition of constructor '{constructor_name}'.") + } + } + + fn redefinition_of_type_msg(type_name: &str) -> String { + if crate::fun::builtins::BUILTIN_TYPES.contains(&type_name) { + format!("Redefinition of builtin (type) '{type_name}'.") + } else { + format!("Redefinition of type '{type_name}'.") + } + } } diff --git a/src/imp/parser.rs b/src/imp/parser.rs index 7fb9e3a38..ce9ffa4eb 100644 --- a/src/imp/parser.rs +++ b/src/imp/parser.rs @@ -1070,11 +1070,11 @@ impl<'a> PyParser<'a> { builtin: bool, ) -> ParseResult<()> { if let Some(def) = book.defs.get(&def.name) { - let msg = self.redefinition_of_function_msg(def.builtin, &def.name); + let msg = Self::redefinition_of_function_msg(def.builtin, &def.name); return self.with_ctx(Err(msg), ini_idx, end_idx); } if book.ctrs.contains_key(&def.name) { - let msg = format!("Redefinition of constructor '{}'.", def.name); + let msg = Self::redefinition_of_constructor_msg(&def.name); return self.with_ctx(Err(msg), ini_idx, end_idx); } def.order_kwargs(book)?; @@ -1093,17 +1093,17 @@ impl<'a> PyParser<'a> { builtin: bool, ) -> ParseResult<()> { if book.adts.contains_key(&r#enum.name) { - let msg = format!("Redefinition of type '{}'.", r#enum.name); + let msg = PyParser::redefinition_of_type_msg(&r#enum.name); return self.with_ctx(Err(msg), ini_idx, end_idx); } let mut adt = Adt { ctrs: Default::default(), builtin }; for variant in r#enum.variants { - if book.defs.contains_key(&variant.name) { - let msg = format!("Redefinition of function '{}'.", variant.name); + if let Some(def) = book.defs.get(&variant.name) { + let msg = PyParser::redefinition_of_function_msg(def.builtin, &variant.name); return self.with_ctx(Err(msg), ini_idx, end_idx); } if book.ctrs.contains_key(&variant.name) { - let msg = format!("Redefinition of constructor '{}'.", variant.name); + let msg = PyParser::redefinition_of_constructor_msg(&variant.name); return self.with_ctx(Err(msg), ini_idx, end_idx); } book.ctrs.insert(variant.name.clone(), r#enum.name.clone()); @@ -1122,16 +1122,16 @@ impl<'a> PyParser<'a> { builtin: bool, ) -> ParseResult<()> { if book.adts.contains_key(&obj.name) { - let msg = format!("Redefinition of type '{}'.", obj.name); + let msg = PyParser::redefinition_of_type_msg(&obj.name); return self.with_ctx(Err(msg), ini_idx, end_idx); } let mut adt = Adt { ctrs: Default::default(), builtin }; - if book.defs.contains_key(&obj.name) { - let msg = format!("Redefinition of function '{}'.", obj.name); + if let Some(def) = book.defs.get(&obj.name) { + let msg = PyParser::redefinition_of_function_msg(def.builtin, &obj.name); return self.with_ctx(Err(msg), ini_idx, end_idx); } if book.ctrs.contains_key(&obj.name) { - let msg = format!("Redefinition of constructor '{}'.", obj.name); + let msg = PyParser::redefinition_of_constructor_msg(&obj.name); return self.with_ctx(Err(msg), ini_idx, end_idx); } book.ctrs.insert(obj.name.clone(), obj.name.clone()); diff --git a/tests/golden_tests/parse_file/redefinition_builtin.bend b/tests/golden_tests/parse_file/redefinition_builtin.bend index 5dae112fc..b2ed168cf 100644 --- a/tests/golden_tests/parse_file/redefinition_builtin.bend +++ b/tests/golden_tests/parse_file/redefinition_builtin.bend @@ -1,5 +1,2 @@ def Map/get(m): return m - -def main: - return *(*(*(*(*(*))))) diff --git a/tests/golden_tests/parse_file/redefinition_ctr_with_fun.bend b/tests/golden_tests/parse_file/redefinition_ctr_with_fun.bend new file mode 100644 index 000000000..3f2ed51fc --- /dev/null +++ b/tests/golden_tests/parse_file/redefinition_ctr_with_fun.bend @@ -0,0 +1,2 @@ +def String/Cons(x): + return x diff --git a/tests/golden_tests/parse_file/redefinition_type_with_object.bend b/tests/golden_tests/parse_file/redefinition_type_with_object.bend new file mode 100644 index 000000000..305287cd5 --- /dev/null +++ b/tests/golden_tests/parse_file/redefinition_type_with_object.bend @@ -0,0 +1 @@ +object IO { run } diff --git a/tests/snapshots/compile_file_o_all__adt_string.bend.snap b/tests/snapshots/compile_file_o_all__adt_string.bend.snap index 086bacc17..f5d4470c6 100644 --- a/tests/snapshots/compile_file_o_all__adt_string.bend.snap +++ b/tests/snapshots/compile_file_o_all__adt_string.bend.snap @@ -4,7 +4,7 @@ input_file: tests/golden_tests/compile_file_o_all/adt_string.bend --- Errors: In tests/golden_tests/compile_file_o_all/adt_string.bend : -String is a built-in datatype and should not be overridden. +Redefinition of builtin (type) 'String'.  1 | type String = S  2 |   3 | main = String/S diff --git a/tests/snapshots/parse_file__redefinition_builtin.bend.snap b/tests/snapshots/parse_file__redefinition_builtin.bend.snap index 2ea6f62fe..8d444e7fb 100644 --- a/tests/snapshots/parse_file__redefinition_builtin.bend.snap +++ b/tests/snapshots/parse_file__redefinition_builtin.bend.snap @@ -7,5 +7,4 @@ In tests/golden_tests/parse_file/redefinition_builtin.bend : Redefinition of builtin (function) 'Map/get'.  1 | def Map/get(m):  2 |  return m - 3 |  - 4 | def main: + diff --git a/tests/snapshots/parse_file__redefinition_ctr_with_fun.bend.snap b/tests/snapshots/parse_file__redefinition_ctr_with_fun.bend.snap new file mode 100644 index 000000000..04f507209 --- /dev/null +++ b/tests/snapshots/parse_file__redefinition_ctr_with_fun.bend.snap @@ -0,0 +1,10 @@ +--- +source: tests/golden_tests.rs +input_file: tests/golden_tests/parse_file/redefinition_ctr_with_fun.bend +--- +Errors: +In tests/golden_tests/parse_file/redefinition_ctr_with_fun.bend : +Redefinition of builtin (constructor) 'String/Cons'. + 1 | def String/Cons(x): + 2 |  return x + diff --git a/tests/snapshots/parse_file__redefinition_type_with_object.bend.snap b/tests/snapshots/parse_file__redefinition_type_with_object.bend.snap new file mode 100644 index 000000000..18d84fd3d --- /dev/null +++ b/tests/snapshots/parse_file__redefinition_type_with_object.bend.snap @@ -0,0 +1,9 @@ +--- +source: tests/golden_tests.rs +input_file: tests/golden_tests/parse_file/redefinition_type_with_object.bend +--- +Errors: +In tests/golden_tests/parse_file/redefinition_type_with_object.bend : +Redefinition of builtin (type) 'IO'. + 1 | object IO { run } + diff --git a/tests/snapshots/parse_file__repeated_adt_name.bend.snap b/tests/snapshots/parse_file__repeated_adt_name.bend.snap index f3b36ef14..0caef38e2 100644 --- a/tests/snapshots/parse_file__repeated_adt_name.bend.snap +++ b/tests/snapshots/parse_file__repeated_adt_name.bend.snap @@ -4,7 +4,7 @@ input_file: tests/golden_tests/parse_file/repeated_adt_name.bend --- Errors: In tests/golden_tests/parse_file/repeated_adt_name.bend : -Repeated datatype 'Foo' +Redefinition of type 'Foo'.  2 | type Foo = B  3 |   4 | main = *