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

Code before:
                                 CoreIdentifier accessor_name)
    -> SemIR::InstId {
  auto accessor_name_id = context.core_identifiers().AddNameId(accessor_name);
  auto accessor_id =
      PerformMemberAccess(context, node_id, optional_id, accessor_name_id);
  return PerformCall(context, node_id, accessor_id, {});
}

auto HandleParseNode(Context& context, Parse::ForHeaderId node_id) -> bool {
  auto range_id = context.node_stack().PopExpr();
  auto pattern_block_id = context.node_stack().Pop<Parse::NodeKind::ForIn>();
  auto pattern_id = context.node_stack().PopPattern();
  auto start_node_id =
      context.node_stack().PopForSoloNodeId<Parse::NodeKind::ForHeaderStart>();

  // Convert the range expression to a value or reference so that we can use it
  // multiple times.
  // TODO: If this produces a temporary, its lifetime should presumably be
  // extended to cover the loop body.
  range_id = ConvertToValueOrRefExpr(context, range_id);

  // Create the cursor variable.
  // TODO: Produce a custom diagnostic if the range operand can't be used as a
  // range.
  // TODO: We need to allocate the `VarStorage` before building the operator.
  // The current order risks violates the preconditions on `Initialize` and
  // risks violating the topological ordering of insts.
  auto cursor_id =
      BuildUnaryOperator(context, node_id,
                         {.interface_name = CoreIdentifier::Iterate,
                          .op_name = CoreIdentifier::NewCursor},
                         range_id);
  auto cursor_type_id = context.insts().Get(cursor_id).type_id();
  auto cursor_var_id = AddInstWithCleanup<SemIR::VarStorage>(
      context, node_id,
      {.type_id = cursor_type_id, .pattern_id = SemIR::AbsoluteInstId::None});
  auto init_id = Initialize(context, node_id, cursor_var_id, cursor_id);
  AddInst<SemIR::Assign>(context, node_id,
                         {.lhs_id = cursor_var_id, .rhs_id = init_id});

  // Start emitting the loop header block.
  auto loop_header_id = StartLoopHeader(context, start_node_id);

  // Call `<range>.(Iterate.Next)(&cursor)`.
  auto cursor_type_inst_id = context.types().GetTypeInstId(cursor_type_id);
  auto cursor_addr_id = AddInst<SemIR::AddrOf>(
      context, node_id,
      {.type_id = GetPointerType(context, cursor_type_inst_id),
       .lvalue_id = cursor_var_id});
  auto element_id =
      BuildBinaryOperator(context, node_id,

Review comment (suggestion):
```suggestion
  // The current order risks violating the preconditions on `Initialize` and
```

or

```suggestion
  // The current order violates the preconditions on `Initialize` and
```

Code after:
                                 CoreIdentifier accessor_name)
    -> SemIR::InstId {
  auto accessor_name_id = context.core_identifiers().AddNameId(accessor_name);
  auto accessor_id =
      PerformMemberAccess(context, node_id, optional_id, accessor_name_id);
  return PerformCall(context, node_id, accessor_id, {});
}

auto HandleParseNode(Context& context, Parse::ForHeaderId node_id) -> bool {
  auto range_id = context.node_stack().PopExpr();
  auto pattern_block_id = context.node_stack().Pop<Parse::NodeKind::ForIn>();
  auto pattern_id = context.node_stack().PopPattern();
  auto start_node_id =
      context.node_stack().PopForSoloNodeId<Parse::NodeKind::ForHeaderStart>();

  // Convert the range expression to a value or reference so that we can use it
  // multiple times.
  // TODO: If this produces a temporary, its lifetime should presumably be
  // extended to cover the loop body.
  range_id = ConvertToValueOrRefExpr(context, range_id);

  // Create the cursor variable.
  // TODO: Produce a custom diagnostic if the range operand can't be used as a
  // range.
  // TODO: We need to allocate the `VarStorage` before building the operator.
  // The current order risks violating the preconditions on `Initialize` and
  // risks violating the topological ordering of insts.
  auto cursor_id =
      BuildUnaryOperator(context, node_id,
                         {.interface_name = CoreIdentifier::Iterate,
                          .op_name = CoreIdentifier::NewCursor},
                         range_id);
  auto cursor_type_id = context.insts().Get(cursor_id).type_id();
  auto cursor_var_id = AddInstWithCleanup<SemIR::VarStorage>(
      context, node_id,
      {.type_id = cursor_type_id, .pattern_id = SemIR::AbsoluteInstId::None});
  auto init_id = Initialize(context, node_id, cursor_var_id, cursor_id);
  AddInst<SemIR::Assign>(context, node_id,
                         {.lhs_id = cursor_var_id, .rhs_id = init_id});

  // Start emitting the loop header block.
  auto loop_header_id = StartLoopHeader(context, start_node_id);

  // Call `<range>.(Iterate.Next)(&cursor)`.
  auto cursor_type_inst_id = context.types().GetTypeInstId(cursor_type_id);
  auto cursor_addr_id = AddInst<SemIR::AddrOf>(
      context, node_id,
      {.type_id = GetPointerType(context, cursor_type_inst_id),
       .lvalue_id = cursor_var_id});
  auto element_id =
      BuildBinaryOperator(context, node_id,