From b6fc9c14acba07761d7a65b77ac669c5d82429dc Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 4 Dec 2024 06:27:43 +0100 Subject: [PATCH 1/3] Better parser recovery for incomplete attributes --- crates/parser/src/grammar/attributes.rs | 10 ++- crates/parser/src/grammar/paths.rs | 19 ++-- crates/parser/src/parser.rs | 11 ++- crates/parser/test_data/generated/runner.rs | 2 + .../parser/err/0004_use_path_bad_segment.rast | 5 +- .../parser/err/0048_double_fish.rast | 10 ++- .../parser/inline/err/meta_recovery.rast | 86 +++++++++++++++++++ .../parser/inline/err/meta_recovery.rs | 6 ++ 8 files changed, 130 insertions(+), 19 deletions(-) create mode 100644 crates/parser/test_data/parser/inline/err/meta_recovery.rast create mode 100644 crates/parser/test_data/parser/inline/err/meta_recovery.rs diff --git a/crates/parser/src/grammar/attributes.rs b/crates/parser/src/grammar/attributes.rs index 82e4d66148..ccb556b2cc 100644 --- a/crates/parser/src/grammar/attributes.rs +++ b/crates/parser/src/grammar/attributes.rs @@ -36,6 +36,14 @@ fn attr(p: &mut Parser<'_>, inner: bool) { attr.complete(p, ATTR); } +// test_err meta_recovery +// #![] +// #![p = ] +// #![p::] +// #![p:: =] +// #![unsafe] +// #![unsafe =] + // test metas // #![simple_ident] // #![simple::path] @@ -63,7 +71,7 @@ pub(super) fn meta(p: &mut Parser<'_>) { if is_unsafe { p.expect(T!['(']); } - paths::use_path(p); + paths::attr_path(p); match p.current() { T![=] => { diff --git a/crates/parser/src/grammar/paths.rs b/crates/parser/src/grammar/paths.rs index 09db921803..057a691ef0 100644 --- a/crates/parser/src/grammar/paths.rs +++ b/crates/parser/src/grammar/paths.rs @@ -19,6 +19,10 @@ pub(super) fn use_path(p: &mut Parser<'_>) { path(p, Mode::Use); } +pub(super) fn attr_path(p: &mut Parser<'_>) { + path(p, Mode::Attr); +} + pub(crate) fn type_path(p: &mut Parser<'_>) { path(p, Mode::Type); } @@ -37,6 +41,7 @@ pub(crate) fn type_path_for_qualifier( #[derive(Clone, Copy, Eq, PartialEq)] enum Mode { Use, + Attr, Type, Expr, } @@ -93,12 +98,7 @@ fn path_segment(p: &mut Parser<'_>, mode: Mode, first: bool) { p.error("expected `::`"); } } else { - let empty = if first { - p.eat(T![::]); - false - } else { - true - }; + let mut empty = if first { !p.eat(T![::]) } else { true }; match p.current() { IDENT => { name_ref(p); @@ -114,10 +114,13 @@ fn path_segment(p: &mut Parser<'_>, mode: Mode, first: bool) { _ => { let recover_set = match mode { Mode::Use => items::ITEM_RECOVERY_SET, + Mode::Attr => { + items::ITEM_RECOVERY_SET.union(TokenSet::new(&[T![']'], T![=], T![#]])) + } Mode::Type => TYPE_PATH_SEGMENT_RECOVERY_SET, Mode::Expr => EXPR_PATH_SEGMENT_RECOVERY_SET, }; - p.err_recover("expected identifier", recover_set); + empty &= p.err_recover("expected identifier", recover_set); if empty { // test_err empty_segment // use crate::; @@ -132,7 +135,7 @@ fn path_segment(p: &mut Parser<'_>, mode: Mode, first: bool) { fn opt_path_type_args(p: &mut Parser<'_>, mode: Mode) { match mode { - Mode::Use => {} + Mode::Use | Mode::Attr => {} Mode::Type => { // test typepathfn_with_coloncolon // type F = Start::(Middle) -> (Middle)::End; diff --git a/crates/parser/src/parser.rs b/crates/parser/src/parser.rs index f6b3783d1c..8078532e0b 100644 --- a/crates/parser/src/parser.rs +++ b/crates/parser/src/parser.rs @@ -258,22 +258,25 @@ impl<'t> Parser<'t> { self.err_recover(message, TokenSet::EMPTY); } - /// Create an error node and consume the next token. - pub(crate) fn err_recover(&mut self, message: &str, recovery: TokenSet) { + /// Create an error node and consume the next token unless it is in the recovery set. + /// + /// Returns true if recovery kicked in. + pub(crate) fn err_recover(&mut self, message: &str, recovery: TokenSet) -> bool { if matches!(self.current(), T!['{'] | T!['}']) { self.error(message); - return; + return true; } if self.at_ts(recovery) { self.error(message); - return; + return true; } let m = self.start(); self.error(message); self.bump_any(); m.complete(self, ERROR); + false } fn do_bump(&mut self, kind: SyntaxKind, n_raw_tokens: u8) { diff --git a/crates/parser/test_data/generated/runner.rs b/crates/parser/test_data/generated/runner.rs index 62b381b668..3db8b51a4b 100644 --- a/crates/parser/test_data/generated/runner.rs +++ b/crates/parser/test_data/generated/runner.rs @@ -772,6 +772,8 @@ mod err { run_and_expect_errors("test_data/parser/inline/err/match_arms_recovery.rs"); } #[test] + fn meta_recovery() { run_and_expect_errors("test_data/parser/inline/err/meta_recovery.rs"); } + #[test] fn method_call_missing_argument_list() { run_and_expect_errors("test_data/parser/inline/err/method_call_missing_argument_list.rs"); } diff --git a/crates/parser/test_data/parser/err/0004_use_path_bad_segment.rast b/crates/parser/test_data/parser/err/0004_use_path_bad_segment.rast index 44e192a5fc..cf455934e9 100644 --- a/crates/parser/test_data/parser/err/0004_use_path_bad_segment.rast +++ b/crates/parser/test_data/parser/err/0004_use_path_bad_segment.rast @@ -9,7 +9,8 @@ SOURCE_FILE NAME_REF IDENT "foo" COLON2 "::" - ERROR - INT_NUMBER "92" + PATH_SEGMENT + ERROR + INT_NUMBER "92" SEMICOLON ";" error 9: expected identifier diff --git a/crates/parser/test_data/parser/err/0048_double_fish.rast b/crates/parser/test_data/parser/err/0048_double_fish.rast index 207a5c24df..7ef1eb98fc 100644 --- a/crates/parser/test_data/parser/err/0048_double_fish.rast +++ b/crates/parser/test_data/parser/err/0048_double_fish.rast @@ -39,8 +39,9 @@ SOURCE_FILE IDENT "lol" R_ANGLE ">" COLON2 "::" - ERROR - L_ANGLE "<" + PATH_SEGMENT + ERROR + L_ANGLE "<" TYPE_ARG PATH_TYPE PATH @@ -91,8 +92,9 @@ SOURCE_FILE IDENT "lol" R_ANGLE ">" COLON2 "::" - ERROR - L_ANGLE "<" + PATH_SEGMENT + ERROR + L_ANGLE "<" EXPR_STMT BIN_EXPR PATH_EXPR diff --git a/crates/parser/test_data/parser/inline/err/meta_recovery.rast b/crates/parser/test_data/parser/inline/err/meta_recovery.rast new file mode 100644 index 0000000000..c4bec849e7 --- /dev/null +++ b/crates/parser/test_data/parser/inline/err/meta_recovery.rast @@ -0,0 +1,86 @@ +SOURCE_FILE + ATTR + POUND "#" + BANG "!" + L_BRACK "[" + META + PATH + R_BRACK "]" + WHITESPACE "\n" + ATTR + POUND "#" + BANG "!" + L_BRACK "[" + META + PATH + PATH_SEGMENT + NAME_REF + IDENT "p" + WHITESPACE " " + EQ "=" + WHITESPACE " " + R_BRACK "]" + WHITESPACE "\n" + ATTR + POUND "#" + BANG "!" + L_BRACK "[" + META + PATH + PATH + PATH_SEGMENT + NAME_REF + IDENT "p" + COLON2 "::" + R_BRACK "]" + WHITESPACE "\n" + ATTR + POUND "#" + BANG "!" + L_BRACK "[" + META + PATH + PATH + PATH_SEGMENT + NAME_REF + IDENT "p" + COLON2 "::" + WHITESPACE " " + EQ "=" + R_BRACK "]" + WHITESPACE "\n" + ATTR + POUND "#" + BANG "!" + L_BRACK "[" + META + UNSAFE_KW "unsafe" + PATH + R_BRACK "]" + WHITESPACE "\n" + ATTR + POUND "#" + BANG "!" + L_BRACK "[" + META + UNSAFE_KW "unsafe" + WHITESPACE " " + PATH + EQ "=" + R_BRACK "]" + WHITESPACE "\n" +error 3: expected identifier +error 11: expected expression +error 11: expected expression +error 20: expected identifier +error 28: expected identifier +error 30: expected expression +error 30: expected expression +error 41: expected L_PAREN +error 41: expected identifier +error 41: expected R_PAREN +error 52: expected L_PAREN +error 53: expected identifier +error 54: expected expression +error 54: expected expression +error 54: expected R_PAREN diff --git a/crates/parser/test_data/parser/inline/err/meta_recovery.rs b/crates/parser/test_data/parser/inline/err/meta_recovery.rs new file mode 100644 index 0000000000..51d30adf8b --- /dev/null +++ b/crates/parser/test_data/parser/inline/err/meta_recovery.rs @@ -0,0 +1,6 @@ +#![] +#![p = ] +#![p::] +#![p:: =] +#![unsafe] +#![unsafe =] From caba872f8883a4ab7f39ba5736ac147d98412fd7 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 4 Dec 2024 06:49:27 +0100 Subject: [PATCH 2/3] fix: Don't create empty path nodes --- crates/parser/src/grammar.rs | 4 ++-- crates/parser/src/grammar/generic_args.rs | 4 ++-- crates/parser/src/grammar/paths.rs | 23 +++++++++++++------ crates/parser/src/parser.rs | 8 +++---- .../err/crate_visibility_empty_recover.rast | 6 +---- .../parser/inline/err/meta_recovery.rast | 5 +--- 6 files changed, 26 insertions(+), 24 deletions(-) diff --git a/crates/parser/src/grammar.rs b/crates/parser/src/grammar.rs index a50a2182a7..c402c49855 100644 --- a/crates/parser/src/grammar.rs +++ b/crates/parser/src/grammar.rs @@ -242,7 +242,7 @@ fn opt_visibility(p: &mut Parser<'_>, in_tuple_field: bool) -> bool { // struct MyStruct(pub ()); if !(in_tuple_field && matches!(p.nth(1), T![ident] | T![')'])) { p.bump(T!['(']); - paths::use_path(p); + paths::vis_path(p); p.expect(T![')']); } } @@ -252,7 +252,7 @@ fn opt_visibility(p: &mut Parser<'_>, in_tuple_field: bool) -> bool { T![in] => { p.bump(T!['(']); p.bump(T![in]); - paths::use_path(p); + paths::vis_path(p); p.expect(T![')']); } _ => {} diff --git a/crates/parser/src/grammar/generic_args.rs b/crates/parser/src/grammar/generic_args.rs index c62c8a9d3f..77379ef147 100644 --- a/crates/parser/src/grammar/generic_args.rs +++ b/crates/parser/src/grammar/generic_args.rs @@ -168,10 +168,10 @@ pub(super) fn const_arg_expr(p: &mut Parser<'_>) { expressions::literal(p); lm.complete(p, PREFIX_EXPR); } - _ if paths::is_use_path_start(p) => { + _ if paths::is_path_start(p) => { // This shouldn't be hit by `const_arg` let lm = p.start(); - paths::use_path(p); + paths::expr_path(p); lm.complete(p, PATH_EXPR); } _ => { diff --git a/crates/parser/src/grammar/paths.rs b/crates/parser/src/grammar/paths.rs index 057a691ef0..15b3529642 100644 --- a/crates/parser/src/grammar/paths.rs +++ b/crates/parser/src/grammar/paths.rs @@ -19,6 +19,10 @@ pub(super) fn use_path(p: &mut Parser<'_>) { path(p, Mode::Use); } +pub(super) fn vis_path(p: &mut Parser<'_>) { + path(p, Mode::Vis); +} + pub(super) fn attr_path(p: &mut Parser<'_>) { path(p, Mode::Attr); } @@ -44,13 +48,17 @@ enum Mode { Attr, Type, Expr, + Vis, } -fn path(p: &mut Parser<'_>, mode: Mode) { +fn path(p: &mut Parser<'_>, mode: Mode) -> Option { let path = p.start(); - path_segment(p, mode, true); + if path_segment(p, mode, true).is_none() { + path.abandon(p); + return None; + } let qual = path.complete(p, PATH); - path_for_qualifier(p, mode, qual); + Some(path_for_qualifier(p, mode, qual)) } fn path_for_qualifier( @@ -76,7 +84,7 @@ const EXPR_PATH_SEGMENT_RECOVERY_SET: TokenSet = items::ITEM_RECOVERY_SET.union(TokenSet::new(&[T![')'], T![,], T![let]])); const TYPE_PATH_SEGMENT_RECOVERY_SET: TokenSet = types::TYPE_RECOVERY_SET; -fn path_segment(p: &mut Parser<'_>, mode: Mode, first: bool) { +fn path_segment(p: &mut Parser<'_>, mode: Mode, first: bool) -> Option { let m = p.start(); // test qual_paths // type X = ::Output; @@ -117,6 +125,7 @@ fn path_segment(p: &mut Parser<'_>, mode: Mode, first: bool) { Mode::Attr => { items::ITEM_RECOVERY_SET.union(TokenSet::new(&[T![']'], T![=], T![#]])) } + Mode::Vis => items::ITEM_RECOVERY_SET.union(TokenSet::new(&[T![')']])), Mode::Type => TYPE_PATH_SEGMENT_RECOVERY_SET, Mode::Expr => EXPR_PATH_SEGMENT_RECOVERY_SET, }; @@ -125,17 +134,17 @@ fn path_segment(p: &mut Parser<'_>, mode: Mode, first: bool) { // test_err empty_segment // use crate::; m.abandon(p); - return; + return None; } } }; } - m.complete(p, PATH_SEGMENT); + Some(m.complete(p, PATH_SEGMENT)) } fn opt_path_type_args(p: &mut Parser<'_>, mode: Mode) { match mode { - Mode::Use | Mode::Attr => {} + Mode::Use | Mode::Attr | Mode::Vis => {} Mode::Type => { // test typepathfn_with_coloncolon // type F = Start::(Middle) -> (Middle)::End; diff --git a/crates/parser/src/parser.rs b/crates/parser/src/parser.rs index 8078532e0b..75a75f601c 100644 --- a/crates/parser/src/parser.rs +++ b/crates/parser/src/parser.rs @@ -327,10 +327,10 @@ impl Marker { self.bomb.defuse(); let idx = self.pos as usize; if idx == p.events.len() - 1 { - match p.events.pop() { - Some(Event::Start { kind: TOMBSTONE, forward_parent: None }) => (), - _ => unreachable!(), - } + assert!(matches!( + p.events.pop(), + Some(Event::Start { kind: TOMBSTONE, forward_parent: None }) + )); } } } diff --git a/crates/parser/test_data/parser/inline/err/crate_visibility_empty_recover.rast b/crates/parser/test_data/parser/inline/err/crate_visibility_empty_recover.rast index 0fe4ca42d7..681ca6b6e0 100644 --- a/crates/parser/test_data/parser/inline/err/crate_visibility_empty_recover.rast +++ b/crates/parser/test_data/parser/inline/err/crate_visibility_empty_recover.rast @@ -3,10 +3,7 @@ SOURCE_FILE VISIBILITY PUB_KW "pub" L_PAREN "(" - PATH - PATH_SEGMENT - ERROR - R_PAREN ")" + R_PAREN ")" WHITESPACE " " STRUCT_KW "struct" WHITESPACE " " @@ -15,4 +12,3 @@ SOURCE_FILE SEMICOLON ";" WHITESPACE "\n" error 4: expected identifier -error 5: expected R_PAREN diff --git a/crates/parser/test_data/parser/inline/err/meta_recovery.rast b/crates/parser/test_data/parser/inline/err/meta_recovery.rast index c4bec849e7..926dd50fc8 100644 --- a/crates/parser/test_data/parser/inline/err/meta_recovery.rast +++ b/crates/parser/test_data/parser/inline/err/meta_recovery.rast @@ -4,7 +4,6 @@ SOURCE_FILE BANG "!" L_BRACK "[" META - PATH R_BRACK "]" WHITESPACE "\n" ATTR @@ -55,7 +54,6 @@ SOURCE_FILE L_BRACK "[" META UNSAFE_KW "unsafe" - PATH R_BRACK "]" WHITESPACE "\n" ATTR @@ -65,7 +63,6 @@ SOURCE_FILE META UNSAFE_KW "unsafe" WHITESPACE " " - PATH EQ "=" R_BRACK "]" WHITESPACE "\n" @@ -80,7 +77,7 @@ error 41: expected L_PAREN error 41: expected identifier error 41: expected R_PAREN error 52: expected L_PAREN -error 53: expected identifier +error 52: expected identifier error 54: expected expression error 54: expected expression error 54: expected R_PAREN From 83f5150978d58614e0ba7c40c2a1e75c9c7fda1b Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 4 Dec 2024 07:03:01 +0100 Subject: [PATCH 3/3] Update mbe test output --- crates/hir-def/src/macro_expansion_tests/mbe.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/hir-def/src/macro_expansion_tests/mbe.rs b/crates/hir-def/src/macro_expansion_tests/mbe.rs index d568f6faa7..5c03fad613 100644 --- a/crates/hir-def/src/macro_expansion_tests/mbe.rs +++ b/crates/hir-def/src/macro_expansion_tests/mbe.rs @@ -1759,8 +1759,9 @@ fn f() { // NAME_REF@6..7 // IDENT@6..7 "K" // COLON2@7..9 "::" -// ERROR@9..10 -// L_PAREN@9..10 "(" +// PATH_SEGMENT@9..10 +// ERROR@9..10 +// L_PAREN@9..10 "(" // EXPR_STMT@10..16 // CALL_EXPR@10..16 // PATH_EXPR@10..11