Repository: carbon-language/carbon-lang
File: toolchain/lower/function_context.cpp (C++)

Code before:
auto FunctionContext::TryToReuseBlock(SemIR::InstBlockId block_id,
                                      llvm::BasicBlock* block) -> bool {
  if (!blocks_.Insert(block_id, block).is_inserted()) {
    return false;
  }
  if (block == synthetic_block_) {
    synthetic_block_ = nullptr;
  }
  if (const auto* inst_namer = file_context_->inst_namer()) {
    block->setName(inst_namer->GetUnscopedLabelFor(block_id));
  }
  return true;
}

auto FunctionContext::LowerBlockContents(SemIR::InstBlockId block_id) -> void {
  auto crash_inst_id = SemIR::InstId::None;

  // On crash, report the instruction we were lowering.
  PrettyStackTraceFunction stack_trace_entry([&](llvm::raw_ostream& output) {
    SemIR::DiagnosticLocConverter converter(
        file_context_->context().tree_and_subtrees_getters(), &sem_ir());
    auto [imports, converted] =
        converter.Convert(SemIR::LocId(crash_inst_id), /*is_token_only=*/false);
    converted.loc.FormatLocation(output);
    // TODO: Format SemIR for the instruction we were lowering?
    output << "lowering "
           << sem_ir().insts().Get(crash_inst_id).kind().ir_name() << "\n";
    // Crash output has a tab indent; try to indent slightly past that.
    converted.loc.FormatSnippet(output, /*indent=*/10);
  });

  for (auto inst_id : sem_ir().inst_blocks().Get(block_id)) {
    crash_inst_id = inst_id;
    LowerInst(inst_id);
  }
}

// Handles typed instructions for LowerInst. Many instructions lower using
// HandleInst, but others are unsupported or have trivial lowering.
//
// This only calls HandleInst for versions that should have implementations. A
// different approach would be to have the logic below implemented as HandleInst
// overloads. However, forward declarations of HandleInst exist for all `InstT`
// types, which would make getting the right overload resolution complex.
template <typename InstT>
static auto LowerInstHelper(FunctionContext& context, SemIR::InstId inst_id,
                            InstT inst) -> void {
  if constexpr (!InstT::Kind.is_lowered()) {
    CARBON_FATAL(
        "Encountered an instruction that isn't expected to lower. It's "
        "possible that logic needs to be changed in order to stop showing this "

Review comment (suggestion):
I know "filename" is lowercased, but I've been wondering if it should be capital-cased because "Program arguments" is capital-cased and we don't control it (actually, filename is a bit of an exception)

Code after:
auto FunctionContext::TryToReuseBlock(SemIR::InstBlockId block_id,
                                      llvm::BasicBlock* block) -> bool {
  if (!blocks_.Insert(block_id, block).is_inserted()) {
    return false;
  }
  if (block == synthetic_block_) {
    synthetic_block_ = nullptr;
  }
  if (const auto* inst_namer = file_context_->inst_namer()) {
    block->setName(inst_namer->GetUnscopedLabelFor(block_id));
  }
  return true;
}

auto FunctionContext::LowerBlockContents(SemIR::InstBlockId block_id) -> void {
  auto inst_id_for_stack_trace = SemIR::InstId::None;

  // On crash, report the instruction we were lowering.
  PrettyStackTraceFunction stack_trace_entry([&](llvm::raw_ostream& output) {
    SemIR::DiagnosticLocConverter converter(
        file_context_->context().tree_and_subtrees_getters(), &sem_ir());
    auto converted = converter.Convert(SemIR::LocId(inst_id_for_stack_trace),
                                       /*token_only=*/false);
    converted.loc.FormatLocation(output);
    // TODO: Format SemIR for the instruction we were lowering?
    output << "Lowering "
           << sem_ir().insts().Get(inst_id_for_stack_trace).kind().ir_name()
           << "\n";
    // Crash output has a tab indent; try to indent slightly past that.
    converted.loc.FormatSnippet(output, /*indent=*/10);
  });

  for (auto inst_id : sem_ir().inst_blocks().Get(block_id)) {
    inst_id_for_stack_trace = inst_id;
    LowerInst(inst_id);
  }
}

// Handles typed instructions for LowerInst. Many instructions lower using
// HandleInst, but others are unsupported or have trivial lowering.
//
// This only calls HandleInst for versions that should have implementations. A
// different approach would be to have the logic below implemented as HandleInst
// overloads. However, forward declarations of HandleInst exist for all `InstT`
// types, which would make getting the right overload resolution complex.
template <typename InstT>
static auto LowerInstHelper(FunctionContext& context, SemIR::InstId inst_id,
                            InstT inst) -> void {
  if constexpr (!InstT::Kind.is_lowered()) {
    CARBON_FATAL(
        "Encountered an instruction that isn't expected to lower. It's "