diff --git a/toolchain/diagnostics/diagnostic_kind.def b/toolchain/diagnostics/diagnostic_kind.def index 6f296874fcbc..0916680ed606 100644 --- a/toolchain/diagnostics/diagnostic_kind.def +++ b/toolchain/diagnostics/diagnostic_kind.def @@ -58,6 +58,7 @@ CARBON_DIAGNOSTIC_KIND(ExpectedParameterName) CARBON_DIAGNOSTIC_KIND(ExpectedParenAfter) CARBON_DIAGNOSTIC_KIND(ExpectedSemiAfter) CARBON_DIAGNOSTIC_KIND(ExpectedSemiAfterExpression) +CARBON_DIAGNOSTIC_KIND(ExpectedSemiAfterVar) CARBON_DIAGNOSTIC_KIND(ExpectedStructLiteralField) CARBON_DIAGNOSTIC_KIND(ExpectedVariableDeclaration) CARBON_DIAGNOSTIC_KIND(ExpectedVariableName) diff --git a/toolchain/lowering/lowering_context.cpp b/toolchain/lowering/lowering_context.cpp index 19081e9d910f..7619f682bae0 100644 --- a/toolchain/lowering/lowering_context.cpp +++ b/toolchain/lowering/lowering_context.cpp @@ -57,14 +57,12 @@ auto LoweringContext::BuildLoweredNodeAsType(SemanticsNodeId node_id) -> llvm::Type* { switch (node_id.index) { case SemanticsBuiltinKind::EmptyStructType.AsInt(): - case SemanticsBuiltinKind::EmptyTuple.AsInt(): case SemanticsBuiltinKind::EmptyTupleType.AsInt(): // Represent empty types as empty structs. // TODO: Investigate special-casing handling of these so that they can be // collectively replaced with LLVM's void, particularly around function // returns. LLVM doesn't allow declaring variables with a void type, so // that may require significant special casing. - // TODO: Work to remove EmptyTuple here. return llvm::StructType::create( *llvm_context_, llvm::ArrayRef(), SemanticsBuiltinKind::FromInt(node_id.index).name()); diff --git a/toolchain/lowering/lowering_handle.cpp b/toolchain/lowering/lowering_handle.cpp index 84224b816bd6..70d8bb3a0077 100644 --- a/toolchain/lowering/lowering_handle.cpp +++ b/toolchain/lowering/lowering_handle.cpp @@ -77,8 +77,9 @@ auto LoweringHandleFunctionDeclaration(LoweringContext& context, } llvm::Type* return_type = context.GetLoweredNodeAsType( - callable.return_type_id.is_valid() ? callable.return_type_id - : SemanticsNodeId::BuiltinEmptyTuple); + callable.return_type_id.is_valid() + ? callable.return_type_id + : SemanticsNodeId::BuiltinEmptyTupleType); llvm::FunctionType* function_type = llvm::FunctionType::get(return_type, args, /*isVarArg=*/false); auto* function = llvm::Function::Create( diff --git a/toolchain/lowering/testdata/function/definition/params_one.carbon b/toolchain/lowering/testdata/function/definition/params_one.carbon index cea7b5e9009f..018202a312ec 100644 --- a/toolchain/lowering/testdata/function/definition/params_one.carbon +++ b/toolchain/lowering/testdata/function/definition/params_one.carbon @@ -6,9 +6,9 @@ // CHECK:STDOUT: ; ModuleID = 'params_one.carbon' // CHECK:STDOUT: source_filename = "params_one.carbon" // CHECK:STDOUT: -// CHECK:STDOUT: %EmptyTuple = type {} +// CHECK:STDOUT: %EmptyTupleType = type {} // CHECK:STDOUT: -// CHECK:STDOUT: define %EmptyTuple @Foo(i32 %a) { +// CHECK:STDOUT: define %EmptyTupleType @Foo(i32 %a) { // CHECK:STDOUT: entry: // CHECK:STDOUT: } diff --git a/toolchain/lowering/testdata/function/definition/params_two.carbon b/toolchain/lowering/testdata/function/definition/params_two.carbon index 7615ac94eda3..21d8dfa120fc 100644 --- a/toolchain/lowering/testdata/function/definition/params_two.carbon +++ b/toolchain/lowering/testdata/function/definition/params_two.carbon @@ -6,9 +6,9 @@ // CHECK:STDOUT: ; ModuleID = 'params_two.carbon' // CHECK:STDOUT: source_filename = "params_two.carbon" // CHECK:STDOUT: -// CHECK:STDOUT: %EmptyTuple = type {} +// CHECK:STDOUT: %EmptyTupleType = type {} // CHECK:STDOUT: -// CHECK:STDOUT: define %EmptyTuple @Foo(i32 %a, i32 %b) { +// CHECK:STDOUT: define %EmptyTupleType @Foo(i32 %a, i32 %b) { // CHECK:STDOUT: entry: // CHECK:STDOUT: } diff --git a/toolchain/lowering/testdata/function/definition/params_zero.carbon b/toolchain/lowering/testdata/function/definition/params_zero.carbon index 28fde4df9c32..18b5082bb6ee 100644 --- a/toolchain/lowering/testdata/function/definition/params_zero.carbon +++ b/toolchain/lowering/testdata/function/definition/params_zero.carbon @@ -6,9 +6,9 @@ // CHECK:STDOUT: ; ModuleID = 'params_zero.carbon' // CHECK:STDOUT: source_filename = "params_zero.carbon" // CHECK:STDOUT: -// CHECK:STDOUT: %EmptyTuple = type {} +// CHECK:STDOUT: %EmptyTupleType = type {} // CHECK:STDOUT: -// CHECK:STDOUT: define %EmptyTuple @Foo() { +// CHECK:STDOUT: define %EmptyTupleType @Foo() { // CHECK:STDOUT: entry: // CHECK:STDOUT: } diff --git a/toolchain/lowering/testdata/return/no_value.carbon b/toolchain/lowering/testdata/return/no_value.carbon index 864e5bfbea79..e8b26e61f33d 100644 --- a/toolchain/lowering/testdata/return/no_value.carbon +++ b/toolchain/lowering/testdata/return/no_value.carbon @@ -6,9 +6,9 @@ // CHECK:STDOUT: ; ModuleID = 'no_value.carbon' // CHECK:STDOUT: source_filename = "no_value.carbon" // CHECK:STDOUT: -// CHECK:STDOUT: %EmptyTuple = type {} +// CHECK:STDOUT: %EmptyTupleType = type {} // CHECK:STDOUT: -// CHECK:STDOUT: define %EmptyTuple @Main() { +// CHECK:STDOUT: define %EmptyTupleType @Main() { // CHECK:STDOUT: entry: // CHECK:STDOUT: ret void // CHECK:STDOUT: } diff --git a/toolchain/parser/parser_context.cpp b/toolchain/parser/parser_context.cpp index 117452bbffd4..8cc000f44cc4 100644 --- a/toolchain/parser/parser_context.cpp +++ b/toolchain/parser/parser_context.cpp @@ -422,4 +422,14 @@ auto ParserContext::RecoverFromDeclarationError(StateStackEntry state, /*has_error=*/true); } +auto ParserContext::EmitExpectedDeclarationSemiOrDefinition( + TokenKind expected_kind) -> void { + CARBON_DIAGNOSTIC(ExpectedDeclarationSemiOrDefinition, Error, + "`{0}` should either end with a `;` for a declaration or " + "have a `{{ ... }` block for a definition.", + TokenKind); + emitter().Emit(*position(), ExpectedDeclarationSemiOrDefinition, + expected_kind); +} + } // namespace Carbon diff --git a/toolchain/parser/parser_context.h b/toolchain/parser/parser_context.h index f19be90ec143..77ba887f2a0f 100644 --- a/toolchain/parser/parser_context.h +++ b/toolchain/parser/parser_context.h @@ -255,6 +255,8 @@ class ParserContext { ParserState keyword_state, int subtree_start) -> void; + auto EmitExpectedDeclarationSemiOrDefinition(TokenKind expected_kind) -> void; + // Handles error recovery in a declaration, particularly before any possible // definition has started (although one could be present). Recover to a // semicolon when it makes sense as a possible end, otherwise use the @@ -302,24 +304,6 @@ class ParserContext { auto ParserHandle##Name(ParserContext& context)->void; #include "toolchain/parser/parser_state.def" -// The diagnostics below may be emitted a couple different ways as part of -// operator parsing. -// TODO: Clean these up, maybe as context functions? - -CARBON_DIAGNOSTIC( - OperatorRequiresParentheses, Error, - "Parentheses are required to disambiguate operator precedence."); - -CARBON_DIAGNOSTIC(ExpectedSemiAfterExpression, Error, - "Expected `;` after expression."); - -CARBON_DIAGNOSTIC(ExpectedDeclarationName, Error, - "`{0}` introducer should be followed by a name.", TokenKind); -CARBON_DIAGNOSTIC(ExpectedDeclarationSemiOrDefinition, Error, - "`{0}` should either end with a `;` for a declaration or " - "have a `{{ ... }` block for a definition.", - TokenKind); - } // namespace Carbon #endif // CARBON_TOOLCHAIN_PARSER_PARSER_CONTEXT_H_ diff --git a/toolchain/parser/parser_handle_declaration_name_and_params.cpp b/toolchain/parser/parser_handle_declaration_name_and_params.cpp index c75929b79709..e78e1f108608 100644 --- a/toolchain/parser/parser_handle_declaration_name_and_params.cpp +++ b/toolchain/parser/parser_handle_declaration_name_and_params.cpp @@ -13,6 +13,9 @@ static auto ParserHandleDeclarationNameAndParams(ParserContext& context, if (!context.ConsumeAndAddLeafNodeIf(TokenKind::Identifier, ParseNodeKind::DeclaredName)) { + CARBON_DIAGNOSTIC(ExpectedDeclarationName, Error, + "`{0}` introducer should be followed by a name.", + TokenKind); context.emitter().Emit(*context.position(), ExpectedDeclarationName, context.tokens().GetKind(state.token)); context.ReturnErrorOnState(); diff --git a/toolchain/parser/parser_handle_expression.cpp b/toolchain/parser/parser_handle_expression.cpp index 8ee4b8ccba9a..808c30291bcc 100644 --- a/toolchain/parser/parser_handle_expression.cpp +++ b/toolchain/parser/parser_handle_expression.cpp @@ -6,6 +6,10 @@ namespace Carbon { +CARBON_DIAGNOSTIC( + OperatorRequiresParentheses, Error, + "Parentheses are required to disambiguate operator precedence."); + auto ParserHandleExpression(ParserContext& context) -> void { auto state = context.PopState(); @@ -209,6 +213,8 @@ auto ParserHandleExpressionStatementFinish(ParserContext& context) -> void { } if (!state.has_error) { + CARBON_DIAGNOSTIC(ExpectedSemiAfterExpression, Error, + "Expected `;` after expression."); context.emitter().Emit(*context.position(), ExpectedSemiAfterExpression); } diff --git a/toolchain/parser/parser_handle_function.cpp b/toolchain/parser/parser_handle_function.cpp index bbc8944fc82f..56399fc22c3a 100644 --- a/toolchain/parser/parser_handle_function.cpp +++ b/toolchain/parser/parser_handle_function.cpp @@ -74,9 +74,7 @@ auto ParserHandleFunctionSignatureFinish(ParserContext& context) -> void { } default: { if (!state.has_error) { - context.emitter().Emit(*context.position(), - ExpectedDeclarationSemiOrDefinition, - TokenKind::Fn); + context.EmitExpectedDeclarationSemiOrDefinition(TokenKind::Fn); } // Only need to skip if we've not already found a new line. bool skip_past_likely_end = diff --git a/toolchain/parser/parser_handle_type.cpp b/toolchain/parser/parser_handle_type.cpp index 8a9f3ccb2fc7..5c2e34d2cbba 100644 --- a/toolchain/parser/parser_handle_type.cpp +++ b/toolchain/parser/parser_handle_type.cpp @@ -58,9 +58,8 @@ static auto ParserHandleTypeAfterParams(ParserContext& context, } if (!context.PositionIs(TokenKind::OpenCurlyBrace)) { - context.emitter().Emit(*context.position(), - ExpectedDeclarationSemiOrDefinition, - context.tokens().GetKind(state.token)); + context.EmitExpectedDeclarationSemiOrDefinition( + context.tokens().GetKind(state.token)); context.RecoverFromDeclarationError(state, declaration_kind, /*skip_past_likely_end=*/true); return; diff --git a/toolchain/parser/parser_handle_var.cpp b/toolchain/parser/parser_handle_var.cpp index c1a0fc4fd88c..e6011a292fec 100644 --- a/toolchain/parser/parser_handle_var.cpp +++ b/toolchain/parser/parser_handle_var.cpp @@ -52,7 +52,10 @@ auto ParserHandleVarFinishAsSemicolon(ParserContext& context) -> void { if (context.PositionIs(TokenKind::Semi)) { end_token = context.Consume(); } else { - context.emitter().Emit(*context.position(), ExpectedSemiAfterExpression); + // TODO: Disambiguate between statement and member declaration. + CARBON_DIAGNOSTIC(ExpectedSemiAfterVar, Error, + "Expected `;` to terminate `var` declaration."); + context.emitter().Emit(*context.position(), ExpectedSemiAfterVar); state.has_error = true; if (auto semi_token = context.SkipPastLikelyEnd(state.token)) { end_token = *semi_token; diff --git a/toolchain/parser/testdata/basics/fail_paren_match_regression.carbon b/toolchain/parser/testdata/basics/fail_paren_match_regression.carbon index 2d6371e2a496..c67048fb1056 100644 --- a/toolchain/parser/testdata/basics/fail_paren_match_regression.carbon +++ b/toolchain/parser/testdata/basics/fail_paren_match_regression.carbon @@ -18,5 +18,5 @@ // CHECK:STDERR: fail_paren_match_regression.carbon:[[@LINE+3]]:5: Expected pattern in `var` declaration. // CHECK:STDERR: fail_paren_match_regression.carbon:[[@LINE+2]]:12: Expected `,` or `)`. -// CHECK:STDERR: fail_paren_match_regression.carbon:[[@LINE+1]]:15: Expected `;` after expression. +// CHECK:STDERR: fail_paren_match_regression.carbon:[[@LINE+1]]:15: Expected `;` to terminate `var` declaration. var = (foo {}) diff --git a/toolchain/parser/testdata/operators/fail_infix_uneven_space_after.carbon b/toolchain/parser/testdata/operators/fail_infix_uneven_space_after.carbon index 54d69fc076b8..61ca15a80608 100644 --- a/toolchain/parser/testdata/operators/fail_infix_uneven_space_after.carbon +++ b/toolchain/parser/testdata/operators/fail_infix_uneven_space_after.carbon @@ -17,5 +17,5 @@ // TODO: We could figure out that this first Failed example is infix // with one-token lookahead. -// CHECK:STDERR: fail_infix_uneven_space_after.carbon:[[@LINE+1]]:16: Expected `;` after expression. +// CHECK:STDERR: fail_infix_uneven_space_after.carbon:[[@LINE+1]]:16: Expected `;` to terminate `var` declaration. var n: i8 = n* n; diff --git a/toolchain/parser/testdata/operators/fail_star_star_no_space.carbon b/toolchain/parser/testdata/operators/fail_star_star_no_space.carbon index 727e5e6a25ac..cacb4d968179 100644 --- a/toolchain/parser/testdata/operators/fail_star_star_no_space.carbon +++ b/toolchain/parser/testdata/operators/fail_star_star_no_space.carbon @@ -20,5 +20,5 @@ // before we notice the missing whitespace around the second `*`. // It'd be better to (somehow) form n*(*p) and reject due to the missing // whitespace around the first `*`. -// CHECK:STDERR: fail_star_star_no_space.carbon:[[@LINE+1]]:16: Expected `;` after expression. +// CHECK:STDERR: fail_star_star_no_space.carbon:[[@LINE+1]]:16: Expected `;` to terminate `var` declaration. var n: i8 = n**p;