Anthony Bullard (Jan 06 2025 at 22:55): Anthony Bullard (Jan 06 2025 at 22:55):I have lost a lot of time to incomplete pattern matches falling back to some kind of fallback behavior. I think I'm OK if the fallback case is an unreachable!(), but nothing else.
Please discuss. :smile:
Sam Mohr (Jan 06 2025 at 23:27):I'm not sure what you're saying.
Sam Mohr (Jan 06 2025 at 23:27):Can you give an example?
Sam Mohr (Jan 06 2025 at 23:28):Is this in canonicalization?
Anthony Bullard (Jan 07 2025 at 00:59):And parse
Anthony Bullard (Jan 07 2025 at 00:59):And fmt
Richard Feldman (Jan 07 2025 at 01:37):you mean stop using _ => in match branches and make them all exhaustive, yeah?
Sam Mohr (Jan 07 2025 at 01:38):I presume so
Sam Mohr (Jan 07 2025 at 01:39):I think this is mostly a problem when it would lead to a crash
Sam Mohr (Jan 07 2025 at 01:40):I'm wondering what your opinion is when we want to unwrap a specific type of node, I'll get you a link
Sam Mohr (Jan 07 2025 at 01:42):Do you think that this is a bad code pattern for checking all abilities while maintaining a single loop over all top-level types? https://github.com/roc-lang/roc/blob/a089cf2487d2cb5f1d977a2e01be5ab7b4de3275/crates/compiler/can/src/def.rs#L1048
for either_index in loc_defs.tags.iter() {
if let Ok(type_index) = either_index.split() {
let type_def = &loc_defs.type_defs[type_index.index()];
let pending_type_def = to_pending_type_def(env, type_def, scope, pattern_type);
if let PendingTypeDef::Ability { name, members } = &pending_type_def {
pending_abilities_in_scope.insert(
name.value,
members.iter().map(|mem| mem.name.value).collect(),
);
}
pending_type_defs.push(pending_type_def);
}
}
The if let PendingTypeDef::Ability ...
Sam Mohr (Jan 07 2025 at 01:44):Similar usage of a while let here: https://github.com/roc-lang/roc/blob/a089cf2487d2cb5f1d977a2e01be5ab7b4de3275/crates/compiler/can/src/def.rs#L2560
fn canonicalize_pending_body<'a>(...) -> DefOutput {
let mut loc_value = &loc_expr.value;
while let ast::Expr::ParensAround(value) = loc_value {
loc_value = value;
}
I think it's a generally useful technique to inexhaustively check an enum when there are many variants and you only care about one or two of them
Sam Mohr (Jan 07 2025 at 01:46):A way to get around this is to model your data more precisely to your needs, but that can be inefficient, and there are some cleanliness things that aren't worth the tradeoff
Ayaz Hafiz (Jan 07 2025 at 02:08):i think this might be a higher level thing where this is too much matching on variants going on in general and they should be centralized to one place or made function calls instead. i have definitely felt this before
Sam Mohr (Jan 07 2025 at 02:09):I definitely have a hard time figuring out how much the team likes helper functions vs. one big stream of code
Sam Mohr (Jan 07 2025 at 02:10):Outside of the Roc compiler, I would normally pull something like fn unparens_expr(Expr) -> Expr
Ayaz Hafiz (Jan 07 2025 at 02:17):saying this only for the code that i have written, but i have written a lot of the code in the currrnt compiler implementation, i wouldn’t take the current patterns as definitive. a lot of what i wrote were semi poorly structured things we expected to fix and never did
Anthony Bullard (Jan 07 2025 at 02:37):The big problem is if you add a new case to an enum, you can silently fall into some bad behavior because you don't get an exhaustiveness error
Anthony Bullard (Jan 07 2025 at 02:37):And it can feel mysterious
Anthony Bullard (Jan 07 2025 at 02:38):The joy of pattern matching is exhaustiveness checking - it's what makes ADTs delicious.
Anthony Bullard (Jan 07 2025 at 02:38):_ => can - in a large codebase especially - make them instead miserable
Anthony Bullard (Jan 07 2025 at 02:39):I don't even mind the long functions as much
Anthony Bullard (Jan 07 2025 at 02:40):I typically only refactor into functions to share either a) long bits of code (especially if likely to change), and b) code that is repeated a LOT of times and is likely to change in the same way in the vast majority of the original code sites
Jakub Konka (Jan 13 2025 at 22:53):FWIW in Zig I would try to always exhaust or fallthrough with else => unreachable, (or an error depending on the context). In Rust, I would usually strive to do the sams however I did notice certain opposition of some Rustacean to _ => unreachable!() or more generally assert-based programming.
Anthony Bullard (Jan 13 2025 at 23:08):Assert-based programming is the most based programming
Last updated: Jul 06 2025 at 12:14 UTC