Abort checking when encountering an invalid parse node (#4700)

Short term solution/block for #4689
carbon-language · Dec 18, 2024 · 4d0a6db · 4d0a6db
1 parent b69f97d
commit 4d0a6db
Show file tree

Hide file tree

Showing 14 changed files with 40 additions and 98 deletions.
diff --git 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
@@ -127,19 +127,17 @@ 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;
 }
 
 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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> = 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
@@ -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
@@ -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> = 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
@@ -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: <witness> = 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: <witness> = 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> = namespace [template] {
-// CHECK:STDOUT:     .C = imports.%import_ref.1
-// CHECK:STDOUT:     .c = %c
-// CHECK:STDOUT:   }
-// CHECK:STDOUT:   %default.import.loc2_6.1 = import <invalid>
-// CHECK:STDOUT:   %default.import.loc2_6.2 = import <invalid>
-// 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
@@ -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
@@ -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;