Replace panic! with internal-error! in ast crate #4297
Conversation
There was a problem hiding this comment.
@ayazhafiz or @folkertdev, could you take a look at this? Some of these cases look like the might be user_error! instead of internal_error!. I just don't know this code well enough.
| @@ -188,7 +189,7 @@ fn canonicalize_field<'a>( | |||
| } | |||
|
|
|||
| Malformed(_string) => { | |||
| panic!("TODO canonicalize malformed record field"); | |||
| internal_error!("TODO canonicalize malformed record field"); | |||
There was a problem hiding this comment.
Can we making things that say TODO into todo!?
| @@ -2064,7 +2065,7 @@ pub mod test_constrain { | |||
|
|
|||
| assert_eq!(actual_str, expected_str); | |||
| } | |||
| Err(e) => panic!("syntax error {:?}", e), | |||
| Err(e) => internal_error!("syntax error {:?}", e), | |||
There was a problem hiding this comment.
I am not very fimilare with this code, but I wonder if this is a user_error! that just doesn't have a pretty message yet.
There was a problem hiding this comment.
I think using internal_error is still okay here, things that don't exit well to the user are internal errors to the compiler.
crates/ast/src/lang/core/def/def.rs
Outdated
| @@ -574,7 +574,7 @@ fn canonicalize_pending_def<'a>( | |||
| .. | |||
| } => { | |||
| if arguments.len() != type_arguments.len() { | |||
| panic!("argument number mismatch"); | |||
| internal_error!("argument number mismatch"); | |||
There was a problem hiding this comment.
Same with this and the errors below in this file. They might really be user_error!
crates/ast/src/solve_type.rs
Outdated
| self.0 | ||
| .get_mut(rank.into_usize()) | ||
| .unwrap_or_else(|| panic!("Compiler bug: could not find pool at rank {}", rank)) | ||
| self.0.get_mut(rank.into_usize()).unwrap_or_else(|| { |
There was a problem hiding this comment.
nit: remove the Compiler bug: prefix here and below
crates/ast/src/module.rs
Outdated
| @@ -20,14 +21,16 @@ pub fn load_module(src_file: &Path, threading: Threading) -> LoadedModule { | |||
| match loaded { | |||
| Ok(x) => x, | |||
| Err(roc_load::LoadingProblem::FormattedReport(report)) => { | |||
| panic!( | |||
| internal_error!( | |||
There was a problem hiding this comment.
Also, here, not sure if this should technically be a user_error!
|
Thanks so much for doing this! These are the small steps to eventually getting a robust and fuzzable compiler. Makes it clear to the end users when they should to file bugs. |
| @@ -188,7 +189,7 @@ fn canonicalize_field<'a>( | |||
| } | |||
|
|
|||
| Malformed(_string) => { | |||
| panic!("TODO canonicalize malformed record field"); | |||
| internal_error!("todo! canonicalize malformed record field"); | |||
There was a problem hiding this comment.
Oh, this was a slight misunderstanding. I meant todo!("canonicalize malformed record field")
There was a problem hiding this comment.
Oh, yes of course.
crates/ast/src/solve_type.rs
Outdated
| .get(rank.into_usize()) | ||
| .unwrap_or_else(|| panic!("Compiler bug: could not find pool at rank {}", rank)) | ||
| self.0.get(rank.into_usize()).unwrap_or_else(|| { | ||
| internal_error!("Compiler bug: could not find pool at rank {}", rank) |
There was a problem hiding this comment.
nit: remove Compiler bug: here as well.
Hi,
I replaced
panic!withinternal_error!.I started with only
panic!and only inside theastcrate to keep this PR small.Related to #2046