From a1f1c7438f4c94716e1eec5eb29f9648b8935bb1 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Tue, 23 Jan 2024 19:37:00 -0800 Subject: [PATCH] Improve source locations for some diagnostics. (#3644) --- toolchain/check/handle_array.cpp | 9 +++---- toolchain/check/handle_class.cpp | 5 ++-- .../testdata/array/fail_bound_overflow.carbon | 4 ++-- .../testdata/array/fail_invalid_type.carbon | 4 ++-- .../testdata/class/fail_base_bad_type.carbon | 24 +++++++++---------- 5 files changed, 24 insertions(+), 22 deletions(-) diff --git a/toolchain/check/handle_array.cpp b/toolchain/check/handle_array.cpp index d4cd6b164d53..bd48358dbc47 100644 --- a/toolchain/check/handle_array.cpp +++ b/toolchain/check/handle_array.cpp @@ -31,7 +31,8 @@ auto HandleArrayExpr(Context& context, Parse::ArrayExprId parse_node) -> bool { auto bound_inst_id = context.node_stack().PopExpr(); context.node_stack() .PopAndDiscardSoloParseNode(); - auto element_type_inst_id = context.node_stack().PopExpr(); + auto [element_type_node_id, element_type_inst_id] = + context.node_stack().PopExprWithParseNode(); // The array bound must be a constant. // @@ -47,9 +48,9 @@ auto HandleArrayExpr(Context& context, Parse::ArrayExprId parse_node) -> bool { } context.AddInstAndPush( - {parse_node, SemIR::ArrayType{ - SemIR::TypeId::TypeType, bound_inst_id, - ExprAsType(context, parse_node, element_type_inst_id)}}); + {parse_node, SemIR::ArrayType{SemIR::TypeId::TypeType, bound_inst_id, + ExprAsType(context, element_type_node_id, + element_type_inst_id)}}); return true; } diff --git a/toolchain/check/handle_class.cpp b/toolchain/check/handle_class.cpp index 6a0e5fe58a59..862a2a97b79d 100644 --- a/toolchain/check/handle_class.cpp +++ b/toolchain/check/handle_class.cpp @@ -249,7 +249,8 @@ static auto CheckBaseType(Context& context, Parse::NodeId parse_node, } auto HandleBaseDecl(Context& context, Parse::BaseDeclId parse_node) -> bool { - auto base_type_expr_id = context.node_stack().PopExpr(); + auto [base_type_node_id, base_type_expr_id] = + context.node_stack().PopExprWithParseNode(); // Process modifiers. `extend` is required, none others are allowed. LimitModifiersOnDecl(context, KeywordModifierSet::Extend, @@ -284,7 +285,7 @@ auto HandleBaseDecl(Context& context, Parse::BaseDeclId parse_node) -> bool { return true; } - auto base_info = CheckBaseType(context, parse_node, base_type_expr_id); + auto base_info = CheckBaseType(context, base_type_node_id, base_type_expr_id); // The `base` value in the class scope has an unbound element type. Instance // binding will be performed when it's found by name lookup into an instance. diff --git a/toolchain/check/testdata/array/fail_bound_overflow.carbon b/toolchain/check/testdata/array/fail_bound_overflow.carbon index 19ad7f81b122..e0c317c25595 100644 --- a/toolchain/check/testdata/array/fail_bound_overflow.carbon +++ b/toolchain/check/testdata/array/fail_bound_overflow.carbon @@ -9,9 +9,9 @@ // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~ var a: [i32; 39999999999999999993]; -// CHECK:STDERR: fail_bound_overflow.carbon:[[@LINE+6]]:8: ERROR: Cannot implicitly convert from `i32` to `type`. +// CHECK:STDERR: fail_bound_overflow.carbon:[[@LINE+6]]:9: ERROR: Cannot implicitly convert from `i32` to `type`. // CHECK:STDERR: var b: [1; 39999999999999999993]; -// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~ +// CHECK:STDERR: ^ // CHECK:STDERR: fail_bound_overflow.carbon:[[@LINE+3]]:12: ERROR: Array bound of 39999999999999999993 is too large. // CHECK:STDERR: var b: [1; 39999999999999999993]; // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~ diff --git a/toolchain/check/testdata/array/fail_invalid_type.carbon b/toolchain/check/testdata/array/fail_invalid_type.carbon index 6fc2942ba263..e71b75944b91 100644 --- a/toolchain/check/testdata/array/fail_invalid_type.carbon +++ b/toolchain/check/testdata/array/fail_invalid_type.carbon @@ -4,9 +4,9 @@ // // AUTOUPDATE -// CHECK:STDERR: fail_invalid_type.carbon:[[@LINE+3]]:8: ERROR: Cannot implicitly convert from `i32` to `type`. +// CHECK:STDERR: fail_invalid_type.carbon:[[@LINE+3]]:9: ERROR: Cannot implicitly convert from `i32` to `type`. // CHECK:STDERR: var a: [1; 1]; -// CHECK:STDERR: ^~~~~~ +// CHECK:STDERR: ^ var a: [1; 1]; // CHECK:STDOUT: --- fail_invalid_type.carbon diff --git a/toolchain/check/testdata/class/fail_base_bad_type.carbon b/toolchain/check/testdata/class/fail_base_bad_type.carbon index c0339e13c9ed..aed2d91a5746 100644 --- a/toolchain/check/testdata/class/fail_base_bad_type.carbon +++ b/toolchain/check/testdata/class/fail_base_bad_type.carbon @@ -20,18 +20,18 @@ class DeriveFromError { fn AccessMemberWithInvalidBaseError(p: DeriveFromError*) -> i32 { return (*p).n; } class DeriveFromNonType { - // CHECK:STDERR: fail_base_bad_type.carbon:[[@LINE+3]]:3: ERROR: Cannot implicitly convert from `i32` to `type`. + // CHECK:STDERR: fail_base_bad_type.carbon:[[@LINE+3]]:16: ERROR: Cannot implicitly convert from `i32` to `type`. // CHECK:STDERR: extend base: 32; - // CHECK:STDERR: ^~~~~~~~~~~~~~~~ + // CHECK:STDERR: ^~ extend base: 32; } fn AccessMemberWithInvalidBasNonType(p: DeriveFromNonType*) -> i32 { return (*p).n; } class DeriveFromi32 { - // CHECK:STDERR: fail_base_bad_type.carbon:[[@LINE+3]]:3: ERROR: Deriving from final type `i32`. Base type must be an `abstract` or `base` class. + // CHECK:STDERR: fail_base_bad_type.carbon:[[@LINE+3]]:16: ERROR: Deriving from final type `i32`. Base type must be an `abstract` or `base` class. // CHECK:STDERR: extend base: i32; - // CHECK:STDERR: ^~~~~~~~~~~~~~~~~ + // CHECK:STDERR: ^~~ extend base: i32; } @@ -45,9 +45,9 @@ fn ConvertToBadBasei32(p: DeriveFromi32*) -> i32* { return p; } fn AccessMemberWithInvalidBasei32(p: DeriveFromi32*) -> i32 { return (*p).n; } class DeriveFromTuple { - // CHECK:STDERR: fail_base_bad_type.carbon:[[@LINE+3]]:3: ERROR: Deriving from final type `(Base,)`. Base type must be an `abstract` or `base` class. + // CHECK:STDERR: fail_base_bad_type.carbon:[[@LINE+3]]:16: ERROR: Deriving from final type `(Base,)`. Base type must be an `abstract` or `base` class. // CHECK:STDERR: extend base: (Base,); - // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~ + // CHECK:STDERR: ^~~~~~~ extend base: (Base,); } @@ -61,9 +61,9 @@ fn AccessMemberWithInvalidBaseTuple(p: DeriveFromTuple*) -> i32 { return (*p).n; // TODO: Should we allow this? // We do allow `{.base = {.a: i32, .b: i32}}`. class DeriveFromStruct { - // CHECK:STDERR: fail_base_bad_type.carbon:[[@LINE+3]]:3: ERROR: Deriving from final type `{.a: i32, .b: i32}`. Base type must be an `abstract` or `base` class. + // CHECK:STDERR: fail_base_bad_type.carbon:[[@LINE+3]]:16: ERROR: Deriving from final type `{.a: i32, .b: i32}`. Base type must be an `abstract` or `base` class. // CHECK:STDERR: extend base: {.a: i32, .b: i32}; - // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~ extend base: {.a: i32, .b: i32}; } @@ -78,9 +78,9 @@ fn AccessMemberWithInvalidBaseStruct(p: DeriveFromStruct*) -> i32 { return (*p). base class Incomplete; class DeriveFromIncomplete { - // CHECK:STDERR: fail_base_bad_type.carbon:[[@LINE+6]]:3: ERROR: Base `Incomplete` is an incomplete type. + // CHECK:STDERR: fail_base_bad_type.carbon:[[@LINE+6]]:16: ERROR: Base `Incomplete` is an incomplete type. // CHECK:STDERR: extend base: Incomplete; - // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~ + // CHECK:STDERR: ^~~~~~~~~~ // CHECK:STDERR: fail_base_bad_type.carbon:[[@LINE-6]]:1: Class was forward declared here. // CHECK:STDERR: base class Incomplete; // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~ @@ -95,9 +95,9 @@ fn ConvertToBadBaseIncomplete(p: DeriveFromIncomplete*) -> Incomplete* { return fn AccessMemberWithInvalidBaseIncomplete(p: DeriveFromIncomplete*) -> i32 { return (*p).n; } class DeriveFromFinal { - // CHECK:STDERR: fail_base_bad_type.carbon:[[@LINE+3]]:3: ERROR: Deriving from final type `Final`. Base type must be an `abstract` or `base` class. + // CHECK:STDERR: fail_base_bad_type.carbon:[[@LINE+3]]:16: ERROR: Deriving from final type `Final`. Base type must be an `abstract` or `base` class. // CHECK:STDERR: extend base: Final; - // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~ + // CHECK:STDERR: ^~~~~ extend base: Final; }