Improve backtrace for lowering crashes. (#5651)

Factor out the logic for mapping from a `LocId` into a diagnostic
location from check into sem_ir so it can be reused by lowering. Include
the function and instruction being lowered in the pretty stack trace.
Example stack trace:

```carbon
2.      filename: examples/sieve.carbon
3.      core/prelude/types/int.carbon:213:3: lowering function Core.Op(Core.IntLiteral as Core.ImplicitAs(i32))
            fn Op[addr self: Self*](other: Self) = "int.sadd_assign";
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4.      core/prelude/operators/arithmetic.carbon:22:27: lowering call
            fn Op[addr self: Self*](other: Other);
                                    ^~~~~~~~~~~~
```

Author: zygoloid

toolchain/check/BUILD

@@ -285,6 +285,7 @@ cc_library(
         "//toolchain/lex:token_index",
         "//toolchain/parse:tree",
         "//toolchain/sem_ir:absolute_node_id",
+        "//toolchain/sem_ir:diagnostic_loc_converter",
         "//toolchain/sem_ir:file",
         "//toolchain/sem_ir:stringify",
         "//toolchain/sem_ir:typed_insts",

toolchain/check/check_unit.cpp

@@ -377,7 +377,7 @@ auto CheckUnit::ProcessNodeIds() -> bool {
     const auto& tree = tree_and_subtrees_getter_();
     auto converted = tree.NodeToDiagnosticLoc(node_id, /*token_only=*/false);
     converted.loc.FormatLocation(output);
-    output << "checking " << context_.parse_tree().node_kind(node_id) << "\n";
+    output << "Checking " << context_.parse_tree().node_kind(node_id) << "\n";
     // Crash output has a tab indent; try to indent slightly past that.
     converted.loc.FormatSnippet(output, /*indent=*/10);
   });

toolchain/check/diagnostic_emitter.cpp

@@ -19,8 +19,12 @@ namespace Carbon::Check {
 auto DiagnosticEmitter::ConvertLoc(LocIdForDiagnostics loc_id,
                                    ContextFnT context_fn) const
     -> Diagnostics::ConvertedLoc {
-  auto converted =
-      ConvertLocImpl(loc_id.loc_id(), loc_id.is_token_only(), context_fn);
+  auto [imports, converted] = loc_converter_.ConvertWithImports(
+      loc_id.loc_id(), loc_id.is_token_only());
+  for (const auto& import : imports) {
+    CARBON_DIAGNOSTIC(InImport, LocationInfo, "in import");
+    context_fn(import.loc, InImport);
+  }
 
   // Use the token when possible, but -1 is the default value.
   auto last_offset = -1;
@@ -40,73 +44,6 @@ auto DiagnosticEmitter::ConvertLoc(LocIdForDiagnostics loc_id,
   return converted;
 }
 
-auto DiagnosticEmitter::ConvertLocImpl(SemIR::LocId loc_id, bool is_token_only,
-                                       ContextFnT context_fn) const
-    -> Diagnostics::ConvertedLoc {
-  llvm::SmallVector<SemIR::AbsoluteNodeId> absolute_node_ids =
-      SemIR::GetAbsoluteNodeId(sem_ir_, loc_id);
-
-  auto final_node_id = absolute_node_ids.pop_back_val();
-  for (const auto& absolute_node_id : absolute_node_ids) {
-    if (!absolute_node_id.node_id().has_value()) {
-      // TODO: Add an "In implicit import of prelude." note for the case where
-      // we don't have a location.
-      continue;
-    }
-    // TODO: Include the name of the imported library in the diagnostic.
-    auto diag_loc =
-        ConvertLocInFile(absolute_node_id, is_token_only, context_fn);
-    AddInImport(diag_loc.loc, context_fn);
-  }
-
-  return ConvertLocInFile(final_node_id, is_token_only, context_fn);
-}
-
-auto DiagnosticEmitter::ConvertLocInFile(SemIR::AbsoluteNodeId absolute_node_id,
-                                         bool token_only,
-                                         ContextFnT context_fn) const
-    -> Diagnostics::ConvertedLoc {
-  if (absolute_node_id.check_ir_id() == SemIR::CheckIRId::Cpp) {
-    // Special handling of Clang source locations.
-    CARBON_CHECK(sem_ir_->import_cpps().size() > 0);
-    // TODO: Use information on the specific C++ import extract from Clang error
-    // message and propagated here instead of using first C++ import
-    // arbitrarily.
-    Parse::NodeId import_node_id =
-        sem_ir_->import_cpps().values().begin()->node_id;
-    AddInImport(ConvertLocInCarbonFile(sem_ir_->check_ir_id(), import_node_id,
-                                       /*token_only=*/false)
-                    .loc,
-                context_fn);
-
-    clang::SourceLocation clang_loc = sem_ir_->clang_source_locs().Get(
-        absolute_node_id.clang_source_loc_id());
-
-    CARBON_CHECK(sem_ir_->cpp_ast());
-    clang::PresumedLoc presumed_loc =
-        sem_ir_->cpp_ast()->getSourceManager().getPresumedLoc(clang_loc);
-
-    return Diagnostics::ConvertedLoc{
-        .loc = {.filename = presumed_loc.getFilename(),
-                .line_number = static_cast<int32_t>(presumed_loc.getLine())},
-        // TODO: Set `last_byte_offset` based on the `import Cpp` location.
-        .last_byte_offset = 0};
-  }
-
-  return ConvertLocInCarbonFile(absolute_node_id.check_ir_id(),
-                                absolute_node_id.node_id(), token_only);
-}
-
-auto DiagnosticEmitter::ConvertLocInCarbonFile(SemIR::CheckIRId check_ir_id,
-                                               Parse::NodeId node_id,
-                                               bool token_only) const
-    -> Diagnostics::ConvertedLoc {
-  CARBON_CHECK(check_ir_id != SemIR::CheckIRId::Cpp);
-  const auto& tree_and_subtrees =
-      tree_and_subtrees_getters_[check_ir_id.index]();
-  return tree_and_subtrees.NodeToDiagnosticLoc(node_id, token_only);
-}
-
 auto DiagnosticEmitter::ConvertArg(llvm::Any arg) const -> llvm::Any {
   if (auto* library_name_id = llvm::any_cast<SemIR::LibraryNameId>(&arg)) {
     std::string library_name;
@@ -178,10 +115,4 @@ auto DiagnosticEmitter::ConvertArg(llvm::Any arg) const -> llvm::Any {
   return DiagnosticEmitterBase::ConvertArg(arg);
 }
 
-auto DiagnosticEmitter::AddInImport(Diagnostics::Loc loc, ContextFnT context_fn)
-    -> void {
-  CARBON_DIAGNOSTIC(InImport, LocationInfo, "in import");
-  context_fn(loc, InImport);
-}
-
 }  // namespace Carbon::Check

toolchain/check/diagnostic_emitter.h

@@ -9,10 +9,7 @@
 #include "toolchain/check/diagnostic_helpers.h"
 #include "toolchain/diagnostics/diagnostic_emitter.h"
 #include "toolchain/lex/token_index.h"
-#include "toolchain/parse/tree_and_subtrees.h"
-#include "toolchain/sem_ir/absolute_node_id.h"
-#include "toolchain/sem_ir/file.h"
-#include "toolchain/sem_ir/ids.h"
+#include "toolchain/sem_ir/diagnostic_loc_converter.h"
 
 namespace Carbon::Check {
 
@@ -24,8 +21,8 @@ class DiagnosticEmitter : public DiagnosticEmitterBase {
       llvm::ArrayRef<Parse::GetTreeAndSubtreesFn> tree_and_subtrees_getters,
       const SemIR::File* sem_ir)
       : DiagnosticEmitterBase(consumer),
-        tree_and_subtrees_getters_(tree_and_subtrees_getters),
-        sem_ir_(sem_ir) {}
+        sem_ir_(sem_ir),
+        loc_converter_(tree_and_subtrees_getters, sem_ir) {}
 
   // If a byte offset is past the current last byte offset, advances forward.
   // Earlier offsets are ignored.
@@ -47,31 +44,12 @@ class DiagnosticEmitter : public DiagnosticEmitterBase {
       -> Diagnostics::ConvertedLoc override;
 
  private:
-  // Implements `ConvertLoc`, but without `last_token_` applied.
-  auto ConvertLocImpl(SemIR::LocId loc_id, bool is_token_only,
-                      ContextFnT context_fn) const -> Diagnostics::ConvertedLoc;
-
-  // Converts an `absolute_node_id` in either a Carbon file or C++ import to a
-  // diagnostic location.
-  auto ConvertLocInFile(SemIR::AbsoluteNodeId absolute_node_id, bool token_only,
-                        ContextFnT context_fn) const
-      -> Diagnostics::ConvertedLoc;
-
-  // Converts a `node_id` corresponding to a specific sem_ir to a diagnostic
-  // location.
-  auto ConvertLocInCarbonFile(SemIR::CheckIRId check_ir_id,
-                              Parse::NodeId node_id, bool token_only) const
-      -> Diagnostics::ConvertedLoc;
-
-  // Adds `in import` note.
-  static auto AddInImport(Diagnostics::Loc loc, ContextFnT context_fn) -> void;
-
-  // Converters for each SemIR.
-  llvm::ArrayRef<Parse::GetTreeAndSubtreesFn> tree_and_subtrees_getters_;
-
   // The current SemIR being processed.
   const SemIR::File* sem_ir_;
 
+  // Converter for locations.
+  SemIR::DiagnosticLocConverter loc_converter_;
+
   // The last token encountered during processing.
   Lex::TokenIndex last_token_ = Lex::TokenIndex::None;
 };

toolchain/driver/compile_subcommand.cpp

@@ -708,13 +708,11 @@ auto CompilationUnit::RunLower() -> void {
     // TODO: Consider disabling instruction naming by default if we're not
     // producing textual LLVM IR.
     SemIR::InstNamer inst_namer(&*sem_ir_);
-    std::optional<llvm::ArrayRef<Parse::GetTreeAndSubtreesFn>> subtrees;
-    if (options_->include_debug_info) {
-      subtrees = cache_->tree_and_subtrees_getters();
-    }
-    module_ = Lower::LowerToLLVM(*llvm_context_, driver_env_->fs, subtrees,
-                                 input_filename_, *sem_ir_, &inst_namer,
-                                 vlog_stream_);
+    llvm::ArrayRef<Parse::GetTreeAndSubtreesFn> subtrees =
+        cache_->tree_and_subtrees_getters();
+    module_ = Lower::LowerToLLVM(
+        *llvm_context_, driver_env_->fs, options_->include_debug_info, subtrees,
+        input_filename_, *sem_ir_, &inst_namer, vlog_stream_);
   });
   if (vlog_stream_) {
     CARBON_VLOG("*** llvm::Module ***\n");
@@ -846,7 +844,7 @@ auto CompilationUnit::LogCall(llvm::StringLiteral logging_label,
                               llvm::StringLiteral timing_label,
                               llvm::function_ref<auto()->void> fn) -> void {
   PrettyStackTraceFunction trace_file([&](llvm::raw_ostream& out) {
-    out << "filename: " << input_filename_ << "\n";
+    out << "Filename: " << input_filename_ << "\n";
   });
   CARBON_VLOG("*** {0}: {1} ***\n", logging_label, input_filename_);
   Timings::ScopedTiming timing(timings_ ? &*timings_ : nullptr, timing_label);

toolchain/lower/BUILD

@@ -53,12 +53,15 @@ cc_library(
         "//common:vlog",
         "//toolchain/base:fixed_size_value_store",
         "//toolchain/base:kind_switch",
+        "//toolchain/base:pretty_stack_trace_function",
         "//toolchain/parse:tree",
         "//toolchain/sem_ir:absolute_node_id",
+        "//toolchain/sem_ir:diagnostic_loc_converter",
         "//toolchain/sem_ir:entry_point",
         "//toolchain/sem_ir:expr_info",
         "//toolchain/sem_ir:file",
         "//toolchain/sem_ir:inst_namer",
+        "//toolchain/sem_ir:stringify",
         "//toolchain/sem_ir:typed_insts",
         "@llvm-project//clang:ast",
         "@llvm-project//clang:basic",

toolchain/lower/context.cpp

@@ -13,21 +13,20 @@
 
 namespace Carbon::Lower {
 
-Context::Context(llvm::LLVMContext& llvm_context,
-                 llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs,
-                 std::optional<llvm::ArrayRef<Parse::GetTreeAndSubtreesFn>>
-                     tree_and_subtrees_getters_for_debug_info,
-                 llvm::StringRef module_name, llvm::raw_ostream* vlog_stream)
+Context::Context(
+    llvm::LLVMContext& llvm_context,
+    llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs, bool want_debug_info,
+    llvm::ArrayRef<Parse::GetTreeAndSubtreesFn> tree_and_subtrees_getters,
+    llvm::StringRef module_name, llvm::raw_ostream* vlog_stream)
     : llvm_context_(&llvm_context),
       llvm_module_(std::make_unique<llvm::Module>(module_name, llvm_context)),
       file_system_(std::move(fs)),
       di_builder_(*llvm_module_),
       di_compile_unit_(
-          tree_and_subtrees_getters_for_debug_info
+          want_debug_info
               ? BuildDICompileUnit(module_name, *llvm_module_, di_builder_)
               : nullptr),
-      tree_and_subtrees_getters_for_debug_info_(
-          tree_and_subtrees_getters_for_debug_info),
+      tree_and_subtrees_getters_(tree_and_subtrees_getters),
       vlog_stream_(vlog_stream) {}
 
 auto Context::GetFileContext(const SemIR::File* file,
@@ -79,8 +78,7 @@ auto Context::BuildDICompileUnit(llvm::StringRef module_name,
 
 auto Context::GetLocForDI(SemIR::AbsoluteNodeId abs_node_id) -> LocForDI {
   const auto& tree_and_subtrees =
-      (*tree_and_subtrees_getters_for_debug_info_)[abs_node_id.check_ir_id()
-                                                       .index]();
+      tree_and_subtrees_getters()[abs_node_id.check_ir_id().index]();
   const auto& tokens = tree_and_subtrees.tree().tokens();
 
   if (abs_node_id.node_id().has_value()) {

toolchain/lower/context.h

@@ -41,11 +41,11 @@ class Context {
     SemIR::SpecificId specific_id;
   };
 
-  explicit Context(llvm::LLVMContext& llvm_context,
-                   llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs,
-                   std::optional<llvm::ArrayRef<Parse::GetTreeAndSubtreesFn>>
-                       tree_and_subtrees_getters_for_debug_info,
-                   llvm::StringRef module_name, llvm::raw_ostream* vlog_stream);
+  explicit Context(
+      llvm::LLVMContext& llvm_context,
+      llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs, bool want_debug_info,
+      llvm::ArrayRef<Parse::GetTreeAndSubtreesFn> tree_and_subtrees_getters,
+      llvm::StringRef module_name, llvm::raw_ostream* vlog_stream);
 
   // Gets or creates the `FileContext` for a given SemIR file. If an
   // `inst_namer` is specified the first time this is called for a file, it will
@@ -95,6 +95,10 @@ class Context {
   }
   auto di_builder() -> llvm::DIBuilder& { return di_builder_; }
   auto di_compile_unit() -> llvm::DICompileUnit* { return di_compile_unit_; }
+  auto tree_and_subtrees_getters()
+      -> llvm::ArrayRef<Parse::GetTreeAndSubtreesFn> {
+    return tree_and_subtrees_getters_;
+  }
 
   auto printf_int_format_string() -> llvm::Value* {
     return printf_int_format_string_;
@@ -128,9 +132,8 @@ class Context {
   // The DICompileUnit, if any - null implies debug info is not being emitted.
   llvm::DICompileUnit* di_compile_unit_;
 
-  // The trees are only provided when debug info should be emitted.
-  std::optional<llvm::ArrayRef<Parse::GetTreeAndSubtreesFn>>
-      tree_and_subtrees_getters_for_debug_info_;
+  // Parse trees. Used for debug information and crash diagnostics.
+  llvm::ArrayRef<Parse::GetTreeAndSubtreesFn> tree_and_subtrees_getters_;
 
   // The optional vlog stream.
   llvm::raw_ostream* vlog_stream_;

toolchain/lower/file_context.cpp

@@ -19,10 +19,12 @@
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"
 #include "toolchain/base/kind_switch.h"
+#include "toolchain/base/pretty_stack_trace_function.h"
 #include "toolchain/lower/constant.h"
 #include "toolchain/lower/function_context.h"
 #include "toolchain/lower/mangler.h"
 #include "toolchain/sem_ir/absolute_node_id.h"
+#include "toolchain/sem_ir/diagnostic_loc_converter.h"
 #include "toolchain/sem_ir/entry_point.h"
 #include "toolchain/sem_ir/expr_info.h"
 #include "toolchain/sem_ir/file.h"
@@ -33,6 +35,7 @@
 #include "toolchain/sem_ir/inst_categories.h"
 #include "toolchain/sem_ir/inst_kind.h"
 #include "toolchain/sem_ir/pattern.h"
+#include "toolchain/sem_ir/stringify.h"
 #include "toolchain/sem_ir/typed_insts.h"
 
 namespace Carbon::Lower {
@@ -757,6 +760,26 @@ auto FileContext::BuildFunctionBody(SemIR::FunctionId function_id,
                                     FileContext& definition_context,
                                     const SemIR::Function& definition_function)
     -> void {
+  // On crash, report the function we were lowering.
+  PrettyStackTraceFunction stack_trace_entry([&](llvm::raw_ostream& output) {
+    SemIR::DiagnosticLocConverter converter(
+        context().tree_and_subtrees_getters(), &sem_ir());
+    auto converted =
+        converter.Convert(SemIR::LocId(declaration_function.definition_id),
+                          /*token_only=*/false);
+    converted.loc.FormatLocation(output);
+    output << "Lowering function ";
+    if (specific_id.has_value()) {
+      output << SemIR::StringifySpecific(sem_ir(), specific_id);
+    } else {
+      output << SemIR::StringifyConstantInst(
+          sem_ir(), declaration_function.definition_id);
+    }
+    output << "\n";
+    // Crash output has a tab indent; try to indent slightly past that.
+    converted.loc.FormatSnippet(output, /*indent=*/10);
+  });
+
   // Note that `definition_function` is potentially from a different SemIR::File
   // than the one that this file context represents. Any lowering done for
   // values derived from `definition_function` should use `definition_context`

toolchain/lower/function_context.cpp

@@ -6,6 +6,8 @@
 
 #include "common/vlog.h"
 #include "toolchain/base/kind_switch.h"
+#include "toolchain/base/pretty_stack_trace_function.h"
+#include "toolchain/sem_ir/diagnostic_loc_converter.h"
 #include "toolchain/sem_ir/file.h"
 #include "toolchain/sem_ir/generic.h"
 
@@ -55,7 +57,25 @@ auto FunctionContext::TryToReuseBlock(SemIR::InstBlockId block_id,
 }
 
 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);
   }
 }