From 408379f8d3cae259657703112aed6ea4f9bfdcf7 Mon Sep 17 00:00:00 2001 From: Joshua Warner Date: Thu, 9 Jan 2025 21:57:36 -0800 Subject: [PATCH 1/6] Canonicalize the pattern in annotations --- crates/compiler/can/src/desugar.rs | 6 +- .../fuzz/fuzz_targets/fuzz_expr.rs | 2 +- crates/compiler/test_syntax/src/minimize.rs | 18 ++++++ ...ace_after_opt_field_pat.expr.formatted.roc | 2 + .../space_after_opt_field_pat.expr.result-ast | 56 +++++++++++++++++++ .../pass/space_after_opt_field_pat.expr.roc | 3 + .../test_syntax/tests/test_snapshots.rs | 1 + crates/error_macros/src/lib.rs | 1 + 8 files changed, 86 insertions(+), 3 deletions(-) create mode 100644 crates/compiler/test_syntax/tests/snapshots/pass/space_after_opt_field_pat.expr.formatted.roc create mode 100644 crates/compiler/test_syntax/tests/snapshots/pass/space_after_opt_field_pat.expr.result-ast create mode 100644 crates/compiler/test_syntax/tests/snapshots/pass/space_after_opt_field_pat.expr.roc diff --git a/crates/compiler/can/src/desugar.rs b/crates/compiler/can/src/desugar.rs index 30bf100503b..b9ee479da67 100644 --- a/crates/compiler/can/src/desugar.rs +++ b/crates/compiler/can/src/desugar.rs @@ -311,7 +311,9 @@ fn desugar_value_def<'a>( desugar_loc_pattern(env, scope, loc_pattern), desugar_expr(env, scope, loc_expr), ), - ann @ Annotation(_, _) => *ann, + Annotation(ann_pattern, ann_type) => { + Annotation(*desugar_loc_pattern(env, scope, ann_pattern), *ann_type) + } AnnotatedBody { ann_pattern, ann_type, @@ -319,7 +321,7 @@ fn desugar_value_def<'a>( body_pattern, body_expr, } => AnnotatedBody { - ann_pattern, + ann_pattern: desugar_loc_pattern(env, scope, ann_pattern), ann_type, lines_between, body_pattern: desugar_loc_pattern(env, scope, body_pattern), diff --git a/crates/compiler/test_syntax/fuzz/fuzz_targets/fuzz_expr.rs b/crates/compiler/test_syntax/fuzz/fuzz_targets/fuzz_expr.rs index 25f948f832b..a83d5a6f0ce 100644 --- a/crates/compiler/test_syntax/fuzz/fuzz_targets/fuzz_expr.rs +++ b/crates/compiler/test_syntax/fuzz/fuzz_targets/fuzz_expr.rs @@ -11,7 +11,7 @@ fuzz_target!(|data: &[u8]| { let ast = input.parse_in(&arena); if let Ok(ast) = ast { if !ast.is_malformed() { - input.check_invariants(|_| (), true, None); + input.check_invariants(|_| (), true, Some(false)); } } } diff --git a/crates/compiler/test_syntax/src/minimize.rs b/crates/compiler/test_syntax/src/minimize.rs index 19b7e16ceed..a9d89a9bfde 100644 --- a/crates/compiler/test_syntax/src/minimize.rs +++ b/crates/compiler/test_syntax/src/minimize.rs @@ -151,6 +151,24 @@ fn round_trip_once(input: Input<'_>, options: Options) -> Option { return Some("Formatting not stable".to_string()); } + let text = input.as_str(); + let res = std::panic::catch_unwind(|| { + let new_arena = Bump::new(); + actual.canonicalize(&new_arena, text); + }); + + if let Err(e) = res { + if options.minimize_full_error { + if let Some(s) = e.downcast_ref::<&'static str>() { + return Some(s.to_string()); + } + if let Some(s) = e.downcast_ref::() { + return Some(s.clone()); + } + } + return Some("Panic during canonicalization".to_string()); + } + None } diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/space_after_opt_field_pat.expr.formatted.roc b/crates/compiler/test_syntax/tests/snapshots/pass/space_after_opt_field_pat.expr.formatted.roc new file mode 100644 index 00000000000..a70b861d147 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/space_after_opt_field_pat.expr.formatted.roc @@ -0,0 +1,2 @@ +{ p ?? m } : J +O \ No newline at end of file diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/space_after_opt_field_pat.expr.result-ast b/crates/compiler/test_syntax/tests/snapshots/pass/space_after_opt_field_pat.expr.result-ast new file mode 100644 index 00000000000..3afcba598f6 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/space_after_opt_field_pat.expr.result-ast @@ -0,0 +1,56 @@ +@0-10 SpaceAfter( + Defs( + Defs { + tags: [ + EitherIndex(2147483648), + ], + regions: [ + @0-8, + ], + space_before: [ + Slice { start: 0, length: 0 }, + ], + space_after: [ + Slice { start: 0, length: 0 }, + ], + spaces: [], + type_defs: [], + value_defs: [ + Annotation( + @0-6 RecordDestructure( + [ + @1-5 OptionalField( + "p", + @4-5 SpaceBefore( + Var { + module_name: "", + ident: "m", + }, + [ + Newline, + ], + ), + ), + ], + ), + @7-8 Apply( + "", + "J", + [], + ), + ), + ], + }, + @9-10 SpaceBefore( + Tag( + "O", + ), + [ + Newline, + ], + ), + ), + [ + Newline, + ], +) diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/space_after_opt_field_pat.expr.roc b/crates/compiler/test_syntax/tests/snapshots/pass/space_after_opt_field_pat.expr.roc new file mode 100644 index 00000000000..1d6b13e7628 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/space_after_opt_field_pat.expr.roc @@ -0,0 +1,3 @@ +{p? +m}:J +O diff --git a/crates/compiler/test_syntax/tests/test_snapshots.rs b/crates/compiler/test_syntax/tests/test_snapshots.rs index a9b5f626ec3..6de51d24c3c 100644 --- a/crates/compiler/test_syntax/tests/test_snapshots.rs +++ b/crates/compiler/test_syntax/tests/test_snapshots.rs @@ -690,6 +690,7 @@ mod test_snapshots { pass/single_arg_with_underscore_closure.expr, pass/single_underscore_closure.expr, pass/sneaky_implements_in_opaque_fn_type.expr, + pass/space_after_opt_field_pat.expr, pass/space_before_colon.full, pass/space_before_parens_space_after.expr, pass/space_only_after_minus.expr, diff --git a/crates/error_macros/src/lib.rs b/crates/error_macros/src/lib.rs index 10ec886aaca..3f743e6ff4b 100644 --- a/crates/error_macros/src/lib.rs +++ b/crates/error_macros/src/lib.rs @@ -43,6 +43,7 @@ pub fn set_panic_not_exit(inp: bool) { #[inline(never)] #[cold] #[cfg(any(unix, windows, target_arch = "wasm32"))] +#[track_caller] pub fn error_and_exit(args: fmt::Arguments) -> ! { use core::panic; From 7d464a29890511fe72c0de23f36c03393229f321 Mon Sep 17 00:00:00 2001 From: Joshua Warner Date: Fri, 10 Jan 2025 21:46:20 -0800 Subject: [PATCH 2/6] add failing test for ignored fields canonicalization --- crates/compiler/can/src/expr.rs | 55 +++++++++++++- crates/compiler/problem/src/can.rs | 16 ++++ ...ignored_field_in_import.expr.formatted.roc | 4 + ...an_ignored_field_in_import.expr.result-ast | 67 +++++++++++++++++ .../pass/can_ignored_field_in_import.expr.roc | 3 + .../test_syntax/tests/test_snapshots.rs | 1 + crates/reporting/src/error/canonicalize.rs | 74 +++++++++++++++++++ 7 files changed, 217 insertions(+), 3 deletions(-) create mode 100644 crates/compiler/test_syntax/tests/snapshots/pass/can_ignored_field_in_import.expr.formatted.roc create mode 100644 crates/compiler/test_syntax/tests/snapshots/pass/can_ignored_field_in_import.expr.result-ast create mode 100644 crates/compiler/test_syntax/tests/snapshots/pass/can_ignored_field_in_import.expr.roc diff --git a/crates/compiler/can/src/expr.rs b/crates/compiler/can/src/expr.rs index 8308f8a06bd..f1b0b3a2542 100644 --- a/crates/compiler/can/src/expr.rs +++ b/crates/compiler/can/src/expr.rs @@ -974,6 +974,18 @@ pub fn canonicalize_expr<'a>( }), Output::default(), ), + Err(CanonicalizeRecordProblem::InvalidIgnoredValue { + field_name, + field_region, + record_region, + }) => ( + Expr::RuntimeError(roc_problem::can::RuntimeError::InvalidIgnoredValue { + field_name, + field_region, + record_region, + }), + Output::default(), + ), } } else { // only (optionally qualified) variables can be updated, not arbitrary expressions @@ -1676,6 +1688,18 @@ pub fn canonicalize_record<'a>( }), Output::default(), ), + Err(CanonicalizeRecordProblem::InvalidIgnoredValue { + field_name, + field_region, + record_region, + }) => ( + Expr::RuntimeError(roc_problem::can::RuntimeError::InvalidIgnoredValue { + field_name, + field_region, + record_region, + }), + Output::default(), + ), } } } @@ -2016,6 +2040,11 @@ enum CanonicalizeRecordProblem { field_region: Region, record_region: Region, }, + InvalidIgnoredValue { + field_name: Lowercase, + field_region: Region, + record_region: Region, + }, } fn canonicalize_fields<'a>( env: &mut Env<'a>, @@ -2064,6 +2093,21 @@ fn canonicalize_fields<'a>( record_region: region, }); } + Err(CanonicalizeFieldProblem::InvalidIgnoredValue { + field_name, + field_region, + }) => { + env.problems.push(Problem::InvalidIgnoredValue { + field_name: field_name.clone(), + field_region, + record_region: region, + }); + return Err(CanonicalizeRecordProblem::InvalidIgnoredValue { + field_name, + field_region, + record_region: region, + }); + } } } @@ -2075,6 +2119,10 @@ enum CanonicalizeFieldProblem { field_name: Lowercase, field_region: Region, }, + InvalidIgnoredValue { + field_name: Lowercase, + field_region: Region, + }, } fn canonicalize_field<'a>( env: &mut Env<'a>, @@ -2105,9 +2153,10 @@ fn canonicalize_field<'a>( }), // An ignored value, e.g. `{ _name: 123 }` - IgnoredValue(_, _, _) => { - internal_error!("Somehow an IgnoredValue record field was not desugared!"); - } + IgnoredValue(name, _, value) => Err(CanonicalizeFieldProblem::InvalidIgnoredValue { + field_name: Lowercase::from(name.value), + field_region: Region::span_across(&name.region, &value.region), + }), // A label with no value, e.g. `{ name }` (this is sugar for { name: name }) LabelOnly(_) => { diff --git a/crates/compiler/problem/src/can.rs b/crates/compiler/problem/src/can.rs index c75dbdd2772..36501284de2 100644 --- a/crates/compiler/problem/src/can.rs +++ b/crates/compiler/problem/src/can.rs @@ -255,6 +255,11 @@ pub enum Problem { SuffixedPureRecordField(Region), EmptyTupleType(Region), UnboundTypeVarsInAs(Region), + InvalidIgnoredValue { + field_name: Lowercase, + field_region: Region, + record_region: Region, + }, } #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -291,6 +296,7 @@ impl Problem { Problem::DuplicateRecordFieldValue { .. } => Warning, Problem::DuplicateRecordFieldType { .. } => RuntimeError, Problem::InvalidOptionalValue { .. } => RuntimeError, + Problem::InvalidIgnoredValue { .. } => RuntimeError, Problem::DuplicateTag { .. } => RuntimeError, Problem::RuntimeError(_) => RuntimeError, Problem::SignatureDefMismatch { .. } => RuntimeError, @@ -401,6 +407,10 @@ impl Problem { record_region: region, .. } + | Problem::InvalidIgnoredValue { + record_region: region, + .. + } | Problem::DuplicateTag { tag_union_region: region, .. @@ -553,6 +563,11 @@ pub enum RuntimeError { record_region: Region, field_region: Region, }, + InvalidIgnoredValue { + field_name: Lowercase, + field_region: Region, + record_region: Region, + }, // Example: (5 = 1 + 2) is an unsupported pattern in an assignment; Int patterns aren't allowed in assignments! UnsupportedPattern(Region), // Example: when 1 is 1.X -> 32 @@ -686,6 +701,7 @@ impl RuntimeError { match self { RuntimeError::Shadowing { shadow, .. } => shadow.region, RuntimeError::InvalidOptionalValue { field_region, .. } => *field_region, + RuntimeError::InvalidIgnoredValue { field_region, .. } => *field_region, RuntimeError::UnsupportedPattern(region) | RuntimeError::MalformedPattern(_, region) | RuntimeError::OpaqueOutsideScope { diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/can_ignored_field_in_import.expr.formatted.roc b/crates/compiler/test_syntax/tests/snapshots/pass/can_ignored_field_in_import.expr.formatted.roc new file mode 100644 index 00000000000..30778599873 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/can_ignored_field_in_import.expr.formatted.roc @@ -0,0 +1,4 @@ +import P { + _: h, +} +t! \ No newline at end of file diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/can_ignored_field_in_import.expr.result-ast b/crates/compiler/test_syntax/tests/snapshots/pass/can_ignored_field_in_import.expr.result-ast new file mode 100644 index 00000000000..a642e159c67 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/can_ignored_field_in_import.expr.result-ast @@ -0,0 +1,67 @@ +@0-17 SpaceAfter( + Defs( + Defs { + tags: [ + EitherIndex(2147483648), + ], + regions: [ + @0-14, + ], + space_before: [ + Slice { start: 0, length: 0 }, + ], + space_after: [ + Slice { start: 0, length: 0 }, + ], + spaces: [], + type_defs: [], + value_defs: [ + ModuleImport( + ModuleImport { + before_name: [], + name: @7-8 ImportedModuleName { + package: None, + name: ModuleName( + "P", + ), + }, + params: Some( + ModuleImportParams { + before: [], + params: @8-14 [ + @9-12 SpaceAfter( + IgnoredValue( + @9-10 "", + [], + @11-12 Var { + module_name: "", + ident: "h", + }, + ), + [ + Newline, + ], + ), + ], + }, + ), + alias: None, + exposed: None, + }, + ), + ], + }, + @15-17 SpaceBefore( + Var { + module_name: "", + ident: "t!", + }, + [ + Newline, + ], + ), + ), + [ + Newline, + ], +) diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/can_ignored_field_in_import.expr.roc b/crates/compiler/test_syntax/tests/snapshots/pass/can_ignored_field_in_import.expr.roc new file mode 100644 index 00000000000..9f57c88a870 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/can_ignored_field_in_import.expr.roc @@ -0,0 +1,3 @@ +import P{_:h +} +t! diff --git a/crates/compiler/test_syntax/tests/test_snapshots.rs b/crates/compiler/test_syntax/tests/test_snapshots.rs index 6de51d24c3c..4de8a141503 100644 --- a/crates/compiler/test_syntax/tests/test_snapshots.rs +++ b/crates/compiler/test_syntax/tests/test_snapshots.rs @@ -357,6 +357,7 @@ mod test_snapshots { pass/body_with_unneeded_parens.expr, pass/call_bang.expr, pass/call_bang_no_space.expr, + pass/can_ignored_field_in_import.expr, pass/capture_body_parens_comment.expr, pass/closure_arg_parens_then_comment.expr, pass/closure_complex_pattern_indent_issue.expr, diff --git a/crates/reporting/src/error/canonicalize.rs b/crates/reporting/src/error/canonicalize.rs index 4cd9ae774e8..5bbd625be18 100644 --- a/crates/reporting/src/error/canonicalize.rs +++ b/crates/reporting/src/error/canonicalize.rs @@ -628,6 +628,20 @@ pub fn can_problem<'b>( record_region, ); } + Problem::InvalidIgnoredValue { + field_name, + field_region, + record_region, + } => { + return to_invalid_ignored_value_report( + alloc, + lines, + filename, + field_name, + field_region, + record_region, + ); + } Problem::DuplicateRecordFieldType { field_name, field_region, @@ -1518,6 +1532,51 @@ fn to_invalid_optional_value_report_help<'b>( ]) } +fn to_invalid_ignored_value_report<'b>( + alloc: &'b RocDocAllocator<'b>, + lines: &LineInfo, + filename: PathBuf, + field_name: Lowercase, + field_region: Region, + record_region: Region, +) -> Report<'b> { + let doc = + to_invalid_ignored_value_report_help(alloc, lines, field_name, field_region, record_region); + + Report { + title: "BAD IGNORED VALUE".to_string(), + filename, + doc, + severity: Severity::RuntimeError, + } +} + +fn to_invalid_ignored_value_report_help<'b>( + alloc: &'b RocDocAllocator<'b>, + lines: &LineInfo, + field_name: Lowercase, + field_region: Region, + record_region: Region, +) -> RocDocBuilder<'b> { + alloc.stack([ + alloc.concat([ + alloc.reflow("This record uses an ignored value for the "), + alloc.record_field(field_name), + alloc.reflow(" field in an incorrect context!"), + ]), + alloc.region_all_the_things( + lines.convert_region(record_region), + lines.convert_region(field_region), + lines.convert_region(field_region), + Annotation::Error, + ), + alloc.reflow(r"You can only use ignored values in record builders, like:"), + alloc + .reflow(r"{ Foo.Bar.baz <- x: 5, y: 0, _z: 3, _: 2 }") + .indent(4), + ]) +} + fn to_bad_ident_expr_report<'b>( alloc: &'b RocDocAllocator<'b>, lines: &LineInfo, @@ -2436,6 +2495,21 @@ fn pretty_runtime_error<'b>( title = SYNTAX_PROBLEM; } + RuntimeError::InvalidIgnoredValue { + field_name, + field_region, + record_region, + } => { + doc = to_invalid_ignored_value_report_help( + alloc, + lines, + field_name, + field_region, + record_region, + ); + + title = SYNTAX_PROBLEM; + } RuntimeError::InvalidRecordUpdate { region } => { doc = alloc.stack([ alloc.concat([ From e0ef01fa82b881c3423c7cac6feab018f00e9234 Mon Sep 17 00:00:00 2001 From: Joshua Warner Date: Sat, 11 Jan 2025 13:51:30 -0800 Subject: [PATCH 3/6] Fix two canonicalization crashes: try() and overflowed tuple indexes --- crates/compiler/can/src/desugar.rs | 11 ++++- crates/compiler/can/src/expr.rs | 49 +++++++++++++------ crates/compiler/problem/src/can.rs | 7 +++ .../pass/empty_try_pnc.expr.formatted.roc | 1 + .../pass/empty_try_pnc.expr.result-ast | 18 +++++++ .../snapshots/pass/empty_try_pnc.expr.roc | 1 + .../pass/large_tuple_index.expr.formatted.roc | 1 + .../pass/large_tuple_index.expr.result-ast | 10 ++++ .../snapshots/pass/large_tuple_index.expr.roc | 1 + .../test_syntax/tests/test_snapshots.rs | 2 + crates/reporting/src/error/canonicalize.rs | 29 +++++++++++ 11 files changed, 114 insertions(+), 16 deletions(-) create mode 100644 crates/compiler/test_syntax/tests/snapshots/pass/empty_try_pnc.expr.formatted.roc create mode 100644 crates/compiler/test_syntax/tests/snapshots/pass/empty_try_pnc.expr.result-ast create mode 100644 crates/compiler/test_syntax/tests/snapshots/pass/empty_try_pnc.expr.roc create mode 100644 crates/compiler/test_syntax/tests/snapshots/pass/large_tuple_index.expr.formatted.roc create mode 100644 crates/compiler/test_syntax/tests/snapshots/pass/large_tuple_index.expr.result-ast create mode 100644 crates/compiler/test_syntax/tests/snapshots/pass/large_tuple_index.expr.roc diff --git a/crates/compiler/can/src/desugar.rs b/crates/compiler/can/src/desugar.rs index b9ee479da67..b65979446c5 100644 --- a/crates/compiler/can/src/desugar.rs +++ b/crates/compiler/can/src/desugar.rs @@ -1003,7 +1003,16 @@ pub fn desugar_expr<'a>( }, loc_args, ) => { - let result_expr = if loc_args.len() == 1 { + let result_expr = if loc_args.is_empty() { + env.problem(Problem::UnderAppliedTry { + region: loc_expr.region, + }); + // Replace with a dummy expression to avoid cascading errors + env.arena.alloc(Loc { + value: Record(Collection::empty()), + region: loc_expr.region, + }) + } else if loc_args.len() == 1 { desugar_expr(env, scope, loc_args.items[0]) } else { let function = desugar_expr(env, scope, loc_args.items.first().unwrap()); diff --git a/crates/compiler/can/src/expr.rs b/crates/compiler/can/src/expr.rs index f1b0b3a2542..da1c3830372 100644 --- a/crates/compiler/can/src/expr.rs +++ b/crates/compiler/can/src/expr.rs @@ -1268,21 +1268,9 @@ pub fn canonicalize_expr<'a>( output, ) } - ast::Expr::AccessorFunction(field) => ( - RecordAccessor(StructAccessorData { - name: scope.gen_unique_symbol(), - function_var: var_store.fresh(), - record_var: var_store.fresh(), - ext_var: var_store.fresh(), - closure_var: var_store.fresh(), - field_var: var_store.fresh(), - field: match field { - Accessor::RecordField(field) => IndexOrField::Field((*field).into()), - Accessor::TupleIndex(index) => IndexOrField::Index(index.parse().unwrap()), - }, - }), - Output::default(), - ), + ast::Expr::AccessorFunction(field) => { + canonicalize_accessor_function(env, var_store, scope, field, region) + } ast::Expr::TupleAccess(tuple_expr, field) => { let (loc_expr, output) = canonicalize_expr(env, var_store, scope, region, tuple_expr); @@ -1656,6 +1644,37 @@ pub fn canonicalize_expr<'a>( ) } +fn canonicalize_accessor_function( + env: &mut Env<'_>, + var_store: &mut VarStore, + scope: &mut Scope, + field: &Accessor<'_>, + region: Region, +) -> (Expr, Output) { + ( + Expr::RecordAccessor(StructAccessorData { + name: scope.gen_unique_symbol(), + function_var: var_store.fresh(), + record_var: var_store.fresh(), + ext_var: var_store.fresh(), + closure_var: var_store.fresh(), + field_var: var_store.fresh(), + field: match field { + Accessor::RecordField(field) => IndexOrField::Field((*field).into()), + Accessor::TupleIndex(index) => match index.parse() { + Ok(index) => IndexOrField::Index(index), + Err(_) => { + let error = roc_problem::can::RuntimeError::InvalidTupleIndex(region); + env.problem(Problem::RuntimeError(error.clone())); + return (Expr::RuntimeError(error), Output::default()); + } + }, + }, + }), + Output::default(), + ) +} + pub fn canonicalize_record<'a>( env: &mut Env<'a>, var_store: &mut VarStore, diff --git a/crates/compiler/problem/src/can.rs b/crates/compiler/problem/src/can.rs index 36501284de2..1a0532c9396 100644 --- a/crates/compiler/problem/src/can.rs +++ b/crates/compiler/problem/src/can.rs @@ -225,6 +225,9 @@ pub enum Problem { OverAppliedDbg { region: Region, }, + UnderAppliedTry { + region: Region, + }, FileProblem { filename: PathBuf, error: io::ErrorKind, @@ -339,6 +342,7 @@ impl Problem { Problem::OverAppliedCrash { .. } => RuntimeError, Problem::UnappliedDbg { .. } => RuntimeError, Problem::OverAppliedDbg { .. } => RuntimeError, + Problem::UnderAppliedTry { .. } => Warning, Problem::DefsOnlyUsedInRecursion(_, _) => Warning, Problem::FileProblem { .. } => Fatal, Problem::ReturnOutsideOfFunction { .. } => Warning, @@ -467,6 +471,7 @@ impl Problem { | Problem::UnappliedCrash { region } | Problem::OverAppliedDbg { region } | Problem::UnappliedDbg { region } + | Problem::UnderAppliedTry { region } | Problem::DefsOnlyUsedInRecursion(_, region) | Problem::ReturnOutsideOfFunction { region, .. } | Problem::StatementsAfterReturn { region } @@ -681,6 +686,7 @@ pub enum RuntimeError { }, NonFunctionHostedAnnotation(Region), + InvalidTupleIndex(Region), } impl RuntimeError { @@ -718,6 +724,7 @@ impl RuntimeError { | RuntimeError::InvalidFloat(_, region, _) | RuntimeError::InvalidInt(_, _, region, _) | RuntimeError::EmptySingleQuote(region) + | RuntimeError::InvalidTupleIndex(region) | RuntimeError::MultipleCharsInSingleQuote(region) | RuntimeError::DegenerateBranch(region) | RuntimeError::InvalidInterpolation(region) diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/empty_try_pnc.expr.formatted.roc b/crates/compiler/test_syntax/tests/snapshots/pass/empty_try_pnc.expr.formatted.roc new file mode 100644 index 00000000000..8ec95711213 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/empty_try_pnc.expr.formatted.roc @@ -0,0 +1 @@ +try() t \ No newline at end of file diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/empty_try_pnc.expr.result-ast b/crates/compiler/test_syntax/tests/snapshots/pass/empty_try_pnc.expr.result-ast new file mode 100644 index 00000000000..1a1d37209d1 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/empty_try_pnc.expr.result-ast @@ -0,0 +1,18 @@ +@0-6 SpaceAfter( + Apply( + @0-5 PncApply( + @0-3 Try, + [], + ), + [ + @5-6 Var { + module_name: "", + ident: "t", + }, + ], + Space, + ), + [ + Newline, + ], +) diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/empty_try_pnc.expr.roc b/crates/compiler/test_syntax/tests/snapshots/pass/empty_try_pnc.expr.roc new file mode 100644 index 00000000000..fce2183b856 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/empty_try_pnc.expr.roc @@ -0,0 +1 @@ +try()t diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/large_tuple_index.expr.formatted.roc b/crates/compiler/test_syntax/tests/snapshots/pass/large_tuple_index.expr.formatted.roc new file mode 100644 index 00000000000..3e5bd29a156 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/large_tuple_index.expr.formatted.roc @@ -0,0 +1 @@ +.18888888888888888888 \ No newline at end of file diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/large_tuple_index.expr.result-ast b/crates/compiler/test_syntax/tests/snapshots/pass/large_tuple_index.expr.result-ast new file mode 100644 index 00000000000..7167d88d01f --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/large_tuple_index.expr.result-ast @@ -0,0 +1,10 @@ +@0-21 SpaceAfter( + AccessorFunction( + TupleIndex( + "18888888888888888888", + ), + ), + [ + Newline, + ], +) diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/large_tuple_index.expr.roc b/crates/compiler/test_syntax/tests/snapshots/pass/large_tuple_index.expr.roc new file mode 100644 index 00000000000..20949e7f7e6 --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/large_tuple_index.expr.roc @@ -0,0 +1 @@ +.18888888888888888888 diff --git a/crates/compiler/test_syntax/tests/test_snapshots.rs b/crates/compiler/test_syntax/tests/test_snapshots.rs index 4de8a141503..a026e9c3350 100644 --- a/crates/compiler/test_syntax/tests/test_snapshots.rs +++ b/crates/compiler/test_syntax/tests/test_snapshots.rs @@ -436,6 +436,7 @@ mod test_snapshots { pass/empty_record_newline_assign.expr, pass/empty_record_update.expr, pass/empty_string.expr, + pass/empty_try_pnc.expr, pass/equals.expr, pass/equals_with_spaces.expr, pass/expect.expr, @@ -485,6 +486,7 @@ mod test_snapshots { pass/int_with_underscore.expr, pass/lambda_in_chain.expr, pass/lambda_indent.expr, + pass/large_tuple_index.expr, pass/list_closing_indent_not_enough.expr, pass/list_closing_same_indent_no_trailing_comma.expr, pass/list_closing_same_indent_with_trailing_comma.expr, diff --git a/crates/reporting/src/error/canonicalize.rs b/crates/reporting/src/error/canonicalize.rs index 5bbd625be18..d3fe2c8164a 100644 --- a/crates/reporting/src/error/canonicalize.rs +++ b/crates/reporting/src/error/canonicalize.rs @@ -1338,6 +1338,21 @@ pub fn can_problem<'b>( ]); title = "OVERAPPLIED DBG".to_string(); } + Problem::UnderAppliedTry { region } => { + doc = alloc.stack([ + alloc.concat([ + alloc.reflow("This "), + alloc.keyword("try"), + alloc.reflow(" has too few values given to it:"), + ]), + alloc.region(lines.convert_region(region), severity), + alloc.concat([ + alloc.keyword("try"), + alloc.reflow(" must be given exactly one value to try."), + ]), + ]); + title = "UNDERAPPLIED TRY".to_string(); + } Problem::FileProblem { filename, error } => { let report = to_file_problem_report(alloc, filename, error); doc = report.doc; @@ -2480,6 +2495,20 @@ fn pretty_runtime_error<'b>( title = NUMBER_UNDERFLOWS_SUFFIX; } + RuntimeError::InvalidTupleIndex(region) => { + doc = alloc.stack([ + alloc.concat([alloc + .reflow("This tuple accessor index is invalid:")]), + alloc.region(lines.convert_region(region), severity), + alloc.tip().append(alloc.concat([ + alloc.reflow("Note that .2 in Roc is a function that extracts the tuple element at index 2 from its argument."), + alloc.reflow("If you mean to create a floating point number, make sure it starts with a digit like 0.2"), + alloc.reflow("."), + ])), + ]); + + title = NUMBER_UNDERFLOWS_SUFFIX; + } RuntimeError::InvalidOptionalValue { field_name, field_region, From 61bc0b3464593aaa5c2765d9b9e6155153c9f367 Mon Sep 17 00:00:00 2001 From: Joshua Warner Date: Sat, 11 Jan 2025 19:49:47 -0800 Subject: [PATCH 4/6] Resolve TODO around handling non-plain strings --- crates/compiler/can/src/def.rs | 113 +++++++++++++----- crates/compiler/problem/src/can.rs | 5 + .../import_backslash_as_m.expr.formatted.roc | 2 + .../import_backslash_as_m.expr.result-ast | 55 +++++++++ .../pass/import_backslash_as_m.expr.roc | 2 + .../test_syntax/tests/test_snapshots.rs | 1 + crates/reporting/src/error/canonicalize.rs | 18 +++ 7 files changed, 168 insertions(+), 28 deletions(-) create mode 100644 crates/compiler/test_syntax/tests/snapshots/pass/import_backslash_as_m.expr.formatted.roc create mode 100644 crates/compiler/test_syntax/tests/snapshots/pass/import_backslash_as_m.expr.result-ast create mode 100644 crates/compiler/test_syntax/tests/snapshots/pass/import_backslash_as_m.expr.roc diff --git a/crates/compiler/can/src/def.rs b/crates/compiler/can/src/def.rs index 6cde3be2eba..8d1ece47803 100644 --- a/crates/compiler/can/src/def.rs +++ b/crates/compiler/can/src/def.rs @@ -2453,38 +2453,33 @@ fn canonicalize_pending_value_def<'a>( } } IngestedFile(loc_pattern, opt_loc_ann, path_literal) => { - let relative_path = - if let ast::StrLiteral::PlainLine(ingested_path) = path_literal.value { - ingested_path - } else { - todo!( - "Only plain strings are supported. Other cases should be made impossible here" - ); - }; - - let mut file_path: PathBuf = env.module_path.into(); - // Remove the header file name and push the new path. - file_path.pop(); - file_path.push(relative_path); + let expr = if let Some(relative_path) = extract_str_literal(env, path_literal) { + let mut file_path: PathBuf = env.module_path.into(); + // Remove the header file name and push the new path. + file_path.pop(); + file_path.push(relative_path); - let mut bytes = vec![]; + let mut bytes = vec![]; - let expr = match fs::File::open(&file_path) - .and_then(|mut file| file.read_to_end(&mut bytes)) - { - Ok(_) => Expr::IngestedFile(file_path.into(), Arc::new(bytes), var_store.fresh()), - Err(e) => { - env.problems.push(Problem::FileProblem { - filename: file_path.to_path_buf(), - error: e.kind(), - }); + match fs::File::open(&file_path).and_then(|mut file| file.read_to_end(&mut bytes)) { + Ok(_) => { + Expr::IngestedFile(file_path.into(), Arc::new(bytes), var_store.fresh()) + } + Err(e) => { + env.problems.push(Problem::FileProblem { + filename: file_path.to_path_buf(), + error: e.kind(), + }); - Expr::RuntimeError(RuntimeError::ReadIngestedFileError { - filename: file_path.to_path_buf(), - error: e.kind(), - region: path_literal.region, - }) + Expr::RuntimeError(RuntimeError::ReadIngestedFileError { + filename: file_path.to_path_buf(), + error: e.kind(), + region: path_literal.region, + }) + } } + } else { + Expr::RuntimeError(RuntimeError::IngestedFilePathError(path_literal.region)) }; let loc_expr = Loc::at(path_literal.region, expr); @@ -2539,6 +2534,68 @@ fn canonicalize_pending_value_def<'a>( output } +fn extract_str_literal<'a>( + env: &mut Env<'a>, + literal: Loc>, +) -> Option<&'a str> { + let relative_path = match literal.value { + ast::StrLiteral::PlainLine(ingested_path) => ingested_path, + ast::StrLiteral::Line(text) => { + let mut result_text = bumpalo::collections::String::new_in(env.arena); + if !extract_str_segments(env, text, &mut result_text) { + return None; + } + result_text.into_bump_str() + } + ast::StrLiteral::Block(lines) => { + let mut result_text = bumpalo::collections::String::new_in(env.arena); + for text in lines { + if !extract_str_segments(env, text, &mut result_text) { + return None; + } + } + result_text.into_bump_str() + } + }; + Some(relative_path) +} + +fn extract_str_segments<'a>( + env: &mut Env<'a>, + segments: &[ast::StrSegment<'a>], + result_text: &mut bumpalo::collections::String<'a>, +) -> bool { + for segment in segments.iter() { + match segment { + ast::StrSegment::Plaintext(t) => { + result_text.push_str(t); + } + ast::StrSegment::Unicode(t) => { + let hex_code: &str = t.value; + if let Some(c) = u32::from_str_radix(hex_code, 16) + .ok() + .and_then(char::from_u32) + { + result_text.push(c); + } else { + env.problem(Problem::InvalidUnicodeCodePt(t.region)); + return false; + } + } + ast::StrSegment::EscapedChar(c) => { + result_text.push(c.unescape()); + } + ast::StrSegment::Interpolated(e) => { + // TODO: maybe in the future we do want to allow building up the path with local bindings? + // This would require an interpreter tho; so for now we just disallow it. + env.problem(Problem::InterpolatedStringNotAllowed(e.region)); + return false; + } + } + } + true +} + // TODO trim down these arguments! #[allow(clippy::too_many_arguments)] #[allow(clippy::cognitive_complexity)] diff --git a/crates/compiler/problem/src/can.rs b/crates/compiler/problem/src/can.rs index 1a0532c9396..8ca2b0db968 100644 --- a/crates/compiler/problem/src/can.rs +++ b/crates/compiler/problem/src/can.rs @@ -263,6 +263,7 @@ pub enum Problem { field_region: Region, record_region: Region, }, + InterpolatedStringNotAllowed(Region), } #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -280,6 +281,7 @@ impl Problem { Problem::UnusedDef(_, _) => Warning, Problem::UnusedImport(_, _) => Warning, Problem::UnusedModuleImport(_, _) => Warning, + Problem::InterpolatedStringNotAllowed(_) => RuntimeError, Problem::ImportNameConflict { .. } => RuntimeError, Problem::ExplicitBuiltinImport(_, _) => Warning, Problem::ExplicitBuiltinTypeImport(_, _) => Warning, @@ -376,6 +378,7 @@ impl Problem { .. } | Problem::ExplicitBuiltinImport(_, region) + | Problem::InterpolatedStringNotAllowed(region) | Problem::ExplicitBuiltinTypeImport(_, region) | Problem::ImportShadowsSymbol { region, .. } | Problem::UnusedArgument(_, _, _, region) @@ -687,6 +690,7 @@ pub enum RuntimeError { NonFunctionHostedAnnotation(Region), InvalidTupleIndex(Region), + IngestedFilePathError(Region), } impl RuntimeError { @@ -725,6 +729,7 @@ impl RuntimeError { | RuntimeError::InvalidInt(_, _, region, _) | RuntimeError::EmptySingleQuote(region) | RuntimeError::InvalidTupleIndex(region) + | RuntimeError::IngestedFilePathError(region) | RuntimeError::MultipleCharsInSingleQuote(region) | RuntimeError::DegenerateBranch(region) | RuntimeError::InvalidInterpolation(region) diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/import_backslash_as_m.expr.formatted.roc b/crates/compiler/test_syntax/tests/snapshots/pass/import_backslash_as_m.expr.formatted.roc new file mode 100644 index 00000000000..9c6d0b7618b --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/import_backslash_as_m.expr.formatted.roc @@ -0,0 +1,2 @@ +import "\\" as m +e \ No newline at end of file diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/import_backslash_as_m.expr.result-ast b/crates/compiler/test_syntax/tests/snapshots/pass/import_backslash_as_m.expr.result-ast new file mode 100644 index 00000000000..8e894da8f4e --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/import_backslash_as_m.expr.result-ast @@ -0,0 +1,55 @@ +@0-16 SpaceAfter( + Defs( + Defs { + tags: [ + EitherIndex(2147483648), + ], + regions: [ + @0-14, + ], + space_before: [ + Slice { start: 0, length: 0 }, + ], + space_after: [ + Slice { start: 0, length: 0 }, + ], + spaces: [], + type_defs: [], + value_defs: [ + IngestedFileImport( + IngestedFileImport { + before_path: [], + path: @6-10 Line( + [ + EscapedChar( + Backslash, + ), + ], + ), + name: KeywordItem { + keyword: Spaces { + before: [], + item: ImportAsKeyword, + after: [], + }, + item: @13-14 "m", + }, + annotation: None, + }, + ), + ], + }, + @15-16 SpaceBefore( + Var { + module_name: "", + ident: "e", + }, + [ + Newline, + ], + ), + ), + [ + Newline, + ], +) diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/import_backslash_as_m.expr.roc b/crates/compiler/test_syntax/tests/snapshots/pass/import_backslash_as_m.expr.roc new file mode 100644 index 00000000000..65ec998d22d --- /dev/null +++ b/crates/compiler/test_syntax/tests/snapshots/pass/import_backslash_as_m.expr.roc @@ -0,0 +1,2 @@ +import"\\"as m +e diff --git a/crates/compiler/test_syntax/tests/test_snapshots.rs b/crates/compiler/test_syntax/tests/test_snapshots.rs index a026e9c3350..ddbbf4741c0 100644 --- a/crates/compiler/test_syntax/tests/test_snapshots.rs +++ b/crates/compiler/test_syntax/tests/test_snapshots.rs @@ -473,6 +473,7 @@ mod test_snapshots { pass/implements_not_keyword.expr, pass/implements_record_destructure.expr, pass/import.moduledefs, + pass/import_backslash_as_m.expr, pass/import_from_package.moduledefs, pass/import_in_closure_with_curlies_after.expr, pass/import_with_alias.moduledefs, diff --git a/crates/reporting/src/error/canonicalize.rs b/crates/reporting/src/error/canonicalize.rs index d3fe2c8164a..75668bf3c0e 100644 --- a/crates/reporting/src/error/canonicalize.rs +++ b/crates/reporting/src/error/canonicalize.rs @@ -67,6 +67,7 @@ const MISSING_EXCLAMATION: &str = "MISSING EXCLAMATION"; const UNNECESSARY_EXCLAMATION: &str = "UNNECESSARY EXCLAMATION"; const EMPTY_TUPLE_TYPE: &str = "EMPTY TUPLE TYPE"; const UNBOUND_TYPE_VARS_IN_AS: &str = "UNBOUND TYPE VARIABLES IN AS"; +const INTERPOLATED_STRING_NOT_ALLOWED: &str = "INTERPOLATED STRING NOT ALLOWED"; pub fn can_problem<'b>( alloc: &'b RocDocAllocator<'b>, @@ -1478,6 +1479,15 @@ pub fn can_problem<'b>( title = UNBOUND_TYPE_VARS_IN_AS.to_string(); } + Problem::InterpolatedStringNotAllowed(region) => { + doc = alloc.stack([ + alloc.reflow("Interpolated strings are not allowed here:"), + alloc.region(lines.convert_region(region), severity), + alloc.reflow(r#"Only plain strings like "foo" or "foo\n" are allowed."#), + ]); + + title = INTERPOLATED_STRING_NOT_ALLOWED.to_string(); + } }; Report { @@ -2249,6 +2259,14 @@ fn pretty_runtime_error<'b>( doc = report.doc; title = INGESTED_FILE_ERROR; } + RuntimeError::IngestedFilePathError(region) => { + doc = alloc.stack([ + alloc.reflow(r"I tried to read this file, but something about the path is wrong:"), + alloc.region(lines.convert_region(region), severity), + ]); + + title = INGESTED_FILE_ERROR; + } RuntimeError::InvalidPrecedence(_, _) => { // do nothing, reported with PrecedenceProblem unreachable!(); From 853dd5725b1ade0387bbb26ef6bdb635b4d5e739 Mon Sep 17 00:00:00 2001 From: Joshua Warner Date: Sat, 11 Jan 2025 20:13:07 -0800 Subject: [PATCH 5/6] Also handle large tuple indices in a.123 form --- crates/compiler/can/src/expr.rs | 14 +++++++----- .../pass/large_tuple_index.expr.formatted.roc | 2 +- .../pass/large_tuple_index.expr.result-ast | 22 +++++++++++++++---- .../snapshots/pass/large_tuple_index.expr.roc | 2 +- 4 files changed, 29 insertions(+), 11 deletions(-) diff --git a/crates/compiler/can/src/expr.rs b/crates/compiler/can/src/expr.rs index da1c3830372..c3e8b7bbfcd 100644 --- a/crates/compiler/can/src/expr.rs +++ b/crates/compiler/can/src/expr.rs @@ -1274,16 +1274,20 @@ pub fn canonicalize_expr<'a>( ast::Expr::TupleAccess(tuple_expr, field) => { let (loc_expr, output) = canonicalize_expr(env, var_store, scope, region, tuple_expr); - ( + let res = if let Ok(index) = field.parse() { TupleAccess { tuple_var: var_store.fresh(), ext_var: var_store.fresh(), elem_var: var_store.fresh(), loc_expr: Box::new(loc_expr), - index: field.parse().unwrap(), - }, - output, - ) + index, + } + } else { + let error = roc_problem::can::RuntimeError::InvalidTupleIndex(region); + env.problem(Problem::RuntimeError(error.clone())); + Expr::RuntimeError(error) + }; + (res, output) } ast::Expr::TrySuffix { .. } => internal_error!( "a Expr::TrySuffix expression was not completely removed in desugar_value_def_suffixed" diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/large_tuple_index.expr.formatted.roc b/crates/compiler/test_syntax/tests/snapshots/pass/large_tuple_index.expr.formatted.roc index 3e5bd29a156..52927e736a2 100644 --- a/crates/compiler/test_syntax/tests/snapshots/pass/large_tuple_index.expr.formatted.roc +++ b/crates/compiler/test_syntax/tests/snapshots/pass/large_tuple_index.expr.formatted.roc @@ -1 +1 @@ -.18888888888888888888 \ No newline at end of file +.18888888888888888888 + h.22222222222222222222 \ No newline at end of file diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/large_tuple_index.expr.result-ast b/crates/compiler/test_syntax/tests/snapshots/pass/large_tuple_index.expr.result-ast index 7167d88d01f..9770ab427f2 100644 --- a/crates/compiler/test_syntax/tests/snapshots/pass/large_tuple_index.expr.result-ast +++ b/crates/compiler/test_syntax/tests/snapshots/pass/large_tuple_index.expr.result-ast @@ -1,7 +1,21 @@ -@0-21 SpaceAfter( - AccessorFunction( - TupleIndex( - "18888888888888888888", +@0-46 SpaceAfter( + BinOps( + [ + ( + @0-21 AccessorFunction( + TupleIndex( + "18888888888888888888", + ), + ), + @22-23 Plus, + ), + ], + @24-46 TupleAccess( + Var { + module_name: "", + ident: "h", + }, + "22222222222222222222", ), ), [ diff --git a/crates/compiler/test_syntax/tests/snapshots/pass/large_tuple_index.expr.roc b/crates/compiler/test_syntax/tests/snapshots/pass/large_tuple_index.expr.roc index 20949e7f7e6..e46886f8500 100644 --- a/crates/compiler/test_syntax/tests/snapshots/pass/large_tuple_index.expr.roc +++ b/crates/compiler/test_syntax/tests/snapshots/pass/large_tuple_index.expr.roc @@ -1 +1 @@ -.18888888888888888888 +.18888888888888888888 + h.22222222222222222222 From 5ebd6e0884db65f2d099dfb0ebf255d0bdf80c2b Mon Sep 17 00:00:00 2001 From: Joshua Warner Date: Sat, 11 Jan 2025 20:23:42 -0800 Subject: [PATCH 6/6] Only conditionally fuzz canonicalize --- .../compiler/test_syntax/fuzz/fuzz_targets/fuzz_expr.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/crates/compiler/test_syntax/fuzz/fuzz_targets/fuzz_expr.rs b/crates/compiler/test_syntax/fuzz/fuzz_targets/fuzz_expr.rs index a83d5a6f0ce..068e20589d2 100644 --- a/crates/compiler/test_syntax/fuzz/fuzz_targets/fuzz_expr.rs +++ b/crates/compiler/test_syntax/fuzz/fuzz_targets/fuzz_expr.rs @@ -5,13 +5,19 @@ use roc_parse::ast::Malformed; use test_syntax::test_helpers::Input; fuzz_target!(|data: &[u8]| { + let canonicalize_fuzz_config = if cfg!(feature = "fuzz_canonicalize") { + Some(false) + } else { + None + }; + if let Ok(input) = std::str::from_utf8(data) { let input = Input::Expr(input); let arena = Bump::new(); let ast = input.parse_in(&arena); if let Ok(ast) = ast { if !ast.is_malformed() { - input.check_invariants(|_| (), true, Some(false)); + input.check_invariants(|_| (), true, canonicalize_fuzz_config); } } }