From 4d0a6db49bdd7eb1c13e20f1614e45b9e2abed0d Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Tue, 17 Dec 2024 16:19:12 -0800 Subject: [PATCH] Abort checking when encountering an invalid parse node (#4700) Short term solution/block for #4689 --- toolchain/check/check_unit.cpp | 5 ++ toolchain/check/handle_name.cpp | 10 ++-- toolchain/check/handle_noop.cpp | 17 +++--- .../testdata/basics/type_literals.carbon | 4 +- .../class/fail_base_as_declared_name.carbon | 2 +- .../testdata/class/fail_base_misplaced.carbon | 2 +- .../class/fail_self_type_member.carbon | 2 +- .../class/fail_todo_local_class.carbon | 2 +- .../no_prelude/extern_library.carbon | 18 ++----- .../fail_pattern_in_signature.carbon | 2 +- .../no_prelude/fail_todo_no_params.carbon | 16 +----- .../packages/no_prelude/export_import.carbon | 54 +++---------------- .../testdata/struct/fail_keyword_name.carbon | 2 +- .../basics/fail_before_lowering.carbon | 2 +- 14 files changed, 40 insertions(+), 98 deletions(-) diff --git a/toolchain/check/check_unit.cpp b/toolchain/check/check_unit.cpp index 54a5e5f9e8d76..87a6b830a5795 100644 --- a/toolchain/check/check_unit.cpp +++ b/toolchain/check/check_unit.cpp @@ -346,6 +346,11 @@ auto CheckUnit::ProcessNodeIds() -> bool { node_id = *maybe_node_id; auto parse_kind = context_.parse_tree().node_kind(node_id); + if (context_.parse_tree().node_has_error(node_id)) { + context_.TODO(node_id, "handle invalid parse trees in `check`"); + return false; + } + bool result; switch (parse_kind) { #define CARBON_PARSE_NODE_KIND(Name) \ diff --git a/toolchain/check/handle_name.cpp b/toolchain/check/handle_name.cpp index 7e17619501f23..81880fe14df69 100644 --- a/toolchain/check/handle_name.cpp +++ b/toolchain/check/handle_name.cpp @@ -127,9 +127,8 @@ auto HandleParseNode(Context& context, Parse::IdentifierNameId node_id) -> bool { // The parent is responsible for binding the name. auto name_id = GetIdentifierAsName(context, node_id); - if (!name_id) { - return context.TODO(node_id, "Error recovery from keyword name."); - } + CARBON_CHECK(name_id, + "Unreachable until we support checking error parse nodes"); context.node_stack().Push(node_id, *name_id); return true; } @@ -137,9 +136,8 @@ auto HandleParseNode(Context& context, Parse::IdentifierNameId node_id) auto HandleParseNode(Context& context, Parse::IdentifierNameExprId node_id) -> bool { auto name_id = GetIdentifierAsName(context, node_id); - if (!name_id) { - return context.TODO(node_id, "Error recovery from keyword name."); - } + CARBON_CHECK(name_id, + "Unreachable until we support checking error parse nodes"); context.node_stack().Push(node_id, HandleNameAsExpr(context, node_id, *name_id)); return true; diff --git a/toolchain/check/handle_noop.cpp b/toolchain/check/handle_noop.cpp index 251db60e7ab2f..b71f7b1d567de 100644 --- a/toolchain/check/handle_noop.cpp +++ b/toolchain/check/handle_noop.cpp @@ -13,18 +13,19 @@ auto HandleParseNode(Context& /*context*/, Parse::EmptyDeclId /*node_id*/) return true; } -auto HandleParseNode(Context& context, Parse::InvalidParseId node_id) -> bool { - return context.TODO(node_id, "HandleInvalidParse"); +auto HandleParseNode(Context& /*context*/, Parse::InvalidParseId /*node_id*/) + -> bool { + CARBON_FATAL("Unreachable until we support checking error parse nodes"); } -auto HandleParseNode(Context& context, Parse::InvalidParseStartId node_id) - -> bool { - return context.TODO(node_id, "HandleInvalidParseStart"); +auto HandleParseNode(Context& /*context*/, + Parse::InvalidParseStartId /*node_id*/) -> bool { + CARBON_FATAL("Unreachable until we support checking error parse nodes"); } -auto HandleParseNode(Context& context, Parse::InvalidParseSubtreeId node_id) - -> bool { - return context.TODO(node_id, "HandleInvalidParseSubtree"); +auto HandleParseNode(Context& /*context*/, + Parse::InvalidParseSubtreeId /*node_id*/) -> bool { + CARBON_FATAL("Unreachable until we support checking error parse nodes"); } auto HandleParseNode(Context& /*context*/, Parse::PlaceholderId /*node_id*/) diff --git a/toolchain/check/testdata/basics/type_literals.carbon b/toolchain/check/testdata/basics/type_literals.carbon index 2fec59c203ab4..00aaf3a58c843 100644 --- a/toolchain/check/testdata/basics/type_literals.carbon +++ b/toolchain/check/testdata/basics/type_literals.carbon @@ -42,7 +42,7 @@ var test_i15: i15; // CHECK:STDERR: var test_i1000000000: i1000000000; // TODO: This diagnostic is not very good. -// CHECK:STDERR: fail_iN_bad_width.carbon:[[@LINE+8]]:33: error: semantics TODO: `HandleInvalidParse` [SemanticsTodo] +// CHECK:STDERR: fail_iN_bad_width.carbon:[[@LINE+8]]:33: error: semantics TODO: `handle invalid parse trees in `check`` [SemanticsTodo] // CHECK:STDERR: var test_i10000000000000000000: i10000000000000000000; // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~ // CHECK:STDERR: @@ -86,7 +86,7 @@ var test_u15: u15; // CHECK:STDERR: var test_u1000000000: u1000000000; // TODO: This diagnostic is not very good. -// CHECK:STDERR: fail_uN_bad_width.carbon:[[@LINE+8]]:33: error: semantics TODO: `HandleInvalidParse` [SemanticsTodo] +// CHECK:STDERR: fail_uN_bad_width.carbon:[[@LINE+8]]:33: error: semantics TODO: `handle invalid parse trees in `check`` [SemanticsTodo] // CHECK:STDERR: var test_u10000000000000000000: u10000000000000000000; // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~ // CHECK:STDERR: diff --git a/toolchain/check/testdata/class/fail_base_as_declared_name.carbon b/toolchain/check/testdata/class/fail_base_as_declared_name.carbon index 301cce52e191c..59dc0914996c4 100644 --- a/toolchain/check/testdata/class/fail_base_as_declared_name.carbon +++ b/toolchain/check/testdata/class/fail_base_as_declared_name.carbon @@ -14,7 +14,7 @@ namespace N; // CHECK:STDERR: fn N.base() {} // CHECK:STDERR: ^~~~ // CHECK:STDERR: -// CHECK:STDERR: fail_base_as_declared_name.carbon:[[@LINE+3]]:6: error: semantics TODO: `HandleInvalidParse` [SemanticsTodo] +// CHECK:STDERR: fail_base_as_declared_name.carbon:[[@LINE+3]]:6: error: semantics TODO: `handle invalid parse trees in `check`` [SemanticsTodo] // CHECK:STDERR: fn N.base() {} // CHECK:STDERR: ^~~~ fn N.base() {} diff --git a/toolchain/check/testdata/class/fail_base_misplaced.carbon b/toolchain/check/testdata/class/fail_base_misplaced.carbon index 36a735093ba22..fbd0ce4c4fae9 100644 --- a/toolchain/check/testdata/class/fail_base_misplaced.carbon +++ b/toolchain/check/testdata/class/fail_base_misplaced.carbon @@ -21,7 +21,7 @@ fn F() { // CHECK:STDERR: extend base: B; // CHECK:STDERR: ^~~~~~ // CHECK:STDERR: - // CHECK:STDERR: fail_base_misplaced.carbon:[[@LINE+3]]:3: error: semantics TODO: `HandleInvalidParse` [SemanticsTodo] + // CHECK:STDERR: fail_base_misplaced.carbon:[[@LINE+3]]:3: error: semantics TODO: `handle invalid parse trees in `check`` [SemanticsTodo] // CHECK:STDERR: extend base: B; // CHECK:STDERR: ^~~~~~ extend base: B; diff --git a/toolchain/check/testdata/class/fail_self_type_member.carbon b/toolchain/check/testdata/class/fail_self_type_member.carbon index 37d8e67936930..ce7510e8cf3fc 100644 --- a/toolchain/check/testdata/class/fail_self_type_member.carbon +++ b/toolchain/check/testdata/class/fail_self_type_member.carbon @@ -18,7 +18,7 @@ fn F() -> bool { // CHECK:STDERR: var c2: Class.Self = c1; // CHECK:STDERR: ^~~~ // CHECK:STDERR: - // CHECK:STDERR: fail_self_type_member.carbon:[[@LINE+3]]:17: error: semantics TODO: `Error recovery from keyword name.` [SemanticsTodo] + // CHECK:STDERR: fail_self_type_member.carbon:[[@LINE+3]]:17: error: semantics TODO: `handle invalid parse trees in `check`` [SemanticsTodo] // CHECK:STDERR: var c2: Class.Self = c1; // CHECK:STDERR: ^~~~ var c2: Class.Self = c1; diff --git a/toolchain/check/testdata/class/fail_todo_local_class.carbon b/toolchain/check/testdata/class/fail_todo_local_class.carbon index 26ef9a5d28cd1..39c8fc2aa61c9 100644 --- a/toolchain/check/testdata/class/fail_todo_local_class.carbon +++ b/toolchain/check/testdata/class/fail_todo_local_class.carbon @@ -15,7 +15,7 @@ class A { // CHECK:STDERR: class B { // CHECK:STDERR: ^~~~~ // CHECK:STDERR: - // CHECK:STDERR: fail_todo_local_class.carbon:[[@LINE+3]]:5: error: semantics TODO: `HandleInvalidParse` [SemanticsTodo] + // CHECK:STDERR: fail_todo_local_class.carbon:[[@LINE+3]]:5: error: semantics TODO: `handle invalid parse trees in `check`` [SemanticsTodo] // CHECK:STDERR: class B { // CHECK:STDERR: ^~~~~ class B { diff --git a/toolchain/check/testdata/function/declaration/no_prelude/extern_library.carbon b/toolchain/check/testdata/function/declaration/no_prelude/extern_library.carbon index e6f40a066995c..b616a7a3b381d 100644 --- a/toolchain/check/testdata/function/declaration/no_prelude/extern_library.carbon +++ b/toolchain/check/testdata/function/declaration/no_prelude/extern_library.carbon @@ -102,6 +102,10 @@ extern library "extern_library_owner" fn F(); library "[[@TEST_NAME]]"; +// CHECK:STDERR: fail_extern_library_mismatch_owner.carbon:[[@LINE+4]]:1: error: semantics TODO: `handle invalid parse trees in `check`` [SemanticsTodo] +// CHECK:STDERR: import library "extern_library_mismatch" +// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +// CHECK:STDERR: import library "extern_library_mismatch" // CHECK:STDERR: fail_extern_library_mismatch_owner.carbon:[[@LINE+4]]:1: error: `import` declarations must end with a `;` [ExpectedDeclSemi] @@ -294,19 +298,7 @@ extern library "extern_library_owner" fn F() {} // CHECK:STDOUT: // CHECK:STDOUT: --- fail_extern_library_mismatch_owner.carbon // CHECK:STDOUT: -// CHECK:STDOUT: constants { -// CHECK:STDOUT: %F.type: type = fn_type @F [template] -// CHECK:STDOUT: %F: %F.type = struct_value () [template] -// CHECK:STDOUT: } -// CHECK:STDOUT: -// CHECK:STDOUT: file { -// CHECK:STDOUT: package: = namespace [template] { -// CHECK:STDOUT: .F = %F.decl -// CHECK:STDOUT: } -// CHECK:STDOUT: %F.decl: %F.type = fn_decl @F [template = constants.%F] {} {} -// CHECK:STDOUT: } -// CHECK:STDOUT: -// CHECK:STDOUT: fn @F(); +// CHECK:STDOUT: file {} // CHECK:STDOUT: // CHECK:STDOUT: --- fail_extern_self_library.carbon // CHECK:STDOUT: diff --git a/toolchain/check/testdata/function/declaration/no_prelude/fail_pattern_in_signature.carbon b/toolchain/check/testdata/function/declaration/no_prelude/fail_pattern_in_signature.carbon index c3f32d0239ff8..00f1d603d4052 100644 --- a/toolchain/check/testdata/function/declaration/no_prelude/fail_pattern_in_signature.carbon +++ b/toolchain/check/testdata/function/declaration/no_prelude/fail_pattern_in_signature.carbon @@ -12,7 +12,7 @@ // CHECK:STDERR: fn F((a: {}, b: {}), c: {}); // CHECK:STDERR: ^ // CHECK:STDERR: -// CHECK:STDERR: fail_pattern_in_signature.carbon:[[@LINE+3]]:6: error: semantics TODO: `Error recovery from keyword name.` [SemanticsTodo] +// CHECK:STDERR: fail_pattern_in_signature.carbon:[[@LINE+3]]:6: error: semantics TODO: `handle invalid parse trees in `check`` [SemanticsTodo] // CHECK:STDERR: fn F((a: {}, b: {}), c: {}); // CHECK:STDERR: ^ fn F((a: {}, b: {}), c: {}); diff --git a/toolchain/check/testdata/function/declaration/no_prelude/fail_todo_no_params.carbon b/toolchain/check/testdata/function/declaration/no_prelude/fail_todo_no_params.carbon index 9161cdcec295c..9629f465b0be7 100644 --- a/toolchain/check/testdata/function/declaration/no_prelude/fail_todo_no_params.carbon +++ b/toolchain/check/testdata/function/declaration/no_prelude/fail_todo_no_params.carbon @@ -31,7 +31,7 @@ fn A { // TODO: We don't have parsing support for this yet. library "[[@TEST_NAME]]"; -// CHECK:STDERR: fail_todo_arrow_body.carbon:[[@LINE+8]]:1: error: semantics TODO: `function with positional parameters` [SemanticsTodo] +// CHECK:STDERR: fail_todo_arrow_body.carbon:[[@LINE+8]]:1: error: semantics TODO: `handle invalid parse trees in `check`` [SemanticsTodo] // CHECK:STDERR: fn A => 0; // CHECK:STDERR: ^~~~~~~~~~ // CHECK:STDERR: @@ -95,19 +95,7 @@ fn A { // CHECK:STDOUT: // CHECK:STDOUT: --- fail_todo_arrow_body.carbon // CHECK:STDOUT: -// CHECK:STDOUT: constants { -// CHECK:STDOUT: %A.type: type = fn_type @A [template] -// CHECK:STDOUT: %A: %A.type = struct_value () [template] -// CHECK:STDOUT: } -// CHECK:STDOUT: -// CHECK:STDOUT: file { -// CHECK:STDOUT: package: = namespace [template] { -// CHECK:STDOUT: .A = %A.decl -// CHECK:STDOUT: } -// CHECK:STDOUT: %A.decl: %A.type = fn_decl @A [template = constants.%A] {} {} -// CHECK:STDOUT: } -// CHECK:STDOUT: -// CHECK:STDOUT: fn @A(); +// CHECK:STDOUT: file {} // CHECK:STDOUT: // CHECK:STDOUT: --- fail_invalid_file_generic_regression_test.carbon // CHECK:STDOUT: diff --git a/toolchain/check/testdata/packages/no_prelude/export_import.carbon b/toolchain/check/testdata/packages/no_prelude/export_import.carbon index b9bb77b4e5ea8..a66d289a5162f 100644 --- a/toolchain/check/testdata/packages/no_prelude/export_import.carbon +++ b/toolchain/check/testdata/packages/no_prelude/export_import.carbon @@ -72,10 +72,14 @@ var c: C = {.x = ()}; impl library "[[@TEST_NAME]]"; -// CHECK:STDERR: fail_export_in_impl.impl.carbon:[[@LINE+4]]:1: error: `export` is only allowed in API files [ExportFromImpl] +// CHECK:STDERR: fail_export_in_impl.impl.carbon:[[@LINE+8]]:1: error: `export` is only allowed in API files [ExportFromImpl] // CHECK:STDERR: export import library "base"; // CHECK:STDERR: ^~~~~~ // CHECK:STDERR: +// CHECK:STDERR: fail_export_in_impl.impl.carbon:[[@LINE+4]]:1: error: semantics TODO: `handle invalid parse trees in `check`` [SemanticsTodo] +// CHECK:STDERR: export import library "base"; +// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +// CHECK:STDERR: export import library "base"; // Note the import still occurs. @@ -317,53 +321,7 @@ var indirect_c: C = {.x = ()}; // CHECK:STDOUT: // CHECK:STDOUT: --- fail_export_in_impl.impl.carbon // CHECK:STDOUT: -// CHECK:STDOUT: constants { -// CHECK:STDOUT: %C: type = class_type @C [template] -// CHECK:STDOUT: %empty_tuple.type: type = tuple_type () [template] -// CHECK:STDOUT: %struct_type.x: type = struct_type {.x: %empty_tuple.type} [template] -// CHECK:STDOUT: %complete_type: = complete_type_witness %struct_type.x [template] -// CHECK:STDOUT: %empty_tuple: %empty_tuple.type = tuple_value () [template] -// CHECK:STDOUT: %C.val: %C = struct_value (%empty_tuple) [template] -// CHECK:STDOUT: } -// CHECK:STDOUT: -// CHECK:STDOUT: imports { -// CHECK:STDOUT: %import_ref.1: type = import_ref Main//base, C, loaded [template = constants.%C] -// CHECK:STDOUT: %import_ref.2: = import_ref Main//base, loc6_1, loaded [template = constants.%complete_type] -// CHECK:STDOUT: %import_ref.3 = import_ref Main//base, inst14 [no loc], unloaded -// CHECK:STDOUT: %import_ref.4 = import_ref Main//base, loc5_8, unloaded -// CHECK:STDOUT: } -// CHECK:STDOUT: -// CHECK:STDOUT: file { -// CHECK:STDOUT: package: = namespace [template] { -// CHECK:STDOUT: .C = imports.%import_ref.1 -// CHECK:STDOUT: .c = %c -// CHECK:STDOUT: } -// CHECK:STDOUT: %default.import.loc2_6.1 = import -// CHECK:STDOUT: %default.import.loc2_6.2 = import -// CHECK:STDOUT: %C.ref: type = name_ref C, imports.%import_ref.1 [template = constants.%C] -// CHECK:STDOUT: %c.var: ref %C = var c -// CHECK:STDOUT: %c: ref %C = bind_name c, %c.var -// CHECK:STDOUT: } -// CHECK:STDOUT: -// CHECK:STDOUT: class @C [from "base.carbon"] { -// CHECK:STDOUT: !members: -// CHECK:STDOUT: .Self = imports.%import_ref.3 -// CHECK:STDOUT: .x = imports.%import_ref.4 -// CHECK:STDOUT: complete_type_witness = imports.%import_ref.2 -// CHECK:STDOUT: } -// CHECK:STDOUT: -// CHECK:STDOUT: fn @__global_init() { -// CHECK:STDOUT: !entry: -// CHECK:STDOUT: %.loc11_19.1: %empty_tuple.type = tuple_literal () -// CHECK:STDOUT: %.loc11_20.1: %struct_type.x = struct_literal (%.loc11_19.1) -// CHECK:STDOUT: %.loc11_20.2: ref %empty_tuple.type = class_element_access file.%c.var, element0 -// CHECK:STDOUT: %.loc11_19.2: init %empty_tuple.type = tuple_init () to %.loc11_20.2 [template = constants.%empty_tuple] -// CHECK:STDOUT: %.loc11_20.3: init %empty_tuple.type = converted %.loc11_19.1, %.loc11_19.2 [template = constants.%empty_tuple] -// CHECK:STDOUT: %.loc11_20.4: init %C = class_init (%.loc11_20.3), file.%c.var [template = constants.%C.val] -// CHECK:STDOUT: %.loc11_21: init %C = converted %.loc11_20.1, %.loc11_20.4 [template = constants.%C.val] -// CHECK:STDOUT: assign file.%c.var, %.loc11_21 -// CHECK:STDOUT: return -// CHECK:STDOUT: } +// CHECK:STDOUT: file {} // CHECK:STDOUT: // CHECK:STDOUT: --- export_export.impl.carbon // CHECK:STDOUT: diff --git a/toolchain/check/testdata/struct/fail_keyword_name.carbon b/toolchain/check/testdata/struct/fail_keyword_name.carbon index 605a8b0394675..6958dfc372db4 100644 --- a/toolchain/check/testdata/struct/fail_keyword_name.carbon +++ b/toolchain/check/testdata/struct/fail_keyword_name.carbon @@ -12,7 +12,7 @@ // CHECK:STDERR: fn F() -> {.class: i32}; // CHECK:STDERR: ^~~~~ // CHECK:STDERR: -// CHECK:STDERR: fail_keyword_name.carbon:[[@LINE+4]]:13: error: semantics TODO: `Error recovery from keyword name.` [SemanticsTodo] +// CHECK:STDERR: fail_keyword_name.carbon:[[@LINE+4]]:13: error: semantics TODO: `handle invalid parse trees in `check`` [SemanticsTodo] // CHECK:STDERR: fn F() -> {.class: i32}; // CHECK:STDERR: ^~~~~ // CHECK:STDERR: diff --git a/toolchain/lower/testdata/basics/fail_before_lowering.carbon b/toolchain/lower/testdata/basics/fail_before_lowering.carbon index cf4d03be94951..c5cde99a113fd 100644 --- a/toolchain/lower/testdata/basics/fail_before_lowering.carbon +++ b/toolchain/lower/testdata/basics/fail_before_lowering.carbon @@ -13,7 +13,7 @@ // CHECK:STDERR: a; // CHECK:STDERR: ^ // CHECK:STDERR: -// CHECK:STDERR: fail_before_lowering.carbon:[[@LINE+3]]:1: error: semantics TODO: `HandleInvalidParseStart` [SemanticsTodo] +// CHECK:STDERR: fail_before_lowering.carbon:[[@LINE+3]]:1: error: semantics TODO: `handle invalid parse trees in `check`` [SemanticsTodo] // CHECK:STDERR: a; // CHECK:STDERR: ^ a;