From 64ce58dd64e0c0da586ff05400a2183ba8e504f7 Mon Sep 17 00:00:00 2001 From: simontran7 Date: Tue, 15 Sep 2026 23:10:17 +0000 Subject: [PATCH] fix malformed parse tree for invalid let struct pattern (#7782) Fixes the malformed parse tree produced for an invalid let struct pattern containing a single identifier (e.g., `let {s};`). As pointed out by @DavidLoftus, the parser should produce a parse tree similar to that of `let {ref s};`, since both are missing a binding power operator `:`, and both do not have a `.` preceding the identifier (i.e., `state.in_field_shorthand_pattern == true`). This means that the parser can produce an the `InvalidParse` node just as it does for `let {ref s};`. Closes #7674 --- toolchain/parse/handle_pattern.cpp | 7 +- .../fail_let_struct_identifier_only.carbon | 28 ++++++++ .../testdata/struct/struct_pattern.carbon | 68 ++++++++++++------- 3 files changed, 74 insertions(+), 29 deletions(-) create mode 100644 toolchain/parse/testdata/let/fail_let_struct_identifier_only.carbon diff --git a/toolchain/parse/handle_pattern.cpp b/toolchain/parse/handle_pattern.cpp index 904d5df0b851..274a656f35aa 100644 --- a/toolchain/parse/handle_pattern.cpp +++ b/toolchain/parse/handle_pattern.cpp @@ -47,9 +47,10 @@ auto HandlePattern(Context& context) -> void { state.binding_context, state.ambient_precedence); break; default: - if (context.PositionKind().is_word() && - context.PositionKind(Lookahead::NextToken) - .is_binding_pattern_operator()) { + if ((context.PositionKind().is_word() && + context.PositionKind(Lookahead::NextToken) + .is_binding_pattern_operator()) || + state.in_field_shorthand_pattern) { context.PushStateForPattern( StateKind::BindingPattern, state.in_var_pattern, state.in_unused_pattern, state.in_field_shorthand_pattern, diff --git a/toolchain/parse/testdata/let/fail_let_struct_identifier_only.carbon b/toolchain/parse/testdata/let/fail_let_struct_identifier_only.carbon new file mode 100644 index 000000000000..4a8555db17b6 --- /dev/null +++ b/toolchain/parse/testdata/let/fail_let_struct_identifier_only.carbon @@ -0,0 +1,28 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// AUTOUPDATE +// TIP: To test this file alone, run: +// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/parse/testdata/let/fail_let_struct_identifier_only.carbon +// TIP: To dump output, run: +// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/parse/testdata/let/fail_let_struct_identifier_only.carbon + +// CHECK:STDERR: fail_let_struct_identifier_only.carbon:[[@LINE+4]]:7: error: expected `:` or `:?` in binding pattern [ExpectedBindingPattern] +// CHECK:STDERR: let {s}; +// CHECK:STDERR: ^ +// CHECK:STDERR: +let {s}; + +// CHECK:STDOUT: - filename: fail_let_struct_identifier_only.carbon +// CHECK:STDOUT: ╭─FileStart '' +// CHECK:STDOUT: │ ╭─LetIntroducer 'let' +// CHECK:STDOUT: │ │ ╭─StructPatternStart '{' +// CHECK:STDOUT: │ │ │ ╭─IdentifierNameNotBeforeSignature 's' +// CHECK:STDOUT: │ │ │ ├─InvalidParse '}' has_error +// CHECK:STDOUT: │ │ │ ├─InvalidParse '}' has_error +// CHECK:STDOUT: │ │ ├─LetBindingPattern 's' has_error +// CHECK:STDOUT: │ ├─StructPattern '}' has_error +// CHECK:STDOUT: ├─LetDecl ';' +// CHECK:STDOUT: ├─FileEnd '' +// CHECK:STDOUT: (root) diff --git a/toolchain/parse/testdata/struct/struct_pattern.carbon b/toolchain/parse/testdata/struct/struct_pattern.carbon index 1a5584593895..115cf6005354 100644 --- a/toolchain/parse/testdata/struct/struct_pattern.carbon +++ b/toolchain/parse/testdata/struct/struct_pattern.carbon @@ -58,7 +58,7 @@ let{x: i32, .y = {a: i32, b: i32}} = {}; // --- fail_struct_pattern_comma_only.carbon -// CHECK:STDERR: fail_struct_pattern_comma_only.carbon:[[@LINE+4]]:6: error: expected pattern [ExpectedPattern] +// CHECK:STDERR: fail_struct_pattern_comma_only.carbon:[[@LINE+4]]:6: error: expected name in binding pattern [ExpectedBindingPattern] // CHECK:STDERR: let {,} = {}; // CHECK:STDERR: ^ // CHECK:STDERR: @@ -66,7 +66,7 @@ let {,} = {}; // --- fail_struct_pattern_missing_designator.carbon -// CHECK:STDERR: fail_struct_pattern_missing_designator.carbon:[[@LINE+4]]:6: error: expected pattern [ExpectedPattern] +// CHECK:STDERR: fail_struct_pattern_missing_designator.carbon:[[@LINE+4]]:6: error: expected name in binding pattern [ExpectedBindingPattern] // CHECK:STDERR: let {= y: i32} = {}; // CHECK:STDERR: ^ // CHECK:STDERR: @@ -106,7 +106,7 @@ let {.} = {}; // --- fail_struct_pattern_repeated_comma.carbon -// CHECK:STDERR: fail_struct_pattern_repeated_comma.carbon:[[@LINE+4]]:18: error: expected pattern [ExpectedPattern] +// CHECK:STDERR: fail_struct_pattern_repeated_comma.carbon:[[@LINE+4]]:18: error: expected name in binding pattern [ExpectedBindingPattern] // CHECK:STDERR: let {.x = x: i32,,} = {}; // CHECK:STDERR: ^ // CHECK:STDERR: @@ -130,7 +130,7 @@ let {.x = } = {}; // --- fail_struct_pattern_shorthand_missing_identifier.carbon -// CHECK:STDERR: fail_struct_pattern_shorthand_missing_identifier.carbon:[[@LINE+4]]:6: error: expected pattern [ExpectedPattern] +// CHECK:STDERR: fail_struct_pattern_shorthand_missing_identifier.carbon:[[@LINE+4]]:6: error: expected name in binding pattern [ExpectedBindingPattern] // CHECK:STDERR: let {:i32} = {}; // CHECK:STDERR: ^ // CHECK:STDERR: @@ -186,9 +186,9 @@ let {.x = x: i32 foo, .y = y: i32} = {}; // --- fail_struct_pattern_extra_token_start.carbon -// CHECK:STDERR: fail_struct_pattern_extra_token_start.carbon:[[@LINE+4]]:29: error: expected `,` or `}` [UnexpectedTokenAfterListElement] +// CHECK:STDERR: fail_struct_pattern_extra_token_start.carbon:[[@LINE+4]]:23: error: expected `:` or `:?` in binding pattern [ExpectedBindingPattern] // CHECK:STDERR: let {.x = x: i32, foo .y = y: i32} = {}; -// CHECK:STDERR: ^ +// CHECK:STDERR: ^ // CHECK:STDERR: let {.x = x: i32, foo .y = y: i32} = {}; @@ -202,7 +202,7 @@ let {x: i32 foo, y: i32} = {}; // --- fail_struct_pattern_shorthand_extra_token_start.carbon -// CHECK:STDERR: fail_struct_pattern_shorthand_extra_token_start.carbon:[[@LINE+4]]:18: error: expected `,` or `}` [UnexpectedTokenAfterListElement] +// CHECK:STDERR: fail_struct_pattern_shorthand_extra_token_start.carbon:[[@LINE+4]]:18: error: expected `:` or `:?` in binding pattern [ExpectedBindingPattern] // CHECK:STDERR: let {x: i32, foo y: i32} = {}; // CHECK:STDERR: ^ // CHECK:STDERR: @@ -210,7 +210,7 @@ let {x: i32, foo y: i32} = {}; // --- fail_struct_pattern_invalid_introducer.carbon -// CHECK:STDERR: fail_struct_pattern_invalid_introducer.carbon:[[@LINE+4]]:6: error: expected pattern [ExpectedPattern] +// CHECK:STDERR: fail_struct_pattern_invalid_introducer.carbon:[[@LINE+4]]:6: error: expected name in binding pattern [ExpectedBindingPattern] // CHECK:STDERR: let {!x = x: i32} = {}; // CHECK:STDERR: ^ // CHECK:STDERR: @@ -248,9 +248,9 @@ let {.x foo x: (), .y = y: ()} = {}; // --- fail_struct_pattern_invalid_field.carbon -// CHECK:STDERR: fail_struct_pattern_invalid_field.carbon:[[@LINE+4]]:14: error: expected expression [ExpectedExpr] +// CHECK:STDERR: fail_struct_pattern_invalid_field.carbon:[[@LINE+4]]:6: error: expected name in binding pattern [ExpectedBindingPattern] // CHECK:STDERR: let {"foo" = , .x = x:()} = {}; -// CHECK:STDERR: ^ +// CHECK:STDERR: ^~~~~ // CHECK:STDERR: let {"foo" = , .x = x:()} = {}; @@ -553,7 +553,10 @@ let {var _: i32} = {}; // CHECK:STDOUT: ╭─FileStart '' // CHECK:STDOUT: │ ╭─LetIntroducer 'let' // CHECK:STDOUT: │ │ ╭─StructPatternStart '{' -// CHECK:STDOUT: │ │ ├─InvalidParse ',' has_error +// CHECK:STDOUT: │ │ │ ╭─IdentifierNameNotBeforeSignature ',' has_error +// CHECK:STDOUT: │ │ │ ├─InvalidParse ',' has_error +// CHECK:STDOUT: │ │ │ ├─InvalidParse ',' has_error +// CHECK:STDOUT: │ │ ├─LetBindingPattern ',' has_error // CHECK:STDOUT: │ │ ├─PatternListComma ',' // CHECK:STDOUT: │ ├─StructPattern '}' has_error // CHECK:STDOUT: │ ├─LetInitializer '=' @@ -566,7 +569,10 @@ let {var _: i32} = {}; // CHECK:STDOUT: ╭─FileStart '' // CHECK:STDOUT: │ ╭─LetIntroducer 'let' // CHECK:STDOUT: │ │ ╭─StructPatternStart '{' -// CHECK:STDOUT: │ │ │ ╭─InvalidParse '=' has_error +// CHECK:STDOUT: │ │ │ ╭─IdentifierNameNotBeforeSignature '=' has_error +// CHECK:STDOUT: │ │ │ ├─InvalidParse '=' has_error +// CHECK:STDOUT: │ │ │ ├─InvalidParse '=' has_error +// CHECK:STDOUT: │ │ │ ╭─LetBindingPattern '=' has_error // CHECK:STDOUT: │ │ │ ├─DefaultValueExprStart '=' // CHECK:STDOUT: │ │ │ ├─IdentifierNameExpr 'y' // CHECK:STDOUT: │ │ ├─DefaultValuePattern '=' has_error @@ -652,7 +658,10 @@ let {var _: i32} = {}; // CHECK:STDOUT: │ │ │ ├─LetBindingPattern ':' // CHECK:STDOUT: │ │ ├─StructPatternDesignatedField '=' // CHECK:STDOUT: │ │ ├─PatternListComma ',' -// CHECK:STDOUT: │ │ ├─InvalidParse ',' has_error +// CHECK:STDOUT: │ │ │ ╭─IdentifierNameNotBeforeSignature ',' has_error +// CHECK:STDOUT: │ │ │ ├─InvalidParse ',' has_error +// CHECK:STDOUT: │ │ │ ├─InvalidParse ',' has_error +// CHECK:STDOUT: │ │ ├─LetBindingPattern ',' has_error // CHECK:STDOUT: │ │ ├─PatternListComma ',' // CHECK:STDOUT: │ ├─StructPattern '}' has_error // CHECK:STDOUT: │ ├─LetInitializer '=' @@ -700,7 +709,10 @@ let {var _: i32} = {}; // CHECK:STDOUT: ╭─FileStart '' // CHECK:STDOUT: │ ╭─LetIntroducer 'let' // CHECK:STDOUT: │ │ ╭─StructPatternStart '{' -// CHECK:STDOUT: │ │ ├─InvalidParse ':' has_error +// CHECK:STDOUT: │ │ │ ╭─IdentifierNameNotBeforeSignature ':' has_error +// CHECK:STDOUT: │ │ │ ├─BindingPatternTypeStart ':' has_error +// CHECK:STDOUT: │ │ │ ├─IntTypeLiteral 'i32' +// CHECK:STDOUT: │ │ ├─LetBindingPattern ':' has_error // CHECK:STDOUT: │ ├─StructPattern '}' has_error // CHECK:STDOUT: │ ├─LetInitializer '=' // CHECK:STDOUT: │ │ ╭─StructLiteralStart '{' @@ -832,12 +844,10 @@ let {var _: i32} = {}; // CHECK:STDOUT: │ │ │ ├─LetBindingPattern ':' // CHECK:STDOUT: │ │ ├─StructPatternDesignatedField '=' // CHECK:STDOUT: │ │ ├─PatternListComma ',' -// CHECK:STDOUT: │ │ │ ╭─IdentifierNameExpr 'foo' -// CHECK:STDOUT: │ │ │ ├─IdentifierNameNotBeforeSignature 'y' -// CHECK:STDOUT: │ │ │ ╭─MemberAccessExpr '.' -// CHECK:STDOUT: │ │ │ ├─DefaultValueExprStart '=' -// CHECK:STDOUT: │ │ │ ├─IdentifierNameExpr 'y' -// CHECK:STDOUT: │ │ ├─DefaultValuePattern '=' +// CHECK:STDOUT: │ │ │ ╭─IdentifierNameNotBeforeSignature 'foo' +// CHECK:STDOUT: │ │ │ ├─InvalidParse '.' has_error +// CHECK:STDOUT: │ │ │ ├─InvalidParse '.' has_error +// CHECK:STDOUT: │ │ ├─LetBindingPattern 'foo' has_error // CHECK:STDOUT: │ ├─StructPattern '}' has_error // CHECK:STDOUT: │ ├─LetInitializer '=' // CHECK:STDOUT: │ │ ╭─StructLiteralStart '{' @@ -874,7 +884,10 @@ let {var _: i32} = {}; // CHECK:STDOUT: │ │ │ ├─IntTypeLiteral 'i32' // CHECK:STDOUT: │ │ ├─LetBindingPattern ':' // CHECK:STDOUT: │ │ ├─PatternListComma ',' -// CHECK:STDOUT: │ │ ├─IdentifierNameExpr 'foo' +// CHECK:STDOUT: │ │ │ ╭─IdentifierNameNotBeforeSignature 'foo' +// CHECK:STDOUT: │ │ │ ├─InvalidParse 'y' has_error +// CHECK:STDOUT: │ │ │ ├─InvalidParse 'y' has_error +// CHECK:STDOUT: │ │ ├─LetBindingPattern 'foo' has_error // CHECK:STDOUT: │ ├─StructPattern '}' has_error // CHECK:STDOUT: │ ├─LetInitializer '=' // CHECK:STDOUT: │ │ ╭─StructLiteralStart '{' @@ -886,7 +899,10 @@ let {var _: i32} = {}; // CHECK:STDOUT: ╭─FileStart '' // CHECK:STDOUT: │ ╭─LetIntroducer 'let' // CHECK:STDOUT: │ │ ╭─StructPatternStart '{' -// CHECK:STDOUT: │ │ ├─InvalidParse '!' has_error +// CHECK:STDOUT: │ │ │ ╭─IdentifierNameNotBeforeSignature '!' has_error +// CHECK:STDOUT: │ │ │ ├─InvalidParse '!' has_error +// CHECK:STDOUT: │ │ │ ├─InvalidParse '!' has_error +// CHECK:STDOUT: │ │ ├─LetBindingPattern '!' has_error // CHECK:STDOUT: │ ├─StructPattern '}' has_error // CHECK:STDOUT: │ ├─LetInitializer '=' // CHECK:STDOUT: │ │ ╭─StructLiteralStart '{' @@ -960,10 +976,10 @@ let {var _: i32} = {}; // CHECK:STDOUT: ╭─FileStart '' // CHECK:STDOUT: │ ╭─LetIntroducer 'let' // CHECK:STDOUT: │ │ ╭─StructPatternStart '{' -// CHECK:STDOUT: │ │ │ ╭─StringLiteral '"foo"' -// CHECK:STDOUT: │ │ │ ├─DefaultValueExprStart '=' -// CHECK:STDOUT: │ │ │ ├─InvalidParse ',' has_error -// CHECK:STDOUT: │ │ ├─DefaultValuePattern '=' has_error +// CHECK:STDOUT: │ │ │ ╭─IdentifierNameNotBeforeSignature '"foo"' has_error +// CHECK:STDOUT: │ │ │ ├─InvalidParse '"foo"' has_error +// CHECK:STDOUT: │ │ │ ├─InvalidParse '"foo"' has_error +// CHECK:STDOUT: │ │ ├─LetBindingPattern '"foo"' has_error // CHECK:STDOUT: │ │ ├─PatternListComma ',' // CHECK:STDOUT: │ │ │ ╭─IdentifierNameNotBeforeSignature 'x' // CHECK:STDOUT: │ │ │ ╭─StructFieldDesignator '.'