From 35dfa5f03cc0aa3f6a153b5215048ae1cfbb2ffb Mon Sep 17 00:00:00 2001 From: josh11b <15258583+josh11b@users.noreply.github.com> Date: Wed, 11 Sep 2024 18:39:22 -0700 Subject: [PATCH] Only allow designators when parsing `where __ = ...` (#4304) Implements [TODO](https://github.com/carbon-language/carbon-lang/pull/4275/files#r1751000646) introduced in #4275 . Note that this enforces the restriction syntactically in parse, following the [design](https://github.com/carbon-language/carbon-lang/blob/trunk/docs/design/generics/details.md#rewrite-constraints). --------- Co-authored-by: Josh L --- toolchain/diagnostics/diagnostic_kind.def | 1 + toolchain/parse/handle_requirement.cpp | 40 +++- toolchain/parse/state.def | 4 + .../testdata/where_expr/fail_rewrite.carbon | 197 ++++++++++++++++++ toolchain/parse/typed_nodes.h | 3 +- 5 files changed, 234 insertions(+), 11 deletions(-) create mode 100644 toolchain/parse/testdata/where_expr/fail_rewrite.carbon diff --git a/toolchain/diagnostics/diagnostic_kind.def b/toolchain/diagnostics/diagnostic_kind.def index 8ef113a39243..40ce21c96548 100644 --- a/toolchain/diagnostics/diagnostic_kind.def +++ b/toolchain/diagnostics/diagnostic_kind.def @@ -129,6 +129,7 @@ CARBON_DIAGNOSTIC_KIND(ExpectedAliasInitializer) // Where requirement diagnostics. CARBON_DIAGNOSTIC_KIND(ExpectedRequirementOperator) +CARBON_DIAGNOSTIC_KIND(RequirementEqualAfterNonDesignator) // ============================================================================ // Semantics diagnostics diff --git a/toolchain/parse/handle_requirement.cpp b/toolchain/parse/handle_requirement.cpp index cdebda1c6f0c..086500697e3a 100644 --- a/toolchain/parse/handle_requirement.cpp +++ b/toolchain/parse/handle_requirement.cpp @@ -9,9 +9,23 @@ namespace Carbon::Parse { auto HandleRequirementBegin(Context& context) -> void { - context.PopAndDiscardState(); - // TODO: Peek ahead for `.designator = ...`, and give it special handling. - context.PushState(State::RequirementOperator); + auto state = context.PopState(); + + // Peek ahead for `.designator = ...`, and give it special handling. + if (context.PositionKind() == Lex::TokenKind::Period && + context.PositionKind(Lookahead::NextToken) == + Lex::TokenKind::Identifier && + context.PositionKind(static_cast(2)) == + Lex::TokenKind::Equal) { + auto period = context.Consume(); + context.AddNode(NodeKind::IdentifierName, context.Consume(), + /*has_error=*/false); + context.AddNode(NodeKind::DesignatorExpr, period, /*has_error=*/false); + state.token = context.Consume(); + context.PushState(state, State::RequirementOperatorFinish); + } else { + context.PushState(State::RequirementOperator); + } context.PushStateForExpr(PrecedenceGroup::ForRequirements()); } @@ -19,14 +33,22 @@ auto HandleRequirementOperator(Context& context) -> void { auto state = context.PopState(); switch (context.PositionKind()) { - case Lex::TokenKind::Impls: { + // Accept either `impls` or `==` + case Lex::TokenKind::Impls: + case Lex::TokenKind::EqualEqual: break; - } + + // Reject `=` since correct usage is consumed in `HandleRequirementBegin`. case Lex::TokenKind::Equal: { - break; - } - case Lex::TokenKind::EqualEqual: { - break; + if (!state.has_error) { + CARBON_DIAGNOSTIC( + RequirementEqualAfterNonDesignator, Error, + "Requirement can only use `=` after `.member` designator."); + context.emitter().Emit(*context.position(), + RequirementEqualAfterNonDesignator); + } + context.ReturnErrorOnState(); + return; } default: { if (!state.has_error) { diff --git a/toolchain/parse/state.def b/toolchain/parse/state.def index a956ed4559aa..5ea5d1afe1c1 100644 --- a/toolchain/parse/state.def +++ b/toolchain/parse/state.def @@ -618,6 +618,10 @@ CARBON_PARSE_STATE(IfExprFinish) // an expression. // TODO: Also a `require` declaration? // +// expr where .designator = ... +// ^~~~~~~~~~~~~ +// 1. Expr +// 2. RequirementOperatorFinish // expr where ... // ^ // 1. Expr diff --git a/toolchain/parse/testdata/where_expr/fail_rewrite.carbon b/toolchain/parse/testdata/where_expr/fail_rewrite.carbon new file mode 100644 index 000000000000..81cc9b5edc3d --- /dev/null +++ b/toolchain/parse/testdata/where_expr/fail_rewrite.carbon @@ -0,0 +1,197 @@ +// 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/where_expr/fail_rewrite.carbon +// TIP: To dump output, run: +// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/parse/testdata/where_expr/fail_rewrite.carbon + +// Tests for reporting errors when a rewrite requirement doesn't follow +// `where .designator = rewrite`. + +// --- fail_not_designator.carbon + +// CHECK:STDERR: fail_not_designator.carbon:[[@LINE+4]]:37: ERROR: Requirement can only use `=` after `.member` designator. +// CHECK:STDERR: fn NotDesignator(T:! type where i32 = .U); +// CHECK:STDERR: ^ +// CHECK:STDERR: +fn NotDesignator(T:! type where i32 = .U); + +// --- fail_designator_in_parens.carbon + +// CHECK:STDERR: fail_designator_in_parens.carbon:[[@LINE+4]]:40: ERROR: Requirement can only use `=` after `.member` designator. +// CHECK:STDERR: fn DesignatorInParens(V:! I where (.J) = bool); +// CHECK:STDERR: ^ +// CHECK:STDERR: +fn DesignatorInParens(V:! I where (.J) = bool); + +// --- fail_dot_self.carbon + +// CHECK:STDERR: fail_dot_self.carbon:[[@LINE+4]]:30: ERROR: Requirement can only use `=` after `.member` designator. +// CHECK:STDERR: fn DotSelf(W:! K where .Self = f32); +// CHECK:STDERR: ^ +// CHECK:STDERR: +fn DotSelf(W:! K where .Self = f32); + +// --- fail_dot_keyword.carbon + +// CHECK:STDERR: fail_dot_keyword.carbon:[[@LINE+4]]:28: ERROR: Expected identifier or `Self` after `.`. +// CHECK:STDERR: fn DotKeyword(W:! K where .and = u8); +// CHECK:STDERR: ^~~ +// CHECK:STDERR: +fn DotKeyword(W:! K where .and = u8); + +// --- fail_postfix_after_designator.carbon + +// CHECK:STDERR: fail_postfix_after_designator.carbon:[[@LINE+4]]:43: ERROR: Requirement can only use `=` after `.member` designator. +// CHECK:STDERR: fn PostfixAfterDesignator(X:! L where .M* = u64*); +// CHECK:STDERR: ^ +// CHECK:STDERR: +fn PostfixAfterDesignator(X:! L where .M* = u64*); + +// --- fail_binary_op_after_designator.carbon + +// CHECK:STDERR: fail_binary_op_after_designator.carbon:[[@LINE+4]]:48: ERROR: Requirement can only use `=` after `.member` designator. +// CHECK:STDERR: fn BinaryOpAfterDesignator(Y:! N where .O + .P = {}); +// CHECK:STDERR: ^ +// CHECK:STDERR: +fn BinaryOpAfterDesignator(Y:! N where .O + .P = {}); + +// --- fail_after_and.carbon + +// CHECK:STDERR: fail_after_and.carbon:[[@LINE+3]]:43: ERROR: Requirement can only use `=` after `.member` designator. +// CHECK:STDERR: fn AfterAnd(Z:! Q where .R impls S and () = .A); +// CHECK:STDERR: ^ +fn AfterAnd(Z:! Q where .R impls S and () = .A); + +// CHECK:STDOUT: - filename: fail_not_designator.carbon +// CHECK:STDOUT: parse_tree: [ +// CHECK:STDOUT: {kind: 'FileStart', text: ''}, +// CHECK:STDOUT: {kind: 'FunctionIntroducer', text: 'fn'}, +// CHECK:STDOUT: {kind: 'IdentifierName', text: 'NotDesignator'}, +// CHECK:STDOUT: {kind: 'TuplePatternStart', text: '('}, +// CHECK:STDOUT: {kind: 'IdentifierName', text: 'T'}, +// CHECK:STDOUT: {kind: 'TypeTypeLiteral', text: 'type'}, +// CHECK:STDOUT: {kind: 'WhereOperand', text: 'where', subtree_size: 2}, +// CHECK:STDOUT: {kind: 'IntTypeLiteral', text: 'i32'}, +// CHECK:STDOUT: {kind: 'WhereExpr', text: 'where', has_error: yes, subtree_size: 4}, +// CHECK:STDOUT: {kind: 'CompileTimeBindingPattern', text: ':!', has_error: yes, subtree_size: 6}, +// CHECK:STDOUT: {kind: 'TuplePattern', text: ')', has_error: yes, subtree_size: 8}, +// CHECK:STDOUT: {kind: 'FunctionDecl', text: ';', subtree_size: 11}, +// CHECK:STDOUT: {kind: 'FileEnd', text: ''}, +// CHECK:STDOUT: ] +// CHECK:STDOUT: - filename: fail_designator_in_parens.carbon +// CHECK:STDOUT: parse_tree: [ +// CHECK:STDOUT: {kind: 'FileStart', text: ''}, +// CHECK:STDOUT: {kind: 'FunctionIntroducer', text: 'fn'}, +// CHECK:STDOUT: {kind: 'IdentifierName', text: 'DesignatorInParens'}, +// CHECK:STDOUT: {kind: 'TuplePatternStart', text: '('}, +// CHECK:STDOUT: {kind: 'IdentifierName', text: 'V'}, +// CHECK:STDOUT: {kind: 'IdentifierNameExpr', text: 'I'}, +// CHECK:STDOUT: {kind: 'WhereOperand', text: 'where', subtree_size: 2}, +// CHECK:STDOUT: {kind: 'ParenExprStart', text: '('}, +// CHECK:STDOUT: {kind: 'IdentifierName', text: 'J'}, +// CHECK:STDOUT: {kind: 'DesignatorExpr', text: '.', subtree_size: 2}, +// CHECK:STDOUT: {kind: 'ParenExpr', text: ')', subtree_size: 4}, +// CHECK:STDOUT: {kind: 'WhereExpr', text: 'where', has_error: yes, subtree_size: 7}, +// CHECK:STDOUT: {kind: 'CompileTimeBindingPattern', text: ':!', has_error: yes, subtree_size: 9}, +// CHECK:STDOUT: {kind: 'TuplePattern', text: ')', has_error: yes, subtree_size: 11}, +// CHECK:STDOUT: {kind: 'FunctionDecl', text: ';', subtree_size: 14}, +// CHECK:STDOUT: {kind: 'FileEnd', text: ''}, +// CHECK:STDOUT: ] +// CHECK:STDOUT: - filename: fail_dot_self.carbon +// CHECK:STDOUT: parse_tree: [ +// CHECK:STDOUT: {kind: 'FileStart', text: ''}, +// CHECK:STDOUT: {kind: 'FunctionIntroducer', text: 'fn'}, +// CHECK:STDOUT: {kind: 'IdentifierName', text: 'DotSelf'}, +// CHECK:STDOUT: {kind: 'TuplePatternStart', text: '('}, +// CHECK:STDOUT: {kind: 'IdentifierName', text: 'W'}, +// CHECK:STDOUT: {kind: 'IdentifierNameExpr', text: 'K'}, +// CHECK:STDOUT: {kind: 'WhereOperand', text: 'where', subtree_size: 2}, +// CHECK:STDOUT: {kind: 'SelfTypeName', text: 'Self'}, +// CHECK:STDOUT: {kind: 'DesignatorExpr', text: '.', subtree_size: 2}, +// CHECK:STDOUT: {kind: 'WhereExpr', text: 'where', has_error: yes, subtree_size: 5}, +// CHECK:STDOUT: {kind: 'CompileTimeBindingPattern', text: ':!', has_error: yes, subtree_size: 7}, +// CHECK:STDOUT: {kind: 'TuplePattern', text: ')', has_error: yes, subtree_size: 9}, +// CHECK:STDOUT: {kind: 'FunctionDecl', text: ';', subtree_size: 12}, +// CHECK:STDOUT: {kind: 'FileEnd', text: ''}, +// CHECK:STDOUT: ] +// CHECK:STDOUT: - filename: fail_dot_keyword.carbon +// CHECK:STDOUT: parse_tree: [ +// CHECK:STDOUT: {kind: 'FileStart', text: ''}, +// CHECK:STDOUT: {kind: 'FunctionIntroducer', text: 'fn'}, +// CHECK:STDOUT: {kind: 'IdentifierName', text: 'DotKeyword'}, +// CHECK:STDOUT: {kind: 'TuplePatternStart', text: '('}, +// CHECK:STDOUT: {kind: 'IdentifierName', text: 'W'}, +// CHECK:STDOUT: {kind: 'IdentifierNameExpr', text: 'K'}, +// CHECK:STDOUT: {kind: 'WhereOperand', text: 'where', subtree_size: 2}, +// CHECK:STDOUT: {kind: 'IdentifierName', text: 'and', has_error: yes}, +// CHECK:STDOUT: {kind: 'DesignatorExpr', text: '.', has_error: yes, subtree_size: 2}, +// CHECK:STDOUT: {kind: 'WhereExpr', text: 'where', has_error: yes, subtree_size: 5}, +// CHECK:STDOUT: {kind: 'CompileTimeBindingPattern', text: ':!', has_error: yes, subtree_size: 7}, +// CHECK:STDOUT: {kind: 'TuplePattern', text: ')', has_error: yes, subtree_size: 9}, +// CHECK:STDOUT: {kind: 'FunctionDecl', text: ';', subtree_size: 12}, +// CHECK:STDOUT: {kind: 'FileEnd', text: ''}, +// CHECK:STDOUT: ] +// CHECK:STDOUT: - filename: fail_postfix_after_designator.carbon +// CHECK:STDOUT: parse_tree: [ +// CHECK:STDOUT: {kind: 'FileStart', text: ''}, +// CHECK:STDOUT: {kind: 'FunctionIntroducer', text: 'fn'}, +// CHECK:STDOUT: {kind: 'IdentifierName', text: 'PostfixAfterDesignator'}, +// CHECK:STDOUT: {kind: 'TuplePatternStart', text: '('}, +// CHECK:STDOUT: {kind: 'IdentifierName', text: 'X'}, +// CHECK:STDOUT: {kind: 'IdentifierNameExpr', text: 'L'}, +// CHECK:STDOUT: {kind: 'WhereOperand', text: 'where', subtree_size: 2}, +// CHECK:STDOUT: {kind: 'IdentifierName', text: 'M'}, +// CHECK:STDOUT: {kind: 'DesignatorExpr', text: '.', subtree_size: 2}, +// CHECK:STDOUT: {kind: 'PostfixOperatorStar', text: '*', subtree_size: 3}, +// CHECK:STDOUT: {kind: 'WhereExpr', text: 'where', has_error: yes, subtree_size: 6}, +// CHECK:STDOUT: {kind: 'CompileTimeBindingPattern', text: ':!', has_error: yes, subtree_size: 8}, +// CHECK:STDOUT: {kind: 'TuplePattern', text: ')', has_error: yes, subtree_size: 10}, +// CHECK:STDOUT: {kind: 'FunctionDecl', text: ';', subtree_size: 13}, +// CHECK:STDOUT: {kind: 'FileEnd', text: ''}, +// CHECK:STDOUT: ] +// CHECK:STDOUT: - filename: fail_binary_op_after_designator.carbon +// CHECK:STDOUT: parse_tree: [ +// CHECK:STDOUT: {kind: 'FileStart', text: ''}, +// CHECK:STDOUT: {kind: 'FunctionIntroducer', text: 'fn'}, +// CHECK:STDOUT: {kind: 'IdentifierName', text: 'BinaryOpAfterDesignator'}, +// CHECK:STDOUT: {kind: 'TuplePatternStart', text: '('}, +// CHECK:STDOUT: {kind: 'IdentifierName', text: 'Y'}, +// CHECK:STDOUT: {kind: 'IdentifierNameExpr', text: 'N'}, +// CHECK:STDOUT: {kind: 'WhereOperand', text: 'where', subtree_size: 2}, +// CHECK:STDOUT: {kind: 'IdentifierName', text: 'O'}, +// CHECK:STDOUT: {kind: 'DesignatorExpr', text: '.', subtree_size: 2}, +// CHECK:STDOUT: {kind: 'IdentifierName', text: 'P'}, +// CHECK:STDOUT: {kind: 'DesignatorExpr', text: '.', subtree_size: 2}, +// CHECK:STDOUT: {kind: 'InfixOperatorPlus', text: '+', subtree_size: 5}, +// CHECK:STDOUT: {kind: 'WhereExpr', text: 'where', has_error: yes, subtree_size: 8}, +// CHECK:STDOUT: {kind: 'CompileTimeBindingPattern', text: ':!', has_error: yes, subtree_size: 10}, +// CHECK:STDOUT: {kind: 'TuplePattern', text: ')', has_error: yes, subtree_size: 12}, +// CHECK:STDOUT: {kind: 'FunctionDecl', text: ';', subtree_size: 15}, +// CHECK:STDOUT: {kind: 'FileEnd', text: ''}, +// CHECK:STDOUT: ] +// CHECK:STDOUT: - filename: fail_after_and.carbon +// CHECK:STDOUT: parse_tree: [ +// CHECK:STDOUT: {kind: 'FileStart', text: ''}, +// CHECK:STDOUT: {kind: 'FunctionIntroducer', text: 'fn'}, +// CHECK:STDOUT: {kind: 'IdentifierName', text: 'AfterAnd'}, +// CHECK:STDOUT: {kind: 'TuplePatternStart', text: '('}, +// CHECK:STDOUT: {kind: 'IdentifierName', text: 'Z'}, +// CHECK:STDOUT: {kind: 'IdentifierNameExpr', text: 'Q'}, +// CHECK:STDOUT: {kind: 'WhereOperand', text: 'where', subtree_size: 2}, +// CHECK:STDOUT: {kind: 'IdentifierName', text: 'R'}, +// CHECK:STDOUT: {kind: 'DesignatorExpr', text: '.', subtree_size: 2}, +// CHECK:STDOUT: {kind: 'IdentifierNameExpr', text: 'S'}, +// CHECK:STDOUT: {kind: 'RequirementImpls', text: 'impls', subtree_size: 4}, +// CHECK:STDOUT: {kind: 'RequirementAnd', text: 'and'}, +// CHECK:STDOUT: {kind: 'TupleLiteralStart', text: '('}, +// CHECK:STDOUT: {kind: 'TupleLiteral', text: ')', subtree_size: 2}, +// CHECK:STDOUT: {kind: 'WhereExpr', text: 'where', has_error: yes, subtree_size: 10}, +// CHECK:STDOUT: {kind: 'CompileTimeBindingPattern', text: ':!', has_error: yes, subtree_size: 12}, +// CHECK:STDOUT: {kind: 'TuplePattern', text: ')', has_error: yes, subtree_size: 14}, +// CHECK:STDOUT: {kind: 'FunctionDecl', text: ';', subtree_size: 17}, +// CHECK:STDOUT: {kind: 'FileEnd', text: ''}, +// CHECK:STDOUT: ] diff --git a/toolchain/parse/typed_nodes.h b/toolchain/parse/typed_nodes.h index 01a82f4cc535..607cd1d8b939 100644 --- a/toolchain/parse/typed_nodes.h +++ b/toolchain/parse/typed_nodes.h @@ -1037,8 +1037,7 @@ struct DesignatorExpr { struct RequirementEqual { static constexpr auto Kind = NodeKind::RequirementEqual.Define( {.category = NodeCategory::Requirement, .child_count = 2}); - // TODO: Enforce that the lhs is always a DesignatorExpr. - AnyExprId lhs; + DesignatorExprId lhs; Lex::EqualTokenIndex token; AnyExprId rhs; };