From b50f15b47cdbce4949bf7e38a9da7cb75eb8f848 Mon Sep 17 00:00:00 2001 From: Adam Welc Date: Tue, 9 Apr 2024 19:04:32 -0700 Subject: [PATCH 1/8] [move-compiler] Added dot chain parsing resilience --- .../crates/move-compiler/src/parser/syntax.rs | 76 +++++++++---------- .../tests/move_2024/parser/dot_incomplete.exp | 45 +++++++++++ .../move_2024/parser/dot_incomplete.move | 19 +++++ 3 files changed, 99 insertions(+), 41 deletions(-) create mode 100644 external-crates/move/crates/move-compiler/tests/move_2024/parser/dot_incomplete.exp create mode 100644 external-crates/move/crates/move-compiler/tests/move_2024/parser/dot_incomplete.move diff --git a/external-crates/move/crates/move-compiler/src/parser/syntax.rs b/external-crates/move/crates/move-compiler/src/parser/syntax.rs index b5edf3299790a..ba0a444d911cb 100644 --- a/external-crates/move/crates/move-compiler/src/parser/syntax.rs +++ b/external-crates/move/crates/move-compiler/src/parser/syntax.rs @@ -117,12 +117,6 @@ fn unexpected_token_error_( )) } -fn add_type_args_ambiguity_label(loc: Loc, mut diag: Box) -> Box { - const MSG: &str = "Perhaps you need a blank space before this '<' operator?"; - diag.add_secondary_label((loc, MSG)); - diag -} - // A macro for providing better diagnostics when we expect a specific token and find some other // pattern instead. For example, we can use this to handle the case when a const is missing its // type annotation as: @@ -885,9 +879,7 @@ fn parse_macro_opt_and_tyargs_opt( && (context.at_end(end_loc) || is_macro.is_some() || tyargs_whitespace_allowed) { let start_loc = context.tokens.start_loc(); - let loc = make_loc(context.tokens.file_hash(), start_loc, start_loc); - let tys_ = parse_optional_type_args(context) - .map_err(|diag| add_type_args_ambiguity_label(loc, diag))?; + let tys_ = parse_optional_type_args(context); let ty_loc = make_loc( context.tokens.file_hash(), start_loc, @@ -1626,12 +1618,7 @@ fn parse_term(context: &mut Context) -> Result> { consume_identifier(context.tokens, VECTOR_IDENT)?; let vec_end_loc = context.tokens.previous_end_loc(); let vec_loc = make_loc(context.tokens.file_hash(), start_loc, vec_end_loc); - let targs_start_loc = context.tokens.start_loc(); - let tys_opt = parse_optional_type_args(context).map_err(|diag| { - let targ_loc = - make_loc(context.tokens.file_hash(), targs_start_loc, targs_start_loc); - add_type_args_ambiguity_label(targ_loc, diag) - })?; + let tys_opt = parse_optional_type_args(context); let args_start_loc = context.tokens.start_loc(); let args_ = parse_comma_list( context, @@ -1961,7 +1948,7 @@ fn parse_name_exp(context: &mut Context) -> Result> { match context.tokens.peek() { _ if name.value.is_macro().is_some() => { // if in a macro, we must have a call - let rhs = parse_call_args(context)?; + let rhs = parse_call_args(context); Ok(Exp_::Call(name, rhs)) } @@ -1981,7 +1968,7 @@ fn parse_name_exp(context: &mut Context) -> Result> { // Call: "(" Comma ")" Tok::LParen => { debug_assert!(name.value.is_macro().is_none()); - let rhs = parse_call_args(context)?; + let rhs = parse_call_args(context); Ok(Exp_::Call(name, rhs)) } @@ -1991,8 +1978,8 @@ fn parse_name_exp(context: &mut Context) -> Result> { } // Parse the arguments to a call: "(" Comma ")" -fn parse_call_args(context: &mut Context) -> Result>, Box> { - ok_with_loc!( +fn parse_call_args(context: &mut Context) -> Spanned> { + let (loc, args) = with_loc!( context, parse_comma_list( context, @@ -2002,7 +1989,8 @@ fn parse_call_args(context: &mut Context) -> Result>, Box,)+ "}" @@ -2230,7 +2218,7 @@ fn parse_match_pattern(context: &mut Context) -> Result "]" -fn parse_index_args(context: &mut Context) -> Result>, Box> { +fn parse_index_args(context: &mut Context) -> Spanned> { let start_loc = context.tokens.start_loc(); let args = parse_comma_list( context, @@ -2241,12 +2229,7 @@ fn parse_index_args(context: &mut Context) -> Result>, Box Result> { fn parse_dot_or_index_chain(context: &mut Context) -> Result> { let start_loc = context.tokens.start_loc(); let mut lhs = parse_term(context)?; + let mut done_parsing = false; loop { + if done_parsing { + break; + } let exp = match context.tokens.peek() { Tok::Period => { - context.tokens.advance()?; + context.advance(); let loc = current_token_loc(context.tokens); match context.tokens.peek() { Tok::NumValue | Tok::NumTypedValue @@ -2572,7 +2559,7 @@ fn parse_dot_or_index_chain(context: &mut Context) -> Result { let contents = context.tokens.content(); - context.tokens.advance()?; + context.advance(); match parse_u8(contents) { Ok((parsed, NumberFormat::Decimal)) => { let field_access = Name::new(loc, format!("{parsed}").into()); @@ -2602,12 +2589,22 @@ fn parse_dot_or_index_chain(context: &mut Context) -> Result { - let n = parse_identifier(context)?; + let start_loc = context.tokens.start_loc(); + let n = match parse_identifier(context) { + Ok(id) => id, + Err(diag) => { + done_parsing = true; + context.add_diag(*diag); + let end_loc = context.tokens.previous_end_loc(); + let loc = make_loc(context.tokens.file_hash(), start_loc, end_loc); + sp(loc, Symbol::from("")) + } + }; if is_start_of_call_after_function_name(context, &n) { let call_start = context.tokens.start_loc(); let is_macro = if let Tok::Exclaim = context.tokens.peek() { let loc = current_token_loc(context.tokens); - context.tokens.advance()?; + context.advance(); Some(loc) } else { None @@ -2616,12 +2613,9 @@ fn parse_dot_or_index_chain(context: &mut Context) -> Result Result { - let index_args = parse_index_args(context)?; + let index_args = parse_index_args(context); Exp_::Index(Box::new(lhs), index_args) } @@ -2944,21 +2938,21 @@ fn parse_type_( // Parse an optional list of type arguments. // OptionalTypeArgs = '<' Comma ">" | -fn parse_optional_type_args(context: &mut Context) -> Result>, Box> { +fn parse_optional_type_args(context: &mut Context) -> Option> { if context.tokens.peek() == Tok::Less { context.stop_set.union(&TYPE_STOP_SET); - let list = Ok(Some(parse_comma_list( + let list = Some(parse_comma_list( context, Tok::Less, Tok::Greater, &TYPE_START_SET, parse_type, "a type", - ))); + )); context.stop_set.difference(&TYPE_STOP_SET); list } else { - Ok(None) + None } } diff --git a/external-crates/move/crates/move-compiler/tests/move_2024/parser/dot_incomplete.exp b/external-crates/move/crates/move-compiler/tests/move_2024/parser/dot_incomplete.exp new file mode 100644 index 0000000000000..cc5cb0e9435da --- /dev/null +++ b/external-crates/move/crates/move-compiler/tests/move_2024/parser/dot_incomplete.exp @@ -0,0 +1,45 @@ +error[E03010]: unbound field + ┌─ tests/move_2024/parser/dot_incomplete.move:12:21 + │ +12 │ let _tmp1 = s.; // incomplete with `;` (next line should parse) + │ ^^ Unbound field '' in 'a::m::AnotherStruct' + +error[E01002]: unexpected token + ┌─ tests/move_2024/parser/dot_incomplete.move:12:23 + │ +12 │ let _tmp1 = s.; // incomplete with `;` (next line should parse) + │ ^ + │ │ + │ Unexpected ';' + │ Expected an identifier + +error[E03010]: unbound field + ┌─ tests/move_2024/parser/dot_incomplete.move:13:21 + │ +13 │ let _tmp2 = s.another_field.; // incomplete with `;` (next line should parse) + │ ^^^^^^^^^^^^^^^^ Unbound field '' in 'a::m::SomeStruct' + +error[E01002]: unexpected token + ┌─ tests/move_2024/parser/dot_incomplete.move:13:37 + │ +13 │ let _tmp2 = s.another_field.; // incomplete with `;` (next line should parse) + │ ^ + │ │ + │ Unexpected ';' + │ Expected an identifier + +error[E03010]: unbound field + ┌─ tests/move_2024/parser/dot_incomplete.move:14:21 + │ +14 │ let _tmp3 = s.another_field. // incomplete without `;` (unexpected `let`) + │ ^^^^^^^^^^^^^^^^ Unbound field '' in 'a::m::SomeStruct' + +error[E01002]: unexpected token + ┌─ tests/move_2024/parser/dot_incomplete.move:15:9 + │ +15 │ let _tmp4 = s; + │ ^^^ + │ │ + │ Unexpected 'let' + │ Expected an identifier + diff --git a/external-crates/move/crates/move-compiler/tests/move_2024/parser/dot_incomplete.move b/external-crates/move/crates/move-compiler/tests/move_2024/parser/dot_incomplete.move new file mode 100644 index 0000000000000..e768bf77151c5 --- /dev/null +++ b/external-crates/move/crates/move-compiler/tests/move_2024/parser/dot_incomplete.move @@ -0,0 +1,19 @@ +module a::m { + + public struct SomeStruct has drop { + some_field: u64 + } + + public struct AnotherStruct has drop { + another_field: SomeStruct + } + + fun foo(s: AnotherStruct) { + let _tmp1 = s.; // incomplete with `;` (next line should parse) + let _tmp2 = s.another_field.; // incomplete with `;` (next line should parse) + let _tmp3 = s.another_field. // incomplete without `;` (unexpected `let`) + let _tmp4 = s; + } + + +} From 4fe5f5402b9bd74761da9dbba32732a31fe44f97 Mon Sep 17 00:00:00 2001 From: Adam Welc Date: Thu, 11 Apr 2024 10:51:19 -0700 Subject: [PATCH 2/8] Introduced partially parsed dot chain AST node --- .../move/crates/move-analyzer/src/symbols.rs | 97 +++++++++++++++++++ .../move-analyzer/tests/partial-dot/Move.toml | 9 ++ .../tests/partial-dot/sources/M1.move | 17 ++++ .../crates/move-compiler/src/expansion/ast.rs | 5 + .../move-compiler/src/expansion/translate.rs | 21 ++-- .../crates/move-compiler/src/naming/ast.rs | 5 + .../src/naming/resolve_use_funs.rs | 2 +- .../move-compiler/src/naming/translate.rs | 7 +- .../crates/move-compiler/src/parser/ast.rs | 6 ++ .../crates/move-compiler/src/parser/syntax.rs | 69 ++++++------- .../move-compiler/src/typing/macro_expand.rs | 4 +- .../move-compiler/src/typing/translate.rs | 20 +++- .../tests/move_2024/parser/dot_incomplete.exp | 27 ++---- .../move_2024/parser/dot_incomplete.move | 3 +- 14 files changed, 225 insertions(+), 67 deletions(-) create mode 100644 external-crates/move/crates/move-analyzer/tests/partial-dot/Move.toml create mode 100644 external-crates/move/crates/move-analyzer/tests/partial-dot/sources/M1.move diff --git a/external-crates/move/crates/move-analyzer/src/symbols.rs b/external-crates/move/crates/move-analyzer/src/symbols.rs index 8a4dae52ead5a..c881532a7c705 100644 --- a/external-crates/move/crates/move-analyzer/src/symbols.rs +++ b/external-crates/move/crates/move-analyzer/src/symbols.rs @@ -6791,3 +6791,100 @@ fn partial_function_test() { None, ); } + +#[test] +/// Tests if partial dot chains are symbolicated correctly. +fn partial_dot_test() { + let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + + path.push("tests/partial-dot"); + + let ide_files_layer: VfsPath = MemoryFS::new().into(); + let (symbols_opt, _) = get_symbols( + &mut BTreeMap::new(), + ide_files_layer, + path.as_path(), + LintLevel::None, + ) + .unwrap(); + let symbols = symbols_opt.unwrap(); + + let mut fpath = path.clone(); + fpath.push("sources/M1.move"); + let cpath = dunce::canonicalize(&fpath).unwrap(); + + let mod_symbols = symbols.file_use_defs.get(&cpath).unwrap(); + + // struct-typed first part of incomplete dot chain `s.;` + assert_use_def( + mod_symbols, + &symbols, + 1, + 10, + 20, + "M1.move", + 9, + 12, + "M1.move", + "s: PartialDot::M1::AnotherStruct", + Some((5, 18, "M1.move")), + ); + // struct-typed first part of incomplete dot chain `s.another_field.;` + assert_use_def( + mod_symbols, + &symbols, + 1, + 11, + 20, + "M1.move", + 9, + 12, + "M1.move", + "s: PartialDot::M1::AnotherStruct", + Some((5, 18, "M1.move")), + ); + // struct-typed second part of incomplete dot chain `s.another_field.;` + assert_use_def( + mod_symbols, + &symbols, + 2, + 11, + 22, + "M1.move", + 6, + 8, + "M1.move", + "PartialDot::M1::AnotherStruct\nanother_field: PartialDot::M1::SomeStruct", + Some((1, 18, "M1.move")), + ); + // struct-typed second part of incomplete dot chain `s.another_field.` (no `;` but followed by + // `let` on the next line) + assert_use_def( + mod_symbols, + &symbols, + 2, + 12, + 22, + "M1.move", + 6, + 8, + "M1.move", + "PartialDot::M1::AnotherStruct\nanother_field: PartialDot::M1::SomeStruct", + Some((1, 18, "M1.move")), + ); + // struct-typed first part of incomplete dot chain `s.` (no `;` but followed by `}` on the next + // line) + assert_use_def( + mod_symbols, + &symbols, + 1, + 14, + 20, + "M1.move", + 9, + 12, + "M1.move", + "s: PartialDot::M1::AnotherStruct", + Some((5, 18, "M1.move")), + ); +} diff --git a/external-crates/move/crates/move-analyzer/tests/partial-dot/Move.toml b/external-crates/move/crates/move-analyzer/tests/partial-dot/Move.toml new file mode 100644 index 0000000000000..66aeb5dcdcfa7 --- /dev/null +++ b/external-crates/move/crates/move-analyzer/tests/partial-dot/Move.toml @@ -0,0 +1,9 @@ +[package] +name = "PartialDot" +version = "0.0.1" + +[dependencies] +MoveStdlib = { local = "../../../move-stdlib/", addr_subst = { "std" = "0x1" } } + +[addresses] +PartialDot = "0xCAFE" diff --git a/external-crates/move/crates/move-analyzer/tests/partial-dot/sources/M1.move b/external-crates/move/crates/move-analyzer/tests/partial-dot/sources/M1.move new file mode 100644 index 0000000000000..aa9a868c04a83 --- /dev/null +++ b/external-crates/move/crates/move-analyzer/tests/partial-dot/sources/M1.move @@ -0,0 +1,17 @@ +module PartialDot::M1 { + public struct SomeStruct has drop { + some_field: u64 + } + + public struct AnotherStruct has drop { + another_field: SomeStruct + } + + fun foo(s: AnotherStruct) { + let _tmp1 = s.; + let _tmp2 = s.another_field.; + let _tmp3 = s.another_field. + let _tmp4 = s; // statement skipped due to unexpected `let` + let _tmp5 = s. + } +} diff --git a/external-crates/move/crates/move-compiler/src/expansion/ast.rs b/external-crates/move/crates/move-compiler/src/expansion/ast.rs index 22f3c5867912d..2d261314788b9 100644 --- a/external-crates/move/crates/move-compiler/src/expansion/ast.rs +++ b/external-crates/move/crates/move-compiler/src/expansion/ast.rs @@ -327,6 +327,7 @@ pub enum ExpDotted_ { Exp(Box), Dot(Box, Name), Index(Box, Spanned>), + DotUnresolved(Box), // dot where Name could not be parsed } pub type ExpDotted = Spanned; @@ -1734,6 +1735,10 @@ impl AstDebug for ExpDotted_ { w.comma(&rhs.value, |w, e| e.ast_debug(w)); w.write("]"); } + D::DotUnresolved(e) => { + e.ast_debug(w); + w.write("") + } } } } diff --git a/external-crates/move/crates/move-compiler/src/expansion/translate.rs b/external-crates/move/crates/move-compiler/src/expansion/translate.rs index 313c6c86d4776..6749373252b1a 100644 --- a/external-crates/move/crates/move-compiler/src/expansion/translate.rs +++ b/external-crates/move/crates/move-compiler/src/expansion/translate.rs @@ -2632,13 +2632,15 @@ fn exp(context: &mut Context, pe: Box) -> Box { EE::UnresolvedError } }, - pdotted_ @ PE::Dot(_, _) => match exp_dotted(context, Box::new(sp(loc, pdotted_))) { - Some(edotted) => EE::ExpDotted(E::DottedUsage::Use, edotted), - None => { - assert!(context.env().has_errors()); - EE::UnresolvedError + pdotted_ @ (PE::Dot(_, _) | PE::DotUnresolved(_)) => { + match exp_dotted(context, Box::new(sp(loc, pdotted_))) { + Some(edotted) => EE::ExpDotted(E::DottedUsage::Use, edotted), + None => { + assert!(context.env().has_errors()); + EE::UnresolvedError + } } - }, + } pdotted_ @ PE::Index(_, _) => { let cur_pkg = context.current_package(); @@ -2738,6 +2740,7 @@ fn exp_cast(context: &mut Context, in_parens: bool, plhs: Box, pty: P::T PE::DotCall(lhs, _, _, _, _) | PE::Dot(lhs, _) + | PE::DotUnresolved(lhs) | PE::Index(lhs, _) | PE::Borrow(_, lhs) | PE::Dereference(lhs) => ambiguous_cast(lhs), @@ -2854,7 +2857,7 @@ fn move_or_copy_path_(context: &mut Context, case: PathCase, pe: Box) -> return None; } } - E::ExpDotted_::Dot(_, _) | E::ExpDotted_::Index(_, _) => { + E::ExpDotted_::Dot(_, _) | E::ExpDotted_::DotUnresolved(_) | E::ExpDotted_::Index(_, _) => { let current_package = context.current_package(); context .env() @@ -2892,6 +2895,10 @@ fn exp_dotted(context: &mut Context, pdotted: Box) -> Option>(); EE::Index(lhs, sp(argloc, args)) } + PE::DotUnresolved(plhs) => { + let lhs = exp_dotted(context, plhs)?; + EE::DotUnresolved(lhs) + } pe_ => EE::Exp(exp(context, Box::new(sp(loc, pe_)))), }; Some(Box::new(sp(loc, edotted_))) diff --git a/external-crates/move/crates/move-compiler/src/naming/ast.rs b/external-crates/move/crates/move-compiler/src/naming/ast.rs index 4523ba4160e43..d3885e5fe8193 100644 --- a/external-crates/move/crates/move-compiler/src/naming/ast.rs +++ b/external-crates/move/crates/move-compiler/src/naming/ast.rs @@ -356,6 +356,7 @@ pub enum ExpDotted_ { Exp(Box), Dot(Box, Field), Index(Box, Spanned>), + DotUnresolved(Box), // dot where Field could not be parsed } pub type ExpDotted = Spanned; @@ -1882,6 +1883,10 @@ impl AstDebug for ExpDotted_ { w.comma(args, |w, e| e.ast_debug(w)); w.write(")"); } + D::DotUnresolved(e) => { + e.ast_debug(w); + w.write(".") + } } } } diff --git a/external-crates/move/crates/move-compiler/src/naming/resolve_use_funs.rs b/external-crates/move/crates/move-compiler/src/naming/resolve_use_funs.rs index d564acbf60f76..ae7238e0b0717 100644 --- a/external-crates/move/crates/move-compiler/src/naming/resolve_use_funs.rs +++ b/external-crates/move/crates/move-compiler/src/naming/resolve_use_funs.rs @@ -400,7 +400,7 @@ fn exp(context: &mut Context, sp!(_, e_): &mut N::Exp) { fn exp_dotted(context: &mut Context, sp!(_, ed_): &mut N::ExpDotted) { match ed_ { N::ExpDotted_::Exp(e) => exp(context, e), - N::ExpDotted_::Dot(ed, _) => exp_dotted(context, ed), + N::ExpDotted_::Dot(ed, _) | N::ExpDotted_::DotUnresolved(ed) => exp_dotted(context, ed), N::ExpDotted_::Index(ed, sp!(_, es)) => { exp_dotted(context, ed); for e in es { diff --git a/external-crates/move/crates/move-compiler/src/naming/translate.rs b/external-crates/move/crates/move-compiler/src/naming/translate.rs index 049ec4e26cffa..d253af6b2dc01 100644 --- a/external-crates/move/crates/move-compiler/src/naming/translate.rs +++ b/external-crates/move/crates/move-compiler/src/naming/translate.rs @@ -2445,6 +2445,9 @@ fn dotted(context: &mut Context, edot: E::ExpDotted) -> Option { } } E::ExpDotted_::Dot(d, f) => N::ExpDotted_::Dot(Box::new(dotted(context, *d)?), Field(f)), + E::ExpDotted_::DotUnresolved(d) => { + N::ExpDotted_::DotUnresolved(Box::new(dotted(context, *d)?)) + } E::ExpDotted_::Index(inner, args) => { let args = call_args(context, args); let inner = Box::new(dotted(context, *inner)?); @@ -3652,7 +3655,9 @@ fn remove_unused_bindings_exp_dotted( ) { match ed_ { N::ExpDotted_::Exp(e) => remove_unused_bindings_exp(context, used, e), - N::ExpDotted_::Dot(ed, _) => remove_unused_bindings_exp_dotted(context, used, ed), + N::ExpDotted_::Dot(ed, _) | N::ExpDotted_::DotUnresolved(ed) => { + remove_unused_bindings_exp_dotted(context, used, ed) + } N::ExpDotted_::Index(ed, sp!(_, es)) => { for e in es { remove_unused_bindings_exp(context, used, e); diff --git a/external-crates/move/crates/move-compiler/src/parser/ast.rs b/external-crates/move/crates/move-compiler/src/parser/ast.rs index c8af8f9bf9776..2c5d25aaff830 100644 --- a/external-crates/move/crates/move-compiler/src/parser/ast.rs +++ b/external-crates/move/crates/move-compiler/src/parser/ast.rs @@ -624,6 +624,8 @@ pub enum Exp_ { // Internal node marking an error was added to the error list // This is here so the pass can continue even when an error is hit UnresolvedError, + // e.X (where X is not a valid tok after dot and cannot be parsed) + DotUnresolved(Box), } pub type Exp = Spanned; @@ -2103,6 +2105,10 @@ impl AstDebug for Exp_ { w.write(&s.value); } E::UnresolvedError => w.write("_|_"), + E::DotUnresolved(e) => { + e.ast_debug(w); + w.write("."); + } } } } diff --git a/external-crates/move/crates/move-compiler/src/parser/syntax.rs b/external-crates/move/crates/move-compiler/src/parser/syntax.rs index ba0a444d911cb..dc6d21573cff0 100644 --- a/external-crates/move/crates/move-compiler/src/parser/syntax.rs +++ b/external-crates/move/crates/move-compiler/src/parser/syntax.rs @@ -1501,6 +1501,9 @@ fn parse_sequence(context: &mut Context) -> Result> { value: e.value, }); } else { + // we parsed a valid sequence - even though it should be followed by a + // semicolon, let's not drop it on the floor + seq.push(item); context.add_diag(*unexpected_token_error(context.tokens, "';'")); } break; @@ -1565,6 +1568,13 @@ fn parse_sequence(context: &mut Context) -> Result> { // sequence and proceed with parsing top-level definition (assume the second scenario). if !context.at_stop_set() || context.tokens.at(Tok::RBrace) { context.advance(); // consume (the RBrace) + if context.tokens.text.starts_with("module a::m {") { + eprintln!( + "CONSUME: {:?} DIAGS {}", + context.tokens.peek(), + context.env.count_diags() + ); + } } Ok((uses, seq, last_semicolon_loc, Box::new(eopt))) } @@ -2541,11 +2551,7 @@ fn parse_unary_exp(context: &mut Context) -> Result> { fn parse_dot_or_index_chain(context: &mut Context) -> Result> { let start_loc = context.tokens.start_loc(); let mut lhs = parse_term(context)?; - let mut done_parsing = false; loop { - if done_parsing { - break; - } let exp = match context.tokens.peek() { Tok::Period => { context.advance(); @@ -2588,39 +2594,34 @@ fn parse_dot_or_index_chain(context: &mut Context) -> Result { - let start_loc = context.tokens.start_loc(); - let n = match parse_identifier(context) { - Ok(id) => id, - Err(diag) => { - done_parsing = true; - context.add_diag(*diag); - let end_loc = context.tokens.previous_end_loc(); - let loc = make_loc(context.tokens.file_hash(), start_loc, end_loc); - sp(loc, Symbol::from("")) - } - }; - if is_start_of_call_after_function_name(context, &n) { - let call_start = context.tokens.start_loc(); - let is_macro = if let Tok::Exclaim = context.tokens.peek() { - let loc = current_token_loc(context.tokens); - context.advance(); - Some(loc) + _ => match parse_identifier(context) { + Err(diag) => { + context.add_diag(*diag); + Exp_::DotUnresolved(Box::new(lhs)) + } + Ok(n) => { + if is_start_of_call_after_function_name(context, &n) { + let call_start = context.tokens.start_loc(); + let is_macro = if let Tok::Exclaim = context.tokens.peek() { + let loc = current_token_loc(context.tokens); + context.advance(); + Some(loc) + } else { + None + }; + let mut tys = None; + if context.tokens.peek() == Tok::Less + && n.loc.end() as usize == call_start + { + tys = parse_optional_type_args(context); + } + let args = parse_call_args(context); + Exp_::DotCall(Box::new(lhs), n, is_macro, tys, args) } else { - None - }; - let mut tys = None; - if context.tokens.peek() == Tok::Less - && n.loc.end() as usize == call_start - { - tys = parse_optional_type_args(context); + Exp_::Dot(Box::new(lhs), n) } - let args = parse_call_args(context); - Exp_::DotCall(Box::new(lhs), n, is_macro, tys, args) - } else { - Exp_::Dot(Box::new(lhs), n) } - } + }, } } Tok::LBracket => { diff --git a/external-crates/move/crates/move-compiler/src/typing/macro_expand.rs b/external-crates/move/crates/move-compiler/src/typing/macro_expand.rs index 4c8a71b999931..0134c090fe211 100644 --- a/external-crates/move/crates/move-compiler/src/typing/macro_expand.rs +++ b/external-crates/move/crates/move-compiler/src/typing/macro_expand.rs @@ -630,7 +630,7 @@ fn recolor_exp(ctx: &mut Recolor, sp!(_, e_): &mut N::Exp) { fn recolor_exp_dotted(ctx: &mut Recolor, sp!(_, ed_): &mut N::ExpDotted) { match ed_ { N::ExpDotted_::Exp(e) => recolor_exp(ctx, e), - N::ExpDotted_::Dot(ed, _) => recolor_exp_dotted(ctx, ed), + N::ExpDotted_::Dot(ed, _) | N::ExpDotted_::DotUnresolved(ed) => recolor_exp_dotted(ctx, ed), N::ExpDotted_::Index(ed, sp!(_, es)) => { recolor_exp_dotted(ctx, ed); for e in es { @@ -1093,7 +1093,7 @@ fn builtin_function(context: &mut Context, sp!(_, bf_): &mut N::BuiltinFunction) fn exp_dotted(context: &mut Context, sp!(_, ed_): &mut N::ExpDotted) { match ed_ { N::ExpDotted_::Exp(e) => exp(context, e), - N::ExpDotted_::Dot(ed, _) => exp_dotted(context, ed), + N::ExpDotted_::Dot(ed, _) | N::ExpDotted_::DotUnresolved(ed) => exp_dotted(context, ed), N::ExpDotted_::Index(ed, sp!(_, es)) => { exp_dotted(context, ed); for e in es { diff --git a/external-crates/move/crates/move-compiler/src/typing/translate.rs b/external-crates/move/crates/move-compiler/src/typing/translate.rs index 3ee2a9f172687..7e0984b5bfec1 100644 --- a/external-crates/move/crates/move-compiler/src/typing/translate.rs +++ b/external-crates/move/crates/move-compiler/src/typing/translate.rs @@ -2977,6 +2977,7 @@ enum ExpDottedAccess { args: Spanned>, base_type: Type, /* base (non-ref) return type */ }, + UnresolvedError, } #[derive(Debug)] @@ -3005,6 +3006,7 @@ impl ExpDotted { match accessor { ExpDottedAccess::Field(_, ty) => ty.clone(), ExpDottedAccess::Index { base_type, .. } => base_type.clone(), + ExpDottedAccess::UnresolvedError => sp(Loc::invalid(), Type_::UnresolvedError), } } else { self.base_type.clone() @@ -3056,6 +3058,11 @@ fn process_exp_dotted( .push(ExpDottedAccess::Field(field, field_type)); inner } + N::ExpDotted_::DotUnresolved(ndot) => { + let mut inner = process_exp_dotted(context, Some("dot access"), *ndot); + inner.accessors.push(ExpDottedAccess::UnresolvedError); + inner + } N::ExpDotted_::Index(ndot, sp!(argloc, nargs_)) => { let mut inner = process_exp_dotted(context, Some("dot access"), *ndot); let inner_ty = inner.last_type(); @@ -3095,7 +3102,7 @@ fn exp_dotted_usage( let constraint_verb = match &ndotted.value { N::ExpDotted_::Exp(_) => None, _ if matches!(usage, DottedUsage::Borrow(_)) => Some("borrow"), - N::ExpDotted_::Dot(_, _) => Some("dot access"), + N::ExpDotted_::Dot(_, _) | N::ExpDotted_::DotUnresolved(_) => Some("dot access"), N::ExpDotted_::Index(_, _) => Some("index"), }; let edotted = process_exp_dotted(context, constraint_verb, ndotted); @@ -3333,7 +3340,8 @@ fn borrow_exp_dotted(context: &mut Context, mut_: bool, ed: ExpDotted) -> Box Box::new(base), }; - for accessor in accessors { + let num_accessors = accessors.len(); + for (idx, accessor) in accessors.into_iter().enumerate() { check_mut(context, loc, exp.ty.clone(), mut_); match accessor { ExpDottedAccess::Field(name, ty) => { @@ -3389,6 +3397,10 @@ fn borrow_exp_dotted(context: &mut Context, mut_: bool, ed: ExpDotted) -> Box { + assert!(idx == num_accessors - 1); + break; + } } } exp @@ -3417,6 +3429,10 @@ fn exp_dotted_to_owned(context: &mut Context, usage: DottedUsage, ed: ExpDotted) ExpDottedAccess::Index { base_type, .. } => { ("index result".to_string(), base_type.clone()) } + ExpDottedAccess::UnresolvedError => ( + "unresolved field".to_string(), + sp(Loc::invalid(), Type_::UnresolvedError), + ), } } else { context.env.add_diag(ice!(( diff --git a/external-crates/move/crates/move-compiler/tests/move_2024/parser/dot_incomplete.exp b/external-crates/move/crates/move-compiler/tests/move_2024/parser/dot_incomplete.exp index cc5cb0e9435da..540d2524d6394 100644 --- a/external-crates/move/crates/move-compiler/tests/move_2024/parser/dot_incomplete.exp +++ b/external-crates/move/crates/move-compiler/tests/move_2024/parser/dot_incomplete.exp @@ -1,9 +1,3 @@ -error[E03010]: unbound field - ┌─ tests/move_2024/parser/dot_incomplete.move:12:21 - │ -12 │ let _tmp1 = s.; // incomplete with `;` (next line should parse) - │ ^^ Unbound field '' in 'a::m::AnotherStruct' - error[E01002]: unexpected token ┌─ tests/move_2024/parser/dot_incomplete.move:12:23 │ @@ -13,12 +7,6 @@ error[E01002]: unexpected token │ Unexpected ';' │ Expected an identifier -error[E03010]: unbound field - ┌─ tests/move_2024/parser/dot_incomplete.move:13:21 - │ -13 │ let _tmp2 = s.another_field.; // incomplete with `;` (next line should parse) - │ ^^^^^^^^^^^^^^^^ Unbound field '' in 'a::m::SomeStruct' - error[E01002]: unexpected token ┌─ tests/move_2024/parser/dot_incomplete.move:13:37 │ @@ -28,12 +16,6 @@ error[E01002]: unexpected token │ Unexpected ';' │ Expected an identifier -error[E03010]: unbound field - ┌─ tests/move_2024/parser/dot_incomplete.move:14:21 - │ -14 │ let _tmp3 = s.another_field. // incomplete without `;` (unexpected `let`) - │ ^^^^^^^^^^^^^^^^ Unbound field '' in 'a::m::SomeStruct' - error[E01002]: unexpected token ┌─ tests/move_2024/parser/dot_incomplete.move:15:9 │ @@ -43,3 +25,12 @@ error[E01002]: unexpected token │ Unexpected 'let' │ Expected an identifier +error[E01002]: unexpected token + ┌─ tests/move_2024/parser/dot_incomplete.move:17:5 + │ +17 │ } + │ ^ + │ │ + │ Unexpected '}' + │ Expected an identifier + diff --git a/external-crates/move/crates/move-compiler/tests/move_2024/parser/dot_incomplete.move b/external-crates/move/crates/move-compiler/tests/move_2024/parser/dot_incomplete.move index e768bf77151c5..d66d0d7e637d7 100644 --- a/external-crates/move/crates/move-compiler/tests/move_2024/parser/dot_incomplete.move +++ b/external-crates/move/crates/move-compiler/tests/move_2024/parser/dot_incomplete.move @@ -13,7 +13,6 @@ module a::m { let _tmp2 = s.another_field.; // incomplete with `;` (next line should parse) let _tmp3 = s.another_field. // incomplete without `;` (unexpected `let`) let _tmp4 = s; + let _tmp = s. // incomplete without `;` (unexpected `}`) } - - } From 0320c49530d20f5f492520957c833cd1d7fb135a Mon Sep 17 00:00:00 2001 From: Adam Welc Date: Thu, 11 Apr 2024 12:33:06 -0700 Subject: [PATCH 3/8] Removed forgotten debug print --- .../move/crates/move-compiler/src/parser/syntax.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/external-crates/move/crates/move-compiler/src/parser/syntax.rs b/external-crates/move/crates/move-compiler/src/parser/syntax.rs index dc6d21573cff0..485974c6feedb 100644 --- a/external-crates/move/crates/move-compiler/src/parser/syntax.rs +++ b/external-crates/move/crates/move-compiler/src/parser/syntax.rs @@ -1568,13 +1568,6 @@ fn parse_sequence(context: &mut Context) -> Result> { // sequence and proceed with parsing top-level definition (assume the second scenario). if !context.at_stop_set() || context.tokens.at(Tok::RBrace) { context.advance(); // consume (the RBrace) - if context.tokens.text.starts_with("module a::m {") { - eprintln!( - "CONSUME: {:?} DIAGS {}", - context.tokens.peek(), - context.env.count_diags() - ); - } } Ok((uses, seq, last_semicolon_loc, Box::new(eopt))) } From d683704eb9e84a5aee113658da8e4dbb9d27134b Mon Sep 17 00:00:00 2001 From: Adam Welc Date: Thu, 11 Apr 2024 12:33:18 -0700 Subject: [PATCH 4/8] A minor test fix --- .../tests/move_check/parser/let_binding_missing_semicolon.move | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/external-crates/move/crates/move-compiler/tests/move_check/parser/let_binding_missing_semicolon.move b/external-crates/move/crates/move-compiler/tests/move_check/parser/let_binding_missing_semicolon.move index eac9d56180bef..eee58cc63515c 100644 --- a/external-crates/move/crates/move-compiler/tests/move_check/parser/let_binding_missing_semicolon.move +++ b/external-crates/move/crates/move-compiler/tests/move_check/parser/let_binding_missing_semicolon.move @@ -1,5 +1,5 @@ module 0x42::M { fun f() { - let x // Test a missing semicolon + let _x // Test a missing semicolon } } From 980ce0eb4417ea2f562d82178f105a5eb02d3b9d Mon Sep 17 00:00:00 2001 From: Adam Welc Date: Mon, 15 Apr 2024 14:54:13 -0700 Subject: [PATCH 5/8] Added additional info to the error case --- .../crates/move-compiler/src/expansion/ast.rs | 4 +-- .../move-compiler/src/expansion/translate.rs | 12 +++++---- .../src/hlir/detect_dead_code.rs | 4 ++- .../move-compiler/src/hlir/translate.rs | 3 ++- .../crates/move-compiler/src/naming/ast.rs | 4 +-- .../src/naming/resolve_use_funs.rs | 2 +- .../move-compiler/src/naming/translate.rs | 6 ++--- .../crates/move-compiler/src/parser/ast.rs | 6 ++--- .../crates/move-compiler/src/parser/syntax.rs | 2 +- .../crates/move-compiler/src/typing/ast.rs | 7 +++++ .../src/typing/dependency_ordering.rs | 1 + .../crates/move-compiler/src/typing/expand.rs | 3 ++- .../src/typing/infinite_instantiations.rs | 3 ++- .../move-compiler/src/typing/macro_expand.rs | 6 +++-- .../move-compiler/src/typing/translate.rs | 26 +++++++++++-------- .../move-compiler/src/typing/visitor.rs | 1 + 16 files changed, 56 insertions(+), 34 deletions(-) diff --git a/external-crates/move/crates/move-compiler/src/expansion/ast.rs b/external-crates/move/crates/move-compiler/src/expansion/ast.rs index 2d261314788b9..51f2e732af3b6 100644 --- a/external-crates/move/crates/move-compiler/src/expansion/ast.rs +++ b/external-crates/move/crates/move-compiler/src/expansion/ast.rs @@ -327,7 +327,7 @@ pub enum ExpDotted_ { Exp(Box), Dot(Box, Name), Index(Box, Spanned>), - DotUnresolved(Box), // dot where Name could not be parsed + DotUnresolved(Loc, Box), // dot where Name could not be parsed } pub type ExpDotted = Spanned; @@ -1735,7 +1735,7 @@ impl AstDebug for ExpDotted_ { w.comma(&rhs.value, |w, e| e.ast_debug(w)); w.write("]"); } - D::DotUnresolved(e) => { + D::DotUnresolved(_, e) => { e.ast_debug(w); w.write("") } diff --git a/external-crates/move/crates/move-compiler/src/expansion/translate.rs b/external-crates/move/crates/move-compiler/src/expansion/translate.rs index 6749373252b1a..063ce73021cc7 100644 --- a/external-crates/move/crates/move-compiler/src/expansion/translate.rs +++ b/external-crates/move/crates/move-compiler/src/expansion/translate.rs @@ -2632,7 +2632,7 @@ fn exp(context: &mut Context, pe: Box) -> Box { EE::UnresolvedError } }, - pdotted_ @ (PE::Dot(_, _) | PE::DotUnresolved(_)) => { + pdotted_ @ (PE::Dot(_, _) | PE::DotUnresolved(_, _)) => { match exp_dotted(context, Box::new(sp(loc, pdotted_))) { Some(edotted) => EE::ExpDotted(E::DottedUsage::Use, edotted), None => { @@ -2740,7 +2740,7 @@ fn exp_cast(context: &mut Context, in_parens: bool, plhs: Box, pty: P::T PE::DotCall(lhs, _, _, _, _) | PE::Dot(lhs, _) - | PE::DotUnresolved(lhs) + | PE::DotUnresolved(_, lhs) | PE::Index(lhs, _) | PE::Borrow(_, lhs) | PE::Dereference(lhs) => ambiguous_cast(lhs), @@ -2857,7 +2857,9 @@ fn move_or_copy_path_(context: &mut Context, case: PathCase, pe: Box) -> return None; } } - E::ExpDotted_::Dot(_, _) | E::ExpDotted_::DotUnresolved(_) | E::ExpDotted_::Index(_, _) => { + E::ExpDotted_::Dot(_, _) + | E::ExpDotted_::DotUnresolved(_, _) + | E::ExpDotted_::Index(_, _) => { let current_package = context.current_package(); context .env() @@ -2895,9 +2897,9 @@ fn exp_dotted(context: &mut Context, pdotted: Box) -> Option>(); EE::Index(lhs, sp(argloc, args)) } - PE::DotUnresolved(plhs) => { + PE::DotUnresolved(loc, plhs) => { let lhs = exp_dotted(context, plhs)?; - EE::DotUnresolved(lhs) + EE::DotUnresolved(loc, lhs) } pe_ => EE::Exp(exp(context, Box::new(sp(loc, pe_)))), }; diff --git a/external-crates/move/crates/move-compiler/src/hlir/detect_dead_code.rs b/external-crates/move/crates/move-compiler/src/hlir/detect_dead_code.rs index 4ccda256b67be..9323c4284e56f 100644 --- a/external-crates/move/crates/move-compiler/src/hlir/detect_dead_code.rs +++ b/external-crates/move/crates/move-compiler/src/hlir/detect_dead_code.rs @@ -532,7 +532,8 @@ fn value(context: &mut Context, e: &T::Exp) -> Option { | E::UnaryExp(_, base_exp) | E::Borrow(_, base_exp, _) | E::Cast(base_exp, _) - | E::TempBorrow(_, base_exp) => value_report!(base_exp), + | E::TempBorrow(_, base_exp) + | E::InvalidAccess(base_exp) => value_report!(base_exp), E::BorrowLocal(_, _) => None, @@ -746,6 +747,7 @@ fn statement(context: &mut Context, e: &T::Exp) -> Option { | E::ErrorConstant(_) | E::Move { .. } | E::Copy { .. } + | E::InvalidAccess(_) | E::UnresolvedError => value(context, e), E::Value(_) | E::Unit { .. } => None, diff --git a/external-crates/move/crates/move-compiler/src/hlir/translate.rs b/external-crates/move/crates/move-compiler/src/hlir/translate.rs index 71384b8d4b27d..1062ed80408d8 100644 --- a/external-crates/move/crates/move-compiler/src/hlir/translate.rs +++ b/external-crates/move/crates/move-compiler/src/hlir/translate.rs @@ -1850,7 +1850,7 @@ fn value( context.env.add_diag(ice!((eloc, "ICE unexpanded use"))); error_exp(eloc) } - E::UnresolvedError => { + E::UnresolvedError | E::InvalidAccess(_) => { assert!(context.env.has_errors()); make_exp(HE::UnresolvedError) } @@ -2203,6 +2203,7 @@ fn statement(context: &mut Context, block: &mut Block, e: T::Exp) { | E::Move { .. } | E::Copy { .. } | E::UnresolvedError + | E::InvalidAccess(_) | E::NamedBlock(_, _)) => value_statement(context, block, make_exp(e_)), E::Value(_) | E::Unit { .. } => (), diff --git a/external-crates/move/crates/move-compiler/src/naming/ast.rs b/external-crates/move/crates/move-compiler/src/naming/ast.rs index d3885e5fe8193..b8cf64030f7c8 100644 --- a/external-crates/move/crates/move-compiler/src/naming/ast.rs +++ b/external-crates/move/crates/move-compiler/src/naming/ast.rs @@ -356,7 +356,7 @@ pub enum ExpDotted_ { Exp(Box), Dot(Box, Field), Index(Box, Spanned>), - DotUnresolved(Box), // dot where Field could not be parsed + DotUnresolved(Loc, Box), // Dot (and its location) where Field could not be parsed } pub type ExpDotted = Spanned; @@ -1883,7 +1883,7 @@ impl AstDebug for ExpDotted_ { w.comma(args, |w, e| e.ast_debug(w)); w.write(")"); } - D::DotUnresolved(e) => { + D::DotUnresolved(_, e) => { e.ast_debug(w); w.write(".") } diff --git a/external-crates/move/crates/move-compiler/src/naming/resolve_use_funs.rs b/external-crates/move/crates/move-compiler/src/naming/resolve_use_funs.rs index ae7238e0b0717..4af8ff8a7b361 100644 --- a/external-crates/move/crates/move-compiler/src/naming/resolve_use_funs.rs +++ b/external-crates/move/crates/move-compiler/src/naming/resolve_use_funs.rs @@ -400,7 +400,7 @@ fn exp(context: &mut Context, sp!(_, e_): &mut N::Exp) { fn exp_dotted(context: &mut Context, sp!(_, ed_): &mut N::ExpDotted) { match ed_ { N::ExpDotted_::Exp(e) => exp(context, e), - N::ExpDotted_::Dot(ed, _) | N::ExpDotted_::DotUnresolved(ed) => exp_dotted(context, ed), + N::ExpDotted_::Dot(ed, _) | N::ExpDotted_::DotUnresolved(_, ed) => exp_dotted(context, ed), N::ExpDotted_::Index(ed, sp!(_, es)) => { exp_dotted(context, ed); for e in es { diff --git a/external-crates/move/crates/move-compiler/src/naming/translate.rs b/external-crates/move/crates/move-compiler/src/naming/translate.rs index d253af6b2dc01..4e108e212aa03 100644 --- a/external-crates/move/crates/move-compiler/src/naming/translate.rs +++ b/external-crates/move/crates/move-compiler/src/naming/translate.rs @@ -2445,8 +2445,8 @@ fn dotted(context: &mut Context, edot: E::ExpDotted) -> Option { } } E::ExpDotted_::Dot(d, f) => N::ExpDotted_::Dot(Box::new(dotted(context, *d)?), Field(f)), - E::ExpDotted_::DotUnresolved(d) => { - N::ExpDotted_::DotUnresolved(Box::new(dotted(context, *d)?)) + E::ExpDotted_::DotUnresolved(loc, d) => { + N::ExpDotted_::DotUnresolved(loc, Box::new(dotted(context, *d)?)) } E::ExpDotted_::Index(inner, args) => { let args = call_args(context, args); @@ -3655,7 +3655,7 @@ fn remove_unused_bindings_exp_dotted( ) { match ed_ { N::ExpDotted_::Exp(e) => remove_unused_bindings_exp(context, used, e), - N::ExpDotted_::Dot(ed, _) | N::ExpDotted_::DotUnresolved(ed) => { + N::ExpDotted_::Dot(ed, _) | N::ExpDotted_::DotUnresolved(_, ed) => { remove_unused_bindings_exp_dotted(context, used, ed) } N::ExpDotted_::Index(ed, sp!(_, es)) => { diff --git a/external-crates/move/crates/move-compiler/src/parser/ast.rs b/external-crates/move/crates/move-compiler/src/parser/ast.rs index 2c5d25aaff830..3d474e5fa8664 100644 --- a/external-crates/move/crates/move-compiler/src/parser/ast.rs +++ b/external-crates/move/crates/move-compiler/src/parser/ast.rs @@ -624,8 +624,8 @@ pub enum Exp_ { // Internal node marking an error was added to the error list // This is here so the pass can continue even when an error is hit UnresolvedError, - // e.X (where X is not a valid tok after dot and cannot be parsed) - DotUnresolved(Box), + // e.X, where X is not a valid tok after dot and cannot be parsed (includes location of the dot) + DotUnresolved(Loc, Box), } pub type Exp = Spanned; @@ -2105,7 +2105,7 @@ impl AstDebug for Exp_ { w.write(&s.value); } E::UnresolvedError => w.write("_|_"), - E::DotUnresolved(e) => { + E::DotUnresolved(_, e) => { e.ast_debug(w); w.write("."); } diff --git a/external-crates/move/crates/move-compiler/src/parser/syntax.rs b/external-crates/move/crates/move-compiler/src/parser/syntax.rs index 485974c6feedb..b6b789632065a 100644 --- a/external-crates/move/crates/move-compiler/src/parser/syntax.rs +++ b/external-crates/move/crates/move-compiler/src/parser/syntax.rs @@ -2590,7 +2590,7 @@ fn parse_dot_or_index_chain(context: &mut Context) -> Result match parse_identifier(context) { Err(diag) => { context.add_diag(*diag); - Exp_::DotUnresolved(Box::new(lhs)) + Exp_::DotUnresolved(loc, Box::new(lhs)) } Ok(n) => { if is_start_of_call_after_function_name(context, &n) { diff --git a/external-crates/move/crates/move-compiler/src/typing/ast.rs b/external-crates/move/crates/move-compiler/src/typing/ast.rs index 43f5e349a130a..11e1f5aed9185 100644 --- a/external-crates/move/crates/move-compiler/src/typing/ast.rs +++ b/external-crates/move/crates/move-compiler/src/typing/ast.rs @@ -233,6 +233,9 @@ pub enum UnannotatedExp_ { Cast(Box, Box), Annotate(Box, Box), + // unfinished dot access (e.g. `some_field.`) + InvalidAccess(Box), + ErrorConstant(Option), UnresolvedError, } @@ -819,6 +822,10 @@ impl AstDebug for UnannotatedExp_ { ty.ast_debug(w); w.write(")"); } + E::InvalidAccess(e) => { + e.ast_debug(w); + w.write("."); + } E::UnresolvedError => w.write("_|_"), E::ErrorConstant(constant) => { w.write("ErrorConstant"); diff --git a/external-crates/move/crates/move-compiler/src/typing/dependency_ordering.rs b/external-crates/move/crates/move-compiler/src/typing/dependency_ordering.rs index f3d902441f432..28a7f36f9efe7 100644 --- a/external-crates/move/crates/move-compiler/src/typing/dependency_ordering.rs +++ b/external-crates/move/crates/move-compiler/src/typing/dependency_ordering.rs @@ -477,6 +477,7 @@ fn exp(context: &mut Context, e: &T::Exp) { exp(context, e); type_(context, ty) } + E::InvalidAccess(e) => exp(context, e), E::Unit { .. } | E::Value(_) | E::Move { .. } diff --git a/external-crates/move/crates/move-compiler/src/typing/expand.rs b/external-crates/move/crates/move-compiler/src/typing/expand.rs index a5b4d026ac5fd..f92bf92b5381e 100644 --- a/external-crates/move/crates/move-compiler/src/typing/expand.rs +++ b/external-crates/move/crates/move-compiler/src/typing/expand.rs @@ -270,7 +270,8 @@ pub fn exp(context: &mut Context, e: &mut T::Exp) { | E::Dereference(er) | E::UnaryExp(_, er) | E::Borrow(_, er, _) - | E::TempBorrow(_, er) => exp(context, er), + | E::TempBorrow(_, er) + | E::InvalidAccess(er) => exp(context, er), E::Mutate(el, er) => { exp(context, el); exp(context, er) diff --git a/external-crates/move/crates/move-compiler/src/typing/infinite_instantiations.rs b/external-crates/move/crates/move-compiler/src/typing/infinite_instantiations.rs index 6173613f215bc..e7a6deac5d5e9 100644 --- a/external-crates/move/crates/move-compiler/src/typing/infinite_instantiations.rs +++ b/external-crates/move/crates/move-compiler/src/typing/infinite_instantiations.rs @@ -276,7 +276,8 @@ fn exp(context: &mut Context, e: &T::Exp) { | E::Dereference(er) | E::UnaryExp(_, er) | E::Borrow(_, er, _) - | E::TempBorrow(_, er) => exp(context, er), + | E::TempBorrow(_, er) + | E::InvalidAccess(er) => exp(context, er), E::Mutate(el, er) | E::BinopExp(el, _, _, er) => { exp(context, el); exp(context, er) diff --git a/external-crates/move/crates/move-compiler/src/typing/macro_expand.rs b/external-crates/move/crates/move-compiler/src/typing/macro_expand.rs index 0134c090fe211..27a59580c205c 100644 --- a/external-crates/move/crates/move-compiler/src/typing/macro_expand.rs +++ b/external-crates/move/crates/move-compiler/src/typing/macro_expand.rs @@ -630,7 +630,9 @@ fn recolor_exp(ctx: &mut Recolor, sp!(_, e_): &mut N::Exp) { fn recolor_exp_dotted(ctx: &mut Recolor, sp!(_, ed_): &mut N::ExpDotted) { match ed_ { N::ExpDotted_::Exp(e) => recolor_exp(ctx, e), - N::ExpDotted_::Dot(ed, _) | N::ExpDotted_::DotUnresolved(ed) => recolor_exp_dotted(ctx, ed), + N::ExpDotted_::Dot(ed, _) | N::ExpDotted_::DotUnresolved(_, ed) => { + recolor_exp_dotted(ctx, ed) + } N::ExpDotted_::Index(ed, sp!(_, es)) => { recolor_exp_dotted(ctx, ed); for e in es { @@ -1093,7 +1095,7 @@ fn builtin_function(context: &mut Context, sp!(_, bf_): &mut N::BuiltinFunction) fn exp_dotted(context: &mut Context, sp!(_, ed_): &mut N::ExpDotted) { match ed_ { N::ExpDotted_::Exp(e) => exp(context, e), - N::ExpDotted_::Dot(ed, _) | N::ExpDotted_::DotUnresolved(ed) => exp_dotted(context, ed), + N::ExpDotted_::Dot(ed, _) | N::ExpDotted_::DotUnresolved(_, ed) => exp_dotted(context, ed), N::ExpDotted_::Index(ed, sp!(_, es)) => { exp_dotted(context, ed); for e in es { diff --git a/external-crates/move/crates/move-compiler/src/typing/translate.rs b/external-crates/move/crates/move-compiler/src/typing/translate.rs index 7e0984b5bfec1..f2a43ba89e301 100644 --- a/external-crates/move/crates/move-compiler/src/typing/translate.rs +++ b/external-crates/move/crates/move-compiler/src/typing/translate.rs @@ -476,7 +476,11 @@ mod check_valid_constant { //***************************************** // Error cases handled elsewhere //***************************************** - E::Use(_) | E::Continue(_) | E::Give(_, _) | E::UnresolvedError => return, + E::Use(_) + | E::Continue(_) + | E::Give(_, _) + | E::UnresolvedError + | E::InvalidAccess(_) => return, //***************************************** // Valid cases @@ -2977,7 +2981,7 @@ enum ExpDottedAccess { args: Spanned>, base_type: Type, /* base (non-ref) return type */ }, - UnresolvedError, + UnresolvedError(/* dot's location */ Loc), // whatever came after dot could not be parsed } #[derive(Debug)] @@ -3006,7 +3010,7 @@ impl ExpDotted { match accessor { ExpDottedAccess::Field(_, ty) => ty.clone(), ExpDottedAccess::Index { base_type, .. } => base_type.clone(), - ExpDottedAccess::UnresolvedError => sp(Loc::invalid(), Type_::UnresolvedError), + ExpDottedAccess::UnresolvedError(_) => sp(Loc::invalid(), Type_::UnresolvedError), } } else { self.base_type.clone() @@ -3058,9 +3062,9 @@ fn process_exp_dotted( .push(ExpDottedAccess::Field(field, field_type)); inner } - N::ExpDotted_::DotUnresolved(ndot) => { + N::ExpDotted_::DotUnresolved(loc, ndot) => { let mut inner = process_exp_dotted(context, Some("dot access"), *ndot); - inner.accessors.push(ExpDottedAccess::UnresolvedError); + inner.accessors.push(ExpDottedAccess::UnresolvedError(loc)); inner } N::ExpDotted_::Index(ndot, sp!(argloc, nargs_)) => { @@ -3102,7 +3106,7 @@ fn exp_dotted_usage( let constraint_verb = match &ndotted.value { N::ExpDotted_::Exp(_) => None, _ if matches!(usage, DottedUsage::Borrow(_)) => Some("borrow"), - N::ExpDotted_::Dot(_, _) | N::ExpDotted_::DotUnresolved(_) => Some("dot access"), + N::ExpDotted_::Dot(_, _) | N::ExpDotted_::DotUnresolved(_, _) => Some("dot access"), N::ExpDotted_::Index(_, _) => Some("index"), }; let edotted = process_exp_dotted(context, constraint_verb, ndotted); @@ -3340,8 +3344,7 @@ fn borrow_exp_dotted(context: &mut Context, mut_: bool, ed: ExpDotted) -> Box Box::new(base), }; - let num_accessors = accessors.len(); - for (idx, accessor) in accessors.into_iter().enumerate() { + for accessor in accessors { check_mut(context, loc, exp.ty.clone(), mut_); match accessor { ExpDottedAccess::Field(name, ty) => { @@ -3397,8 +3400,9 @@ fn borrow_exp_dotted(context: &mut Context, mut_: bool, ed: ExpDotted) -> Box { - assert!(idx == num_accessors - 1); + ExpDottedAccess::UnresolvedError(loc) => { + let t = sp(Loc::invalid(), Type_::UnresolvedError); + exp = Box::new(T::exp(t, sp(loc, T::UnannotatedExp_::InvalidAccess(exp)))); break; } } @@ -3429,7 +3433,7 @@ fn exp_dotted_to_owned(context: &mut Context, usage: DottedUsage, ed: ExpDotted) ExpDottedAccess::Index { base_type, .. } => { ("index result".to_string(), base_type.clone()) } - ExpDottedAccess::UnresolvedError => ( + ExpDottedAccess::UnresolvedError(_) => ( "unresolved field".to_string(), sp(Loc::invalid(), Type_::UnresolvedError), ), diff --git a/external-crates/move/crates/move-compiler/src/typing/visitor.rs b/external-crates/move/crates/move-compiler/src/typing/visitor.rs index ad81521d2eadd..1bf2adb98b124 100644 --- a/external-crates/move/crates/move-compiler/src/typing/visitor.rs +++ b/external-crates/move/crates/move-compiler/src/typing/visitor.rs @@ -222,6 +222,7 @@ pub trait TypingVisitorContext { E::TempBorrow(_, e) => self.visit_exp(e), E::Cast(e, _) => self.visit_exp(e), E::Annotate(e, _) => self.visit_exp(e), + E::InvalidAccess(e) => self.visit_exp(e), E::Unit { .. } | E::Value(_) | E::Move { .. } From 8b292ba0e24348c15d78180222999211732bc2de Mon Sep 17 00:00:00 2001 From: Adam Welc Date: Tue, 16 Apr 2024 17:25:44 -0700 Subject: [PATCH 6/8] Addressed review comments --- .../crates/move-compiler/src/expansion/ast.rs | 2 +- .../crates/move-compiler/src/parser/syntax.rs | 30 +++++++------------ 2 files changed, 11 insertions(+), 21 deletions(-) diff --git a/external-crates/move/crates/move-compiler/src/expansion/ast.rs b/external-crates/move/crates/move-compiler/src/expansion/ast.rs index 51f2e732af3b6..871e1d3f7bea0 100644 --- a/external-crates/move/crates/move-compiler/src/expansion/ast.rs +++ b/external-crates/move/crates/move-compiler/src/expansion/ast.rs @@ -1737,7 +1737,7 @@ impl AstDebug for ExpDotted_ { } D::DotUnresolved(_, e) => { e.ast_debug(w); - w.write("") + w.write(".") } } } diff --git a/external-crates/move/crates/move-compiler/src/parser/syntax.rs b/external-crates/move/crates/move-compiler/src/parser/syntax.rs index b6b789632065a..865a7d05b297e 100644 --- a/external-crates/move/crates/move-compiler/src/parser/syntax.rs +++ b/external-crates/move/crates/move-compiler/src/parser/syntax.rs @@ -741,7 +741,7 @@ fn parse_name_access_chain_<'a, F: Fn() -> &'a str>( let ln = parse_leading_name_access_(context, global_name, &item_description)?; let (mut is_macro, mut tys) = - parse_macro_opt_and_tyargs_opt(context, tyargs_whitespace_allowed, ln.loc)?; + parse_macro_opt_and_tyargs_opt(context, tyargs_whitespace_allowed, ln.loc); if let Some(loc) = &is_macro { if !macros_allowed { let msg = format!( @@ -823,7 +823,7 @@ fn parse_name_access_chain_<'a, F: Fn() -> &'a str>( )?; let name = parse_identifier(context)?; let (mut is_macro, mut tys) = - parse_macro_opt_and_tyargs_opt(context, tyargs_whitespace_allowed, name.loc)?; + parse_macro_opt_and_tyargs_opt(context, tyargs_whitespace_allowed, name.loc); if let Some(loc) = &is_macro { if !macros_allowed { context.env.add_diag(diag!( @@ -860,13 +860,13 @@ fn parse_macro_opt_and_tyargs_opt( context: &mut Context, tyargs_whitespace_allowed: bool, end_loc: Loc, -) -> Result<(Option, Option>>), Box> { +) -> (Option, Option>>) { let mut is_macro = None; let mut tyargs = None; if let Tok::Exclaim = context.tokens.peek() { let loc = current_token_loc(context.tokens); - context.tokens.advance()?; + context.advance(); is_macro = Some(loc); } @@ -887,7 +887,7 @@ fn parse_macro_opt_and_tyargs_opt( ); tyargs = tys_.map(|tys| sp(ty_loc, tys)); } - Ok((is_macro, tyargs)) + (is_macro, tyargs) } //************************************************************************************************** @@ -2545,6 +2545,7 @@ fn parse_dot_or_index_chain(context: &mut Context) -> Result { context.advance(); @@ -2590,24 +2591,13 @@ fn parse_dot_or_index_chain(context: &mut Context) -> Result match parse_identifier(context) { Err(diag) => { context.add_diag(*diag); - Exp_::DotUnresolved(loc, Box::new(lhs)) + Exp_::DotUnresolved(first_token_loc, Box::new(lhs)) } Ok(n) => { if is_start_of_call_after_function_name(context, &n) { - let call_start = context.tokens.start_loc(); - let is_macro = if let Tok::Exclaim = context.tokens.peek() { - let loc = current_token_loc(context.tokens); - context.advance(); - Some(loc) - } else { - None - }; - let mut tys = None; - if context.tokens.peek() == Tok::Less - && n.loc.end() as usize == call_start - { - tys = parse_optional_type_args(context); - } + let (is_macro, tys) = + parse_macro_opt_and_tyargs_opt(context, false, n.loc); + let tys = tys.map(|t| t.value); let args = parse_call_args(context); Exp_::DotCall(Box::new(lhs), n, is_macro, tys, args) } else { From 0665e4f702022ae29f5c4ee06543bd708ff853c3 Mon Sep 17 00:00:00 2001 From: Adam Welc Date: Tue, 16 Apr 2024 17:42:01 -0700 Subject: [PATCH 7/8] Fixed analyzer tests --- external-crates/move/crates/move-analyzer/src/symbols.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/external-crates/move/crates/move-analyzer/src/symbols.rs b/external-crates/move/crates/move-analyzer/src/symbols.rs index c881532a7c705..5bd9a23c3d8ed 100644 --- a/external-crates/move/crates/move-analyzer/src/symbols.rs +++ b/external-crates/move/crates/move-analyzer/src/symbols.rs @@ -2691,7 +2691,9 @@ impl<'a> TypingSymbolicator<'a> { self.exp_symbols(exp, scope); self.add_type_id_use_def(t); } - + E::InvalidAccess(e) => { + self.exp_symbols(e, scope); + } _ => (), } } @@ -6801,7 +6803,7 @@ fn partial_dot_test() { let ide_files_layer: VfsPath = MemoryFS::new().into(); let (symbols_opt, _) = get_symbols( - &mut BTreeMap::new(), + Arc::new(Mutex::new(BTreeMap::new())), ide_files_layer, path.as_path(), LintLevel::None, From 61466e45e37c15fc6475deed19b5b85260dcbbf0 Mon Sep 17 00:00:00 2001 From: Adam Welc Date: Fri, 19 Apr 2024 12:08:08 -1000 Subject: [PATCH 8/8] Addressed review comments --- .../move/crates/move-analyzer/src/symbols.rs | 17 ++++++++++++++++- .../tests/partial-dot/sources/M1.move | 3 ++- .../crates/move-compiler/src/parser/syntax.rs | 16 +++++++++++++--- .../tests/move_2024/parser/dot_incomplete.exp | 8 ++++---- 4 files changed, 35 insertions(+), 9 deletions(-) diff --git a/external-crates/move/crates/move-analyzer/src/symbols.rs b/external-crates/move/crates/move-analyzer/src/symbols.rs index 5bd9a23c3d8ed..190429b19a3ac 100644 --- a/external-crates/move/crates/move-analyzer/src/symbols.rs +++ b/external-crates/move/crates/move-analyzer/src/symbols.rs @@ -6874,13 +6874,28 @@ fn partial_dot_test() { "PartialDot::M1::AnotherStruct\nanother_field: PartialDot::M1::SomeStruct", Some((1, 18, "M1.move")), ); + // struct-typed second part of incomplete dot chain `s.another_field.` (followed by a list of + // parameters and a semi-colon: `s.another_field.(7, 42);`) + assert_use_def( + mod_symbols, + &symbols, + 2, + 14, + 22, + "M1.move", + 6, + 8, + "M1.move", + "PartialDot::M1::AnotherStruct\nanother_field: PartialDot::M1::SomeStruct", + Some((1, 18, "M1.move")), + ); // struct-typed first part of incomplete dot chain `s.` (no `;` but followed by `}` on the next // line) assert_use_def( mod_symbols, &symbols, 1, - 14, + 15, 20, "M1.move", 9, diff --git a/external-crates/move/crates/move-analyzer/tests/partial-dot/sources/M1.move b/external-crates/move/crates/move-analyzer/tests/partial-dot/sources/M1.move index aa9a868c04a83..ff2047b228cb1 100644 --- a/external-crates/move/crates/move-analyzer/tests/partial-dot/sources/M1.move +++ b/external-crates/move/crates/move-analyzer/tests/partial-dot/sources/M1.move @@ -12,6 +12,7 @@ module PartialDot::M1 { let _tmp2 = s.another_field.; let _tmp3 = s.another_field. let _tmp4 = s; // statement skipped due to unexpected `let` - let _tmp5 = s. + let _tmp5 = s.another_field.(7, 42); + let _tmp6 = s. } } diff --git a/external-crates/move/crates/move-compiler/src/parser/syntax.rs b/external-crates/move/crates/move-compiler/src/parser/syntax.rs index 865a7d05b297e..8d12ae2d7f7eb 100644 --- a/external-crates/move/crates/move-compiler/src/parser/syntax.rs +++ b/external-crates/move/crates/move-compiler/src/parser/syntax.rs @@ -543,6 +543,9 @@ fn report_name_migration(context: &mut Context, name: &str, loc: Loc) { // Parse an identifier: // Identifier = +// +// Expects the current token to be Tok::Identifier or Tok::RestrictedIdentifier and returns +// `Syntax::UnexpectedToken` for the current token if it is not. fn parse_identifier(context: &mut Context) -> Result> { let id: Symbol = match context.tokens.peek() { Tok::Identifier => context.tokens.content().into(), @@ -568,7 +571,7 @@ fn parse_identifier(context: &mut Context) -> Result> { } }; let start_loc = context.tokens.start_loc(); - context.tokens.advance()?; + context.advance(); let end_loc = context.tokens.previous_end_loc(); Ok(spanned(context.tokens.file_hash(), start_loc, end_loc, id)) } @@ -2589,8 +2592,15 @@ fn parse_dot_or_index_chain(context: &mut Context) -> Result match parse_identifier(context) { - Err(diag) => { - context.add_diag(*diag); + Err(_) => { + // if it's neither a number (checked above) nor identifier, it conveys + // more information to the developer to signal that both are a + // possibility here (rather than just identifier which would be signaled + // if we kept the returned diagnostic) + context.add_diag(*unexpected_token_error( + context.tokens, + "an identifier or a decimal number", + )); Exp_::DotUnresolved(first_token_loc, Box::new(lhs)) } Ok(n) => { diff --git a/external-crates/move/crates/move-compiler/tests/move_2024/parser/dot_incomplete.exp b/external-crates/move/crates/move-compiler/tests/move_2024/parser/dot_incomplete.exp index 540d2524d6394..0668e65aaf13c 100644 --- a/external-crates/move/crates/move-compiler/tests/move_2024/parser/dot_incomplete.exp +++ b/external-crates/move/crates/move-compiler/tests/move_2024/parser/dot_incomplete.exp @@ -5,7 +5,7 @@ error[E01002]: unexpected token │ ^ │ │ │ Unexpected ';' - │ Expected an identifier + │ Expected an identifier or a decimal number error[E01002]: unexpected token ┌─ tests/move_2024/parser/dot_incomplete.move:13:37 @@ -14,7 +14,7 @@ error[E01002]: unexpected token │ ^ │ │ │ Unexpected ';' - │ Expected an identifier + │ Expected an identifier or a decimal number error[E01002]: unexpected token ┌─ tests/move_2024/parser/dot_incomplete.move:15:9 @@ -23,7 +23,7 @@ error[E01002]: unexpected token │ ^^^ │ │ │ Unexpected 'let' - │ Expected an identifier + │ Expected an identifier or a decimal number error[E01002]: unexpected token ┌─ tests/move_2024/parser/dot_incomplete.move:17:5 @@ -32,5 +32,5 @@ error[E01002]: unexpected token │ ^ │ │ │ Unexpected '}' - │ Expected an identifier + │ Expected an identifier or a decimal number