From aac75c6a25fbfaea2b1eb30194f833dcefc6819a Mon Sep 17 00:00:00 2001 From: Folkert Date: Thu, 4 Feb 2021 21:34:28 +0100 Subject: [PATCH 1/4] factor out backtracking in def parsing --- compiler/parse/src/expr.rs | 145 ++++++++++++++++--------------------- 1 file changed, 64 insertions(+), 81 deletions(-) diff --git a/compiler/parse/src/expr.rs b/compiler/parse/src/expr.rs index 32870c6f5e..eb568a6d66 100644 --- a/compiler/parse/src/expr.rs +++ b/compiler/parse/src/expr.rs @@ -500,61 +500,82 @@ fn equals_for_def<'a>() -> impl Parser<'a, ()> { /// * A type annotation /// * A type annotation followed on the next line by a pattern, an `=`, and an expression pub fn def<'a>(min_indent: u16) -> impl Parser<'a, Def<'a>> { - // we should fix this backtracking! + let indented_more = min_indent + 1; + + enum DefKind { + DefColon, + DefEqual, + } + + let def_colon_or_equals = one_of![ + map!(equals_for_def(), |_| DefKind::DefEqual), + map!(ascii_char(b':'), |_| DefKind::DefColon) + ]; + attempt( Attempting::Def, - map_with_arena!( - either!(backtrackable(annotated_body(min_indent)), body(min_indent)), - to_def + then( + backtrackable(and!(pattern(min_indent), def_colon_or_equals)), + move |arena, state, _progress, (loc_pattern, def_kind)| match def_kind { + DefKind::DefColon => { + // Spaces after the ':' (at a normal indentation level) and then the type. + // The type itself must be indented more than the pattern and ':' + let (_, ann_type, state) = + space0_before(type_annotation::located(indented_more), min_indent) + .parse(arena, state)?; + + // see if there is a definition (assuming the preceding characters were a type + // annotation + let (_, opt_rest, state) = optional(and!( + spaces_then_comment_or_newline(), + body_at_indent(min_indent) + )) + .parse(arena, state)?; + + let def = match opt_rest { + None => annotation_or_alias( + arena, + &loc_pattern.value, + loc_pattern.region, + ann_type, + ), + Some((opt_comment, (body_pattern, body_expr))) => Def::AnnotatedBody { + ann_pattern: arena.alloc(loc_pattern), + ann_type: arena.alloc(ann_type), + comment: opt_comment, + body_pattern: arena.alloc(body_pattern), + body_expr: arena.alloc(body_expr), + }, + }; + + Ok((MadeProgress, def, state)) + } + DefKind::DefEqual => { + // Spaces after the '=' (at a normal indentation level) and then the expr. + // The expr itself must be indented more than the pattern and '=' + let (_, body_expr, state) = space0_before( + loc!(move |arena, state| { parse_expr(indented_more, arena, state) }), + min_indent, + ) + .parse(arena, state)?; + + Ok(( + MadeProgress, + Def::Body(arena.alloc(loc_pattern), arena.alloc(body_expr)), + state, + )) + } + }, ), ) } -fn to_def<'a>( - arena: &'a Bump, - ann_body_or_body: Either, Body<'a>>, -) -> Def<'a> { - match ann_body_or_body { - Either::First(((ann_pattern, ann_type), None)) => { - annotation_or_alias(arena, &ann_pattern.value, ann_pattern.region, ann_type) - } - Either::First(( - (ann_pattern, ann_type), - Some((opt_comment, (body_pattern, body_expr))), - )) => Def::AnnotatedBody { - ann_pattern: arena.alloc(ann_pattern), - ann_type: arena.alloc(ann_type), - comment: opt_comment, - body_pattern: arena.alloc(body_pattern), - body_expr: arena.alloc(body_expr), - }, - Either::Second((body_pattern, body_expr)) => { - Def::Body(arena.alloc(body_pattern), arena.alloc(body_expr)) - } - } -} - // PARSER HELPERS fn pattern<'a>(min_indent: u16) -> impl Parser<'a, Located>> { space0_after(loc_closure_param(min_indent), min_indent) } -fn annotation<'a>( - min_indent: u16, -) -> impl Parser<'a, (Located>, Located>)> { - let indented_more = min_indent + 1; - and!( - pattern(min_indent), - skip_first!( - ascii_char(b':'), - // Spaces after the ':' (at a normal indentation level) and then the type. - // The type itself must be indented more than the pattern and ':' - space0_before(type_annotation::located(indented_more), min_indent) - ) - ) -} - fn spaces_then_comment_or_newline<'a>() -> impl Parser<'a, Option<&'a str>> { skip_first!( zero_or_more!(ascii_char(b' ')), @@ -570,29 +591,6 @@ fn spaces_then_comment_or_newline<'a>() -> impl Parser<'a, Option<&'a str>> { type Body<'a> = (Located>, Located>); -fn body<'a>(min_indent: u16) -> impl Parser<'a, Body<'a>> { - let indented_more = min_indent + 1; - and!( - // this backtrackable is required for the case - // - // i = 64 - // - // i - // - // so the final i is not considered the start of a def - backtrackable(pattern(min_indent)), - skip_first!( - equals_for_def(), - // Spaces after the '=' (at a normal indentation level) and then the expr. - // The expr itself must be indented more than the pattern and '=' - space0_before( - loc!(move |arena, state| { parse_expr(indented_more, arena, state) }), - min_indent, - ) - ) - ) -} - fn body_at_indent<'a>(indent_level: u16) -> impl Parser<'a, Body<'a>> { let indented_more = indent_level + 1; and!( @@ -609,21 +607,6 @@ fn body_at_indent<'a>(indent_level: u16) -> impl Parser<'a, Body<'a>> { ) } -type AnnotationOrAnnotatedBody<'a> = ( - (Located>, Located>), - Option<(Option<&'a str>, Body<'a>)>, -); - -fn annotated_body<'a>(min_indent: u16) -> impl Parser<'a, AnnotationOrAnnotatedBody<'a>> { - and!( - annotation(min_indent), - optional(and!( - spaces_then_comment_or_newline(), - body_at_indent(min_indent) - )) - ) -} - fn annotation_or_alias<'a>( arena: &'a Bump, pattern: &Pattern<'a>, From 34c3a4147a37fd80004e49ce4cc41a2aecf081bc Mon Sep 17 00:00:00 2001 From: Folkert Date: Thu, 4 Feb 2021 23:26:59 +0100 Subject: [PATCH 2/4] add some bad parse error messages to tests, to track progress --- compiler/reporting/tests/test_reporting.rs | 85 ++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/compiler/reporting/tests/test_reporting.rs b/compiler/reporting/tests/test_reporting.rs index 2e1972e16e..e22e5f2a29 100644 --- a/compiler/reporting/tests/test_reporting.rs +++ b/compiler/reporting/tests/test_reporting.rs @@ -181,6 +181,12 @@ mod test_reporting { list_reports(&arena, src, &mut buf, callback); + if buf != expected_rendering { + for line in buf.split("\n") { + println!(" {}", line); + } + } + assert_eq!(buf, expected_rendering); } @@ -3995,4 +4001,83 @@ mod test_reporting { ), ) } + + #[test] + fn type_annotation_dubble_colon() { + report_problem_as( + indoc!( + r#" + f :: I64 + f = 42 + + f + "# + ), + indoc!( + r#" + ── PARSE PROBLEM ─────────────────────────────────────────────────────────────── + + Unexpected token while parsing a definition: + + 1│ f :: I64 + ^ + "# + ), + ) + } + + #[test] + fn double_equals_in_def() { + // NOTE: VERY BAD ERROR MESSAGE + // + // looks like `x y` are considered argument to the add, even though they are + // on a lower indentation level + report_problem_as( + indoc!( + r#" + x = 3 + y = + x == 5 + Num.add 1 2 + + x y + "# + ), + indoc!( + r#" + ── TOO MANY ARGS ─────────────────────────────────────────────────────────────── + + The `add` function expects 2 arguments, but it got 4 instead: + + 4│ Num.add 1 2 + ^^^^^^^ + + Are there any missing commas? Or missing parentheses? + "# + ), + ) + } + + #[test] + fn invalid_operator() { + // NOTE: VERY BAD ERROR MESSAGE + report_problem_as( + indoc!( + r#" + main = + 5 ** 3 + "# + ), + indoc!( + r#" + ── PARSE PROBLEM ─────────────────────────────────────────────────────────────── + + Unexpected token here: + + 2│ 5 ** 3 + ^ + "# + ), + ) + } } From ae09b0b31194c0d09b81e7864db711c5b208f5fb Mon Sep 17 00:00:00 2001 From: Folkert Date: Thu, 4 Feb 2021 23:27:19 +0100 Subject: [PATCH 3/4] optimize `=` symbol checking --- compiler/parse/src/expr.rs | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/compiler/parse/src/expr.rs b/compiler/parse/src/expr.rs index eb568a6d66..7d530f5654 100644 --- a/compiler/parse/src/expr.rs +++ b/compiler/parse/src/expr.rs @@ -487,10 +487,25 @@ pub fn assigned_pattern_field_to_pattern<'a>( /// The '=' used in a def can't be followed by another '=' (or else it's actually /// an "==") and also it can't be followed by '>' (or else it's actually an "=>") fn equals_for_def<'a>() -> impl Parser<'a, ()> { - not_followed_by( - ascii_char(b'='), - one_of!(ascii_char(b'='), ascii_char(b'>')), - ) + |arena, state: State<'a>| match state.bytes.get(0) { + Some(b'=') => match state.bytes.get(1) { + Some(b'=') | Some(b'>') => Err(( + NoProgress, + Bag::from_state(arena, &state, FailReason::ConditionFailed), + state, + )), + _ => { + let state = state.advance_without_indenting(arena, 1)?; + + Ok((MadeProgress, (), state)) + } + }, + _ => Err(( + NoProgress, + Bag::from_state(arena, &state, FailReason::ConditionFailed), + state, + )), + } } /// A definition, consisting of one of these: @@ -515,7 +530,14 @@ pub fn def<'a>(min_indent: u16) -> impl Parser<'a, Def<'a>> { attempt( Attempting::Def, then( - backtrackable(and!(pattern(min_indent), def_colon_or_equals)), + // backtrackable because + // + // i = 0 + // i + // + // on the last line, we parse a pattern `i`, but it's not actually a def, so need to + // backtrack + and!(backtrackable(pattern(min_indent)), def_colon_or_equals), move |arena, state, _progress, (loc_pattern, def_kind)| match def_kind { DefKind::DefColon => { // Spaces after the ':' (at a normal indentation level) and then the type. From da284861846e94194556b599c593b0ec9ebfb25e Mon Sep 17 00:00:00 2001 From: Folkert Date: Thu, 4 Feb 2021 23:28:33 +0100 Subject: [PATCH 4/4] disable test helper --- compiler/reporting/tests/test_reporting.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/compiler/reporting/tests/test_reporting.rs b/compiler/reporting/tests/test_reporting.rs index e22e5f2a29..70d03b840b 100644 --- a/compiler/reporting/tests/test_reporting.rs +++ b/compiler/reporting/tests/test_reporting.rs @@ -181,9 +181,12 @@ mod test_reporting { list_reports(&arena, src, &mut buf, callback); - if buf != expected_rendering { - for line in buf.split("\n") { - println!(" {}", line); + // convenient to copy-paste the generated message + if true { + if buf != expected_rendering { + for line in buf.split("\n") { + println!(" {}", line); + } } }