Repository: carbon-language/carbon-lang
File: toolchain/check/handle_impl.cpp (C++)

Code before:
  };

  parent_scope.AddExtendedScope(constraint_inst_id);
}

// Pops the parameters of an `impl`, forming a `NameComponent` with no
// associated name that describes them.
static auto PopImplIntroducerAndParamsAsNameComponent(
    Context& context, Parse::AnyImplDeclId end_of_decl_node_id)
    -> NameComponent {
  auto [implicit_params_loc_id, implicit_param_patterns_id] =
      context.node_stack().PopWithNodeIdIf<Parse::NodeKind::ImplForall>();

  if (implicit_param_patterns_id) {
    // Emit the `forall` match. This shouldn't produce any valid `Call` params,
    // because `impl`s are never actually called at runtime.
    auto call_params_id =
        CalleePatternMatch(context, *implicit_param_patterns_id,
                           SemIR::InstBlockId::Invalid, SemIR::InstId::Invalid);
    CARBON_CHECK(call_params_id == SemIR::InstBlockId::Empty ||
                 context.inst_blocks()
                     .Get(call_params_id)
                     .drop_while([](SemIR::InstId inst_id) {
                       return inst_id == SemIR::ErrorInst::SingletonInstId;
                     })
                     .empty());
  }

  Parse::NodeId first_param_node_id =
      context.node_stack().PopForSoloNodeId<Parse::NodeKind::ImplIntroducer>();
  Parse::NodeId last_param_node_id = end_of_decl_node_id;

  return {
      .name_loc_id = Parse::NodeId::Invalid,
      .name_id = SemIR::NameId::Invalid,
      .first_param_node_id = first_param_node_id,
      .last_param_node_id = last_param_node_id,
      .implicit_params_loc_id = implicit_params_loc_id,
      .implicit_param_patterns_id =
          implicit_param_patterns_id.value_or(SemIR::InstBlockId::Invalid),
      .params_loc_id = Parse::NodeId::Invalid,
      .param_patterns_id = SemIR::InstBlockId::Invalid,
      .call_params_id = SemIR::InstBlockId::Invalid,
      .return_slot_pattern_id = SemIR::InstId::Invalid,
      .pattern_block_id = context.pattern_block_stack().Pop(),
  };
}

static auto MergeImplRedecl(Context& context, SemIR::Impl& new_impl,
                            SemIR::ImplId prev_impl_id) -> bool {
  auto& prev_impl = context.impls().Get(prev_impl_id);

Review comment (suggestion):
```suggestion
                 llvm::count_if(context.inst_blocks().Get(call_params_id),
                                [](SemIR::InstId inst_id) {
                                  return inst_id !=
                                         SemIR::ErrorInst::SingletonInstId;
                                }) == 0);
```

Was wondering if a count-based version might be appealing as a way to avoid the intermediary arrayref? Though this only happens on invalid code, so it seems minor -- I think this is basically whichever approach you consider more readable.

Code after:
  };

  parent_scope.AddExtendedScope(constraint_inst_id);
}

// Pops the parameters of an `impl`, forming a `NameComponent` with no
// associated name that describes them.
static auto PopImplIntroducerAndParamsAsNameComponent(
    Context& context, Parse::AnyImplDeclId end_of_decl_node_id)
    -> NameComponent {
  auto [implicit_params_loc_id, implicit_param_patterns_id] =
      context.node_stack().PopWithNodeIdIf<Parse::NodeKind::ImplForall>();

  if (implicit_param_patterns_id) {
    // Emit the `forall` match. This shouldn't produce any valid `Call` params,
    // because `impl`s are never actually called at runtime.
    auto call_params_id =
        CalleePatternMatch(context, *implicit_param_patterns_id,
                           SemIR::InstBlockId::Invalid, SemIR::InstId::Invalid);
    CARBON_CHECK(call_params_id == SemIR::InstBlockId::Empty ||
                 llvm::all_of(context.inst_blocks().Get(call_params_id),
                              [](SemIR::InstId inst_id) {
                                return inst_id ==
                                       SemIR::ErrorInst::SingletonInstId;
                              }));
  }

  Parse::NodeId first_param_node_id =
      context.node_stack().PopForSoloNodeId<Parse::NodeKind::ImplIntroducer>();
  Parse::NodeId last_param_node_id = end_of_decl_node_id;

  return {
      .name_loc_id = Parse::NodeId::Invalid,
      .name_id = SemIR::NameId::Invalid,
      .first_param_node_id = first_param_node_id,
      .last_param_node_id = last_param_node_id,
      .implicit_params_loc_id = implicit_params_loc_id,
      .implicit_param_patterns_id =
          implicit_param_patterns_id.value_or(SemIR::InstBlockId::Invalid),
      .params_loc_id = Parse::NodeId::Invalid,
      .param_patterns_id = SemIR::InstBlockId::Invalid,
      .call_params_id = SemIR::InstBlockId::Invalid,
      .return_slot_pattern_id = SemIR::InstId::Invalid,
      .pattern_block_id = context.pattern_block_stack().Pop(),
  };
}

static auto MergeImplRedecl(Context& context, SemIR::Impl& new_impl,
                            SemIR::ImplId prev_impl_id) -> bool {
  auto& prev_impl = context.impls().Get(prev_impl_id);