[toolchain] Parsing support for struct literals, following #653. #699
Conversation
This supports both struct type literals, `{x: i32, y: i32}`, and struct
value literals, `{.x = 3, .y = 4}`. The degenerate case of `{}` is
treated as a struct value literal, with the expectation that an empty
struct value has the same type/value duality as an empty tuple value.
toolchain/parser/parse_tree_test.cpp
Outdated
| @@ -970,6 +970,82 @@ TEST_F(ParseTreeTest, Tuples) { | |||
| MatchFileEnd()})); | |||
| } | |||
|
|
|||
| TEST_F(ParseTreeTest, Structs) { | |||
| TokenizedBuffer tokens = GetTokenizedBuffer(R"( | |||
| var x: {a: i32, b: i32} = {.a = 1, .b = 2}; | |||
There was a problem hiding this comment.
This proposes something outside of #653, and is nesting patterns into positional fields? Or does this have some other meaning?
Generally not sure what we're doing with struct patterns yet... Do the names a and b have to match here? We never really answered some of these questions in the whole struct pattern discussion.
There was a problem hiding this comment.
The {a: i32, b: i32} here is a type, not a struct pattern, following the syntax in the decision on #653. Though I've now noticed that the syntax decision in #561 and the syntax described in #653 aren't the same and #561 uses {.a: i32, .b: i32} instead. I've raised the question of whether to include the . on #syntax and in #561.
There was a problem hiding this comment.
Rule has been changed in #653 to require . here; patch updated to match.
There was a problem hiding this comment.
(not sure this update got pushed)
There was a problem hiding this comment.
Oops :) Look again now?
toolchain/parser/parse_tree_test.cpp
Outdated
| TokenizedBuffer tokens = GetTokenizedBuffer(R"( | ||
| var x: {a: i32, b: i32} = {.a = 1, .b = 2}; | ||
| var y: {} = {}; | ||
| var z: {c: i32 = 0} = {}; |
There was a problem hiding this comment.
This in particular seems like it should be {.c: i32 = 0}?
There was a problem hiding this comment.
Per #561, support for default values in types removed.
| bool can_be_type; | ||
| bool can_be_value; |
There was a problem hiding this comment.
What do you think about is_type_valid and is_value_valid?
I'm ambivalent. The is_ prefix is more obviously a bool to me, but it reads a touch worse and is a bit longer.
Don't change unless you like it, fine to bias towards what you have.
| static constexpr const char* Message = "Expected `,` or `{0}`."; | ||
|
|
||
| TokenKind close; | ||
|
|
||
| auto Format() -> std::string { | ||
| return llvm::formatv(Message, close.GetFixedSpelling()).str(); | ||
| } |
There was a problem hiding this comment.
This diagnostic change seems untested? (Maybe this would be covered by the requested diagnostic tests above actually.)
toolchain/parser/parse_tree_test.cpp
Outdated
| llvm::StringLiteral testcases[] = { | ||
| "var x: {i32} = {};", | ||
| "var x: {a} = {};", | ||
| "var x: {a:} = {};", | ||
| "var x: {a=} = {};", | ||
| "var x: {.} = {};", | ||
| "var x: {.a} = {};", | ||
| "var x: {.a:} = {};", | ||
| "var x: {.a=} = {};", | ||
| "var x: {.a: i32, .b = 0} = {};", | ||
| "var x: {.a = 0, b: i32} = {};", | ||
| "var x: {,} = {};", | ||
| "var x: {.a: i32,} = {};", | ||
| "var x: {.a = 0,} = {};", | ||
| }; | ||
|
|
||
| for (llvm::StringLiteral testcase : testcases) { | ||
| TokenizedBuffer tokens = GetTokenizedBuffer(testcase); | ||
| ErrorTrackingDiagnosticConsumer error_tracker(consumer); | ||
| ParseTree tree = ParseTree::Parse(tokens, error_tracker); | ||
| EXPECT_TRUE(tree.HasErrors()); | ||
| EXPECT_TRUE(error_tracker.SeenError()); | ||
| } |
There was a problem hiding this comment.
While I'm happy to have a loop covering patterns like this, I think at least the ones that we expect to diagnose differently should be tested with tests against the diagnostics themselves?
… (#699) This supports both struct type literals, `{.x: i32, .y: i32}`, and struct value literals, `{.x = 3, .y = 4}`. The degenerate case of `{}` is treated as a struct value literal, with the expectation that an empty struct value has the same type/value duality as an empty tuple value.
This supports both struct type literals,
{x: i32, y: i32}, and structvalue literals,
{.x = 3, .y = 4}. The degenerate case of{}istreated as a struct value literal, with the expectation that an empty
struct value has the same type/value duality as an empty tuple value.