Don't wrap an ErrorInst as a subpattern of another pattern (#5542)

When a pattern is an error, avoid wrapping it as a subpattern in a
RefParamPattern or VarPattern. This allows further error handling to
observe the error occurred without having to unwrap it.

This avoids a crash when an invalid associated constant is written which
has a VarPattern in it. The associated constant machinery expects an
AssociatedConstantDecl but was getting a VarPattern with an ErrorInst
inside. Now it receives an ErrorInst directly, which it is already
looking for.

This choice means that `var` statements in an `interface` will always be
diagnosed as being non-constant, or as being a constant with a `var`, so
we don't need an extra diagnostic saying that `var` is not allowed in an
interface, as this just leads to two diagnostics on the same thing.

This crash was found by a fuzzer.

toolchain/check/handle_let_and_var.cpp

@@ -21,6 +21,7 @@
 #include "toolchain/diagnostics/format_providers.h"
 #include "toolchain/lex/token_kind.h"
 #include "toolchain/parse/node_kind.h"
+#include "toolchain/parse/typed_nodes.h"
 #include "toolchain/sem_ir/ids.h"
 #include "toolchain/sem_ir/inst.h"
 #include "toolchain/sem_ir/name_scope.h"
@@ -143,6 +144,11 @@ auto HandleParseNode(Context& context, Parse::VariablePatternId node_id)
   auto subpattern_id = context.node_stack().PopPattern();
   auto type_id = context.insts().Get(subpattern_id).type_id();
 
+  if (subpattern_id == SemIR::ErrorInst::InstId) {
+    context.node_stack().Push(node_id, SemIR::ErrorInst::InstId);
+    return true;
+  }
+
   // In a parameter list, a `var` pattern is always a single `Call` parameter,
   // even if it contains multiple binding patterns.
   switch (context.full_pattern_stack().CurrentKind()) {
@@ -369,7 +375,8 @@ auto HandleParseNode(Context& context, Parse::LetDeclId node_id) -> bool {
   return true;
 }
 
-auto HandleParseNode(Context& context, Parse::VariableDeclId node_id) -> bool {
+auto HandleParseNode(Context& context, Parse::VariableDeclId /*node_id*/)
+    -> bool {
   auto decl_info =
       HandleDecl<Lex::TokenKind::Var, Parse::NodeKind::VariableIntroducer,
                  Parse::NodeKind::VariableInitializer>(context);
@@ -378,13 +385,6 @@ auto HandleParseNode(Context& context, Parse::VariableDeclId node_id) -> bool {
       context, decl_info.introducer,
       KeywordModifierSet::Access | KeywordModifierSet::Returned);
 
-  if (context.scope_stack().GetCurrentScopeAs<SemIR::InterfaceDecl>()) {
-    CARBON_DIAGNOSTIC(VarInInterfaceDecl, Error,
-                      "`var` declaration in interface");
-    context.emitter().Emit(node_id, VarInInterfaceDecl);
-    return true;
-  }
-
   LocalPatternMatch(context, decl_info.pattern_id, decl_info.init_id);
   return true;
 }

toolchain/check/testdata/var/min_prelude/var_pattern.carbon

@@ -691,7 +691,7 @@ fn G() {
 // CHECK:STDOUT: constants {
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @F(%.param: <error>);
+// CHECK:STDOUT: fn @F();
 // CHECK:STDOUT:
 // CHECK:STDOUT: --- var_self.carbon
 // CHECK:STDOUT:

toolchain/check/testdata/var/no_prelude/fail_in_interface.carbon

@@ -2,6 +2,8 @@
 // Exceptions. See /LICENSE for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
+// EXTRA-ARGS: --dump-sem-ir-ranges=only
+//
 // AUTOUPDATE
 // TIP: To test this file alone, run:
 // TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/var/no_prelude/fail_in_interface.carbon
@@ -9,36 +11,31 @@
 // TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/var/no_prelude/fail_in_interface.carbon
 
 interface I {
-  // CHECK:STDERR: fail_in_interface.carbon:[[@LINE+8]]:7: error: found runtime binding pattern in associated constant declaration; expected a `:!` binding [ExpectedSymbolicBindingInAssociatedConstant]
-  // CHECK:STDERR:   var v: ();
+  // CHECK:STDERR: fail_in_interface.carbon:[[@LINE+4]]:11: error: found runtime binding pattern in associated constant declaration; expected a `:!` binding [ExpectedSymbolicBindingInAssociatedConstant]
+  // CHECK:STDERR:   let var a: ();
+  // CHECK:STDERR:           ^~~~~
+  // CHECK:STDERR:
+  let var a: ();
+
+  // CHECK:STDERR: fail_in_interface.carbon:[[@LINE+4]]:7: error: found runtime binding pattern in associated constant declaration; expected a `:!` binding [ExpectedSymbolicBindingInAssociatedConstant]
+  // CHECK:STDERR:   var b: ();
   // CHECK:STDERR:       ^~~~~
   // CHECK:STDERR:
-  // CHECK:STDERR: fail_in_interface.carbon:[[@LINE+4]]:3: error: `var` declaration in interface [VarInInterfaceDecl]
-  // CHECK:STDERR:   var v: ();
-  // CHECK:STDERR:   ^~~~~~~~~~
+  var b: ();
+
+  // CHECK:STDERR: fail_in_interface.carbon:[[@LINE+8]]:11: error: semantics TODO: `handle invalid parse trees in `check`` [SemanticsTodo]
+  // CHECK:STDERR:   let var c:! ();
+  // CHECK:STDERR:           ^~~~~~
+  // CHECK:STDERR:
+  // CHECK:STDERR: fail_in_interface.carbon:[[@LINE+4]]:17: error: `var` pattern cannot declare a compile-time binding [CompileTimeBindingInVarDecl]
+  // CHECK:STDERR:   let var c:! ();
+  // CHECK:STDERR:                 ^
   // CHECK:STDERR:
-  var v: ();
-}
+  let var c:! ();
 
-// CHECK:STDOUT: --- fail_in_interface.carbon
-// CHECK:STDOUT:
-// CHECK:STDOUT: constants {
-// CHECK:STDOUT:   %I.type: type = facet_type <@I> [concrete]
-// CHECK:STDOUT:   %Self: %I.type = bind_symbolic_name Self, 0 [symbolic]
-// CHECK:STDOUT: }
-// CHECK:STDOUT:
-// CHECK:STDOUT: file {
-// CHECK:STDOUT:   package: <namespace> = namespace [concrete] {
-// CHECK:STDOUT:     .I = %I.decl
-// CHECK:STDOUT:   }
-// CHECK:STDOUT:   %I.decl: type = interface_decl @I [concrete = constants.%I.type] {} {}
-// CHECK:STDOUT: }
-// CHECK:STDOUT:
-// CHECK:STDOUT: interface @I {
-// CHECK:STDOUT:   %Self: %I.type = bind_symbolic_name Self, 0 [symbolic = constants.%Self]
-// CHECK:STDOUT:
-// CHECK:STDOUT: !members:
-// CHECK:STDOUT:   .Self = %Self
-// CHECK:STDOUT:   witness = ()
-// CHECK:STDOUT: }
-// CHECK:STDOUT:
+  // CHECK:STDERR: fail_in_interface.carbon:[[@LINE+4]]:13: error: `var` pattern cannot declare a compile-time binding [CompileTimeBindingInVarDecl]
+  // CHECK:STDERR:   var d:! ();
+  // CHECK:STDERR:             ^
+  // CHECK:STDERR:
+  var d:! ();
+}

toolchain/diagnostics/diagnostic_kind.def

@@ -288,7 +288,6 @@ CARBON_DIAGNOSTIC_KIND(ExportPrevious)
 // Interface checking.
 CARBON_DIAGNOSTIC_KIND(InterfaceForwardDeclaredHere)
 CARBON_DIAGNOSTIC_KIND(InterfaceIncompleteWithinDefinition)
-CARBON_DIAGNOSTIC_KIND(VarInInterfaceDecl)
 
 // Impl checking.
 CARBON_DIAGNOSTIC_KIND(AssociatedConstantHere)