Separate binding insts for refs and values

toolchain/check/cpp/import.cpp

@@ -1440,11 +1440,11 @@ static auto MakeParamPatternsBlockId(Context& context, SemIR::LocId loc_id,
 
     // TODO: Fix this once templates are supported.
     bool is_template = false;
-    // TODO: Fix this once generics are supported.
-    bool is_generic = false;
+    // TODO: Model reference parameters as ref bindings.
     SemIR::InstId pattern_id =
         AddBindingPattern(context, param_loc_id, name_id, type_id,
-                          type_expr_region_id, is_generic, is_template)
+                          type_expr_region_id, SemIR::ValueBindingPattern::Kind,
+                          is_template)
             .pattern_id;
     pattern_id = AddPatternInst(
         context, {param_loc_id,
@@ -1872,12 +1872,13 @@ static auto ImportVarDecl(Context& context, SemIR::LocId loc_id,
   context.cpp_global_names().Add({.key = {.entity_name_id = entity_name_id},
                                   .clang_decl_id = clang_decl_id});
 
-  // Create `BindingPattern` and `VarPattern` in a `NameBindingDecl`.
+  // Create `RefBindingPattern` and `VarPattern` in a `NameBindingDecl`.
   context.pattern_block_stack().Push();
   SemIR::TypeId pattern_type_id = GetPatternType(context, var_type_id);
-  SemIR::InstId binding_pattern_inst_id = AddPatternInst<SemIR::BindingPattern>(
-      context, loc_id,
-      {.type_id = pattern_type_id, .entity_name_id = entity_name_id});
+  SemIR::InstId binding_pattern_inst_id =
+      AddPatternInst<SemIR::RefBindingPattern>(
+          context, loc_id,
+          {.type_id = pattern_type_id, .entity_name_id = entity_name_id});
   var_storage.pattern_id = AddPatternInst<SemIR::VarPattern>(
       context, Parse::VariablePatternId::None,
       {.type_id = pattern_type_id, .subpattern_id = binding_pattern_inst_id});

toolchain/check/eval_inst.cpp

@@ -105,15 +105,18 @@ auto EvalConstantInst(Context& context, SemIR::BindAlias inst)
       context.constant_values().Get(inst.value_id));
 }
 
-auto EvalConstantInst(Context& context, SemIR::BindName inst)
+auto EvalConstantInst(Context& context, SemIR::RefBinding inst)
     -> ConstantEvalResult {
   // A reference binding evaluates to the value it's bound to.
-  if (inst.value_id.has_value() && SemIR::IsRefCategory(SemIR::GetExprCategory(
-                                       context.sem_ir(), inst.value_id))) {
+  if (inst.value_id.has_value()) {
     return ConstantEvalResult::Existing(
         context.constant_values().Get(inst.value_id));
   }
+  return ConstantEvalResult::NotConstant;
+}
 
+auto EvalConstantInst(Context& /*context*/, SemIR::ValueBinding /*inst*/)
+    -> ConstantEvalResult {
   // Non-`:!` value bindings are not constant.
   return ConstantEvalResult::NotConstant;
 }

toolchain/check/handle_binding_pattern.cpp

@@ -47,6 +47,21 @@ static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id,
   // A non-generic template binding is diagnosed by the parser.
   is_template &= is_generic;
 
+  SemIR::InstKind pattern_inst_kind;
+  switch (node_kind) {
+    case Parse::NodeKind::CompileTimeBindingPattern:
+      pattern_inst_kind = SemIR::InstKind::SymbolicBindingPattern;
+      break;
+    case Parse::NodeKind::LetBindingPattern:
+      pattern_inst_kind = SemIR::InstKind::ValueBindingPattern;
+      break;
+    case Parse::NodeKind::VarBindingPattern:
+      pattern_inst_kind = SemIR::InstKind::RefBindingPattern;
+      break;
+    default:
+      CARBON_FATAL("Unexpected node kind: {0}", node_kind);
+  }
+
   auto [name_node, name_id] = context.node_stack().PopNameWithNodeId();
 
   const DeclIntroducerState& introducer =
@@ -57,7 +72,7 @@ static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id,
     // scopes, but right now we don't support qualified names here.
     auto binding =
         AddBindingPattern(context, name_node, name_id, cast_type_id,
-                          type_expr_region_id, is_generic, is_template);
+                          type_expr_region_id, pattern_inst_kind, is_template);
 
     // TODO: If `is_generic`, then `binding.bind_id is a BindSymbolicName. Subst
     // the `.Self` of type `type` in the `cast_type_id` type (a `FacetType`)

toolchain/check/handle_choice.cpp

@@ -221,7 +221,7 @@ static auto MakeLetBinding(Context& context, const ChoiceInfo& choice_info,
       {.name_id = binding.name_component.name_id,
        .parent_scope_id = choice_info.name_scope_id});
   auto bind_name_id = AddInst(context, binding.node_id,
-                              SemIR::BindName{
+                              SemIR::ValueBinding{
                                   .type_id = choice_info.self_type_id,
                                   .entity_name_id = entity_name_id,
                                   .value_id = self_value_id,

toolchain/check/import.cpp

@@ -45,8 +45,9 @@ static auto GetImportName(const SemIR::File& import_sem_ir,
     -> std::pair<SemIR::NameId, SemIR::NameScopeId> {
   CARBON_KIND_SWITCH(import_inst) {
     case SemIR::BindAlias::Kind:
-    case SemIR::BindName::Kind:
+    case SemIR::RefBinding::Kind:
     case SemIR::BindSymbolicName::Kind:
+    case SemIR::ValueBinding::Kind:
     case SemIR::ExportDecl::Kind: {
       auto bind_inst = import_inst.As<SemIR::AnyBindNameOrExportDecl>();
       return GetImportNameForEntity(

toolchain/check/import_ref.cpp

@@ -3191,7 +3191,7 @@ static auto TryResolveInstCanonical(ImportRefResolver& resolver,
   if (!inst_constant_id.is_constant()) {
     // TODO: Import of non-constant BindNames happens when importing `let`
     // declarations.
-    CARBON_CHECK(resolver.import_insts().Is<SemIR::BindName>(inst_id),
+    CARBON_CHECK(resolver.import_insts().Is<SemIR::AnyBindName>(inst_id),
                  "TryResolveInst on non-constant instruction {0}", inst_id);
     return ResolveResult::Done(SemIR::ConstantId::NotConstant);
   }
@@ -3235,9 +3235,6 @@ static auto TryResolveInstCanonical(ImportRefResolver& resolver,
     case CARBON_KIND(SemIR::BindAlias inst): {
       return TryResolveTypedInst(resolver, inst);
     }
-    case CARBON_KIND(SemIR::BindingPattern inst): {
-      return TryResolveTypedInst(resolver, inst, constant_inst_id);
-    }
     case CARBON_KIND(SemIR::BindSymbolicName inst): {
       return TryResolveTypedInst(resolver, inst);
     }
@@ -3343,6 +3340,9 @@ static auto TryResolveInstCanonical(ImportRefResolver& resolver,
     case CARBON_KIND(SemIR::PointerType inst): {
       return TryResolveTypedInst(resolver, inst);
     }
+    case CARBON_KIND(SemIR::RefBindingPattern inst): {
+      return TryResolveTypedInst(resolver, inst, constant_inst_id);
+    }
     case CARBON_KIND(SemIR::RefParamPattern inst): {
       return TryResolveTypedInst(resolver, inst, constant_inst_id);
     }
@@ -3388,6 +3388,9 @@ static auto TryResolveInstCanonical(ImportRefResolver& resolver,
     case CARBON_KIND(SemIR::UnboundElementType inst): {
       return TryResolveTypedInst(resolver, inst);
     }
+    case CARBON_KIND(SemIR::ValueBindingPattern inst): {
+      return TryResolveTypedInst(resolver, inst, constant_inst_id);
+    }
     case CARBON_KIND(SemIR::ValueParamPattern inst): {
       return TryResolveTypedInst(resolver, inst, constant_inst_id);
     }

toolchain/check/merge.cpp

@@ -272,8 +272,9 @@ static auto CheckRedeclParam(Context& context, bool is_implicit_param,
              .new_id =
                  new_param_pattern.As<SemIR::VarPattern>().subpattern_id});
         break;
-      case SemIR::BindingPattern::Kind:
-      case SemIR::SymbolicBindingPattern::Kind: {
+      case SemIR::RefBindingPattern::Kind:
+      case SemIR::SymbolicBindingPattern::Kind:
+      case SemIR::ValueBindingPattern::Kind: {
         auto new_name_id =
             context.entity_names()
                 .Get(new_param_pattern.As<SemIR::AnyBindingPattern>()

toolchain/check/pattern.cpp

@@ -48,40 +48,46 @@ auto EndSubpatternAsNonExpr(Context& context) -> void {
 
 auto AddBindingPattern(Context& context, SemIR::LocId name_loc,
                        SemIR::NameId name_id, SemIR::TypeId type_id,
-                       SemIR::ExprRegionId type_region_id, bool is_generic,
-                       bool is_template) -> BindingPatternInfo {
+                       SemIR::ExprRegionId type_region_id,
+                       SemIR::InstKind pattern_kind, bool is_template)
+    -> BindingPatternInfo {
+  SemIR::InstKind bind_name_kind;
+  switch (pattern_kind) {
+    case SemIR::InstKind::RefBindingPattern:
+      bind_name_kind = SemIR::InstKind::RefBinding;
+      break;
+    case SemIR::InstKind::SymbolicBindingPattern:
+      bind_name_kind = SemIR::InstKind::BindSymbolicName;
+      break;
+    case SemIR::InstKind::ValueBindingPattern:
+      bind_name_kind = SemIR::InstKind::ValueBinding;
+      break;
+    default:
+      CARBON_FATAL("pattern_kind is not a binding pattern kind");
+  }
+  bool is_generic = pattern_kind == SemIR::SymbolicBindingPattern::Kind;
+
   auto entity_name_id = context.entity_names().AddSymbolicBindingName(
       name_id, context.scope_stack().PeekNameScopeId(),
       is_generic ? context.scope_stack().AddCompileTimeBinding()
                  : SemIR::CompileTimeBindIndex::None,
       is_template);
 
-  auto bind_id = SemIR::InstId::None;
-  if (is_generic) {
-    bind_id = AddInstInNoBlock<SemIR::BindSymbolicName>(
-        context, name_loc,
-        {.type_id = type_id,
-         .entity_name_id = entity_name_id,
-         .value_id = SemIR::InstId::None});
-  } else {
-    bind_id =
-        AddInstInNoBlock<SemIR::BindName>(context, name_loc,
-                                          {.type_id = type_id,
-                                           .entity_name_id = entity_name_id,
-                                           .value_id = SemIR::InstId::None});
-  }
+  auto bind_id = AddInstInNoBlock(
+      context,
+      SemIR::LocIdAndInst::UncheckedLoc(
+          name_loc, SemIR::AnyBindName{.kind = bind_name_kind,
+                                       .type_id = type_id,
+                                       .entity_name_id = entity_name_id,
+                                       .value_id = SemIR::InstId::None}));
 
   auto pattern_type_id = GetPatternType(context, type_id);
-  auto binding_pattern_id = SemIR::InstId::None;
-  if (is_generic) {
-    binding_pattern_id = AddPatternInst<SemIR::SymbolicBindingPattern>(
-        context, name_loc,
-        {.type_id = pattern_type_id, .entity_name_id = entity_name_id});
-  } else {
-    binding_pattern_id = AddPatternInst<SemIR::BindingPattern>(
-        context, name_loc,
-        {.type_id = pattern_type_id, .entity_name_id = entity_name_id});
-  }
+  auto binding_pattern_id = AddPatternInst(
+      context, SemIR::LocIdAndInst::UncheckedLoc(
+                   name_loc,
+                   SemIR::AnyBindingPattern{.kind = pattern_kind,
+                                            .type_id = pattern_type_id,
+                                            .entity_name_id = entity_name_id}));
 
   if (is_generic) {
     context.scope_stack().PushCompileTimeBinding(bind_id);
@@ -138,7 +144,7 @@ auto AddSelfParamPattern(Context& context, SemIR::LocId loc_id,
                          SemIR::TypeId type_id) -> SemIR::InstId {
   SemIR::InstId pattern_id =
       AddBindingPattern(context, loc_id, SemIR::NameId::SelfValue, type_id,
-                        type_expr_region_id, /*is_generic=*/false,
+                        type_expr_region_id, SemIR::ValueBindingPattern::Kind,
                         /*is_template=*/false)
           .pattern_id;
 

toolchain/check/pattern.h

@@ -37,10 +37,13 @@ struct BindingPatternInfo {
 
 // Creates a binding pattern. Returns the binding pattern and the bind name
 // instruction.
+// TODO: remove is_template once we have a separate InstKind for template
+// bindings.
 auto AddBindingPattern(Context& context, SemIR::LocId name_loc,
                        SemIR::NameId name_id, SemIR::TypeId type_id,
-                       SemIR::ExprRegionId type_region_id, bool is_generic,
-                       bool is_template) -> BindingPatternInfo;
+                       SemIR::ExprRegionId type_region_id,
+                       SemIR::InstKind pattern_kind, bool is_template)
+    -> BindingPatternInfo;
 
 // Creates storage for `var` patterns nested within the given pattern at the
 // current location in the output SemIR. For a `returned var`, this

toolchain/check/pattern_match.cpp

@@ -607,8 +607,9 @@ auto MatchContext::EmitPatternMatch(Context& context,
       });
   auto pattern = context.insts().Get(entry.pattern_id);
   CARBON_KIND_SWITCH(pattern) {
-    case SemIR::BindingPattern::Kind:
-    case SemIR::SymbolicBindingPattern::Kind: {
+    case SemIR::RefBindingPattern::Kind:
+    case SemIR::SymbolicBindingPattern::Kind:
+    case SemIR::ValueBindingPattern::Kind: {
       DoEmitPatternMatch(context, pattern.As<SemIR::AnyBindingPattern>(),
                          entry.pattern_id, entry);
       break;

toolchain/check/testdata/alias/basics.carbon

@@ -186,15 +186,15 @@ extern alias C = Class;
 // CHECK:STDOUT:   %b.ref: type = name_ref b, %b [concrete = constants.%C]
 // CHECK:STDOUT:   %c: type = bind_alias c, %b [concrete = constants.%C]
 // CHECK:STDOUT:   name_binding_decl {
-// CHECK:STDOUT:     %d.patt: %pattern_type = binding_pattern d [concrete]
+// CHECK:STDOUT:     %d.patt: %pattern_type = value_binding_pattern d [concrete]
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %c.ref: type = name_ref c, %c [concrete = constants.%C]
 // CHECK:STDOUT:   %.loc10_13.1: ref %C = temporary_storage
 // CHECK:STDOUT:   %.loc10_13.2: init %C = class_init (), %.loc10_13.1 [concrete = constants.%C.val]
 // CHECK:STDOUT:   %.loc10_13.3: ref %C = temporary %.loc10_13.1, %.loc10_13.2
 // CHECK:STDOUT:   %.loc10_13.4: ref %C = converted @__global_init.%.loc10, %.loc10_13.3
 // CHECK:STDOUT:   %.loc10_13.5: %C = bind_value %.loc10_13.4
-// CHECK:STDOUT:   %d: %C = bind_name d, %.loc10_13.5
+// CHECK:STDOUT:   %d: %C = value_binding d, %.loc10_13.5
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @__global_init() {

toolchain/check/testdata/alias/builtins.carbon

@@ -96,14 +96,14 @@ let a_test: bool = a;
 // CHECK:STDOUT:   %false: bool = bool_literal false [concrete = constants.%false]
 // CHECK:STDOUT:   %a: <error> = bind_alias a, <error> [concrete = <error>]
 // CHECK:STDOUT:   name_binding_decl {
-// CHECK:STDOUT:     %a_test.patt: %pattern_type.831 = binding_pattern a_test [concrete]
+// CHECK:STDOUT:     %a_test.patt: %pattern_type.831 = value_binding_pattern a_test [concrete]
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %.loc12_13.1: type = splice_block %.loc12_13.3 [concrete = bool] {
 // CHECK:STDOUT:     %Bool.call: init type = call constants.%Bool() [concrete = bool]
 // CHECK:STDOUT:     %.loc12_13.2: type = value_of_initializer %Bool.call [concrete = bool]
 // CHECK:STDOUT:     %.loc12_13.3: type = converted %Bool.call, %.loc12_13.2 [concrete = bool]
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %a_test: bool = bind_name a_test, <error> [concrete = <error>]
+// CHECK:STDOUT:   %a_test: bool = value_binding a_test, <error> [concrete = <error>]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @__global_init() {

toolchain/check/testdata/alias/export_name.carbon

@@ -210,12 +210,12 @@ var d: D* = &c;
 // CHECK:STDOUT:   %Core.import = import Core
 // CHECK:STDOUT:   %default.import = import <none>
 // CHECK:STDOUT:   name_binding_decl {
-// CHECK:STDOUT:     %d.patt: %pattern_type = binding_pattern d [concrete]
+// CHECK:STDOUT:     %d.patt: %pattern_type = ref_binding_pattern d [concrete]
 // CHECK:STDOUT:     %d.var_patt: %pattern_type = var_pattern %d.patt [concrete]
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %d.var: ref %C = var %d.var_patt [concrete]
 // CHECK:STDOUT:   %D.ref: type = name_ref D, imports.%Main.D [concrete = constants.%C]
-// CHECK:STDOUT:   %d: ref %C = bind_name d, %d.var [concrete = %d.var]
+// CHECK:STDOUT:   %d: ref %C = ref_binding d, %d.var [concrete = %d.var]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: class @C [from "export.carbon"] {
@@ -258,12 +258,12 @@ var d: D* = &c;
 // CHECK:STDOUT:   %Core.import = import Core
 // CHECK:STDOUT:   %default.import = import <none>
 // CHECK:STDOUT:   name_binding_decl {
-// CHECK:STDOUT:     %c.patt: <error> = binding_pattern c [concrete]
+// CHECK:STDOUT:     %c.patt: <error> = ref_binding_pattern c [concrete]
 // CHECK:STDOUT:     %c.var_patt: <error> = var_pattern %c.patt [concrete]
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %c.var: ref <error> = var %c.var_patt [concrete = <error>]
 // CHECK:STDOUT:   %C.ref: <error> = name_ref C, <error> [concrete = <error>]
-// CHECK:STDOUT:   %c: <error> = bind_name c, <error> [concrete = <error>]
+// CHECK:STDOUT:   %c: ref <error> = ref_binding c, <error> [concrete = <error>]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @__global_init() {
@@ -325,22 +325,22 @@ var d: D* = &c;
 // CHECK:STDOUT:   %Core.import = import Core
 // CHECK:STDOUT:   %default.import = import <none>
 // CHECK:STDOUT:   name_binding_decl {
-// CHECK:STDOUT:     %c.patt: %pattern_type.c48 = binding_pattern c [concrete]
+// CHECK:STDOUT:     %c.patt: %pattern_type.c48 = ref_binding_pattern c [concrete]
 // CHECK:STDOUT:     %c.var_patt: %pattern_type.c48 = var_pattern %c.patt [concrete]
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %c.var: ref %C = var %c.var_patt [concrete]
 // CHECK:STDOUT:   %C.ref: type = name_ref C, imports.%Main.C [concrete = constants.%C]
-// CHECK:STDOUT:   %c: ref %C = bind_name c, %c.var [concrete = %c.var]
+// CHECK:STDOUT:   %c: ref %C = ref_binding c, %c.var [concrete = %c.var]
 // CHECK:STDOUT:   name_binding_decl {
-// CHECK:STDOUT:     %d.patt: %pattern_type.44a = binding_pattern d [concrete]
+// CHECK:STDOUT:     %d.patt: %pattern_type.44a = ref_binding_pattern d [concrete]
 // CHECK:STDOUT:     %d.var_patt: %pattern_type.44a = var_pattern %d.patt [concrete]
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:   %d.var: ref %ptr.019 = var %d.var_patt [concrete]
 // CHECK:STDOUT:   %.loc8: type = splice_block %ptr [concrete = constants.%ptr.019] {
 // CHECK:STDOUT:     %D.ref: type = name_ref D, imports.%Main.D [concrete = constants.%C]
 // CHECK:STDOUT:     %ptr: type = ptr_type %D.ref [concrete = constants.%ptr.019]
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %d: ref %ptr.019 = bind_name d, %d.var [concrete = %d.var]
+// CHECK:STDOUT:   %d: ref %ptr.019 = ref_binding d, %d.var [concrete = %d.var]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: class @C [from "export_orig.carbon"] {