Quellcode durchsuchen

Move the logic in `TryConvertClangDiagnosticLoc()` to `ConvertLocInFile()` (#5391)

Make `AbsoluteNodeId` support Clang source locations.

Part of #5245.
Boaz Brickner vor 1 Jahr
Ursprung
Commit
2aa5fbfa4a

+ 21 - 39
toolchain/check/diagnostic_emitter.cpp

@@ -18,15 +18,6 @@ namespace Carbon::Check {
 auto DiagnosticEmitter::ConvertLoc(LocIdForDiagnostics loc_id,
                                    ContextFnT context_fn) const
     -> Diagnostics::ConvertedLoc {
-  // TODO: Instead of special casing Clang location here, support it within
-  // `GetAbsoluteNodeId()`. See discussion in
-  // https://github.com/carbon-language/carbon-lang/pull/5262/files/20a3f9dcfab5c6f6c5089554fd5e22d5f1ca75a3#r2040308805.
-  auto converted_clang_loc =
-      TryConvertClangDiagnosticLoc(static_cast<SemIR::LocId>(loc_id));
-  if (converted_clang_loc) {
-    return *converted_clang_loc;
-  }
-
   auto converted =
       ConvertLocImpl(static_cast<SemIR::LocId>(loc_id), context_fn);
 
@@ -58,7 +49,7 @@ auto DiagnosticEmitter::ConvertLocImpl(SemIR::LocId 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()) {
+    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;
@@ -72,40 +63,31 @@ auto DiagnosticEmitter::ConvertLocImpl(SemIR::LocId loc_id,
   return ConvertLocInFile(final_node_id, token_only, context_fn);
 }
 
-auto DiagnosticEmitter::TryConvertClangDiagnosticLoc(SemIR::LocId loc_id) const
-    -> std::optional<Diagnostics::ConvertedLoc> {
-  if (loc_id.kind() != SemIR::LocId::Kind::ImportIRInstId) {
-    return std::nullopt;
-  }
-
-  SemIR::ImportIRInst import_ir_inst =
-      sem_ir_->import_ir_insts().Get(loc_id.import_ir_inst_id());
-
-  if (import_ir_inst.ir_id() != SemIR::ImportIRId::Cpp) {
-    return std::nullopt;
-  }
-
-  clang::SourceLocation clang_loc =
-      sem_ir_->clang_source_locs().Get(import_ir_inst.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};
-}
-
 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.
+    // TODO: Refactor to add an `InImport` pointing at the `Cpp` import, and
+    // eliminate `InCppImport`.
+    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};
+  }
+
   const auto& tree_and_subtrees =
-      tree_and_subtrees_getters_[absolute_node_id.check_ir_id.index]();
-  return tree_and_subtrees.NodeToDiagnosticLoc(absolute_node_id.node_id,
+      tree_and_subtrees_getters_[absolute_node_id.check_ir_id().index]();
+  return tree_and_subtrees.NodeToDiagnosticLoc(absolute_node_id.node_id(),
                                                token_only);
 }
 

+ 0 - 4
toolchain/check/diagnostic_emitter.h

@@ -51,10 +51,6 @@ class DiagnosticEmitter : public DiagnosticEmitterBase {
   auto ConvertLocImpl(SemIR::LocId loc_id, ContextFnT context_fn) const
       -> Diagnostics::ConvertedLoc;
 
-  // Returns `ConvertedLoc` if `loc` points to a `ClangDiagnostic` instruction.
-  auto TryConvertClangDiagnosticLoc(SemIR::LocId loc_id) const
-      -> std::optional<Diagnostics::ConvertedLoc>;
-
   // Converts a node_id corresponding to a specific sem_ir to a diagnostic
   // location.
   auto ConvertLocInFile(SemIR::AbsoluteNodeId absolute_node_id, bool token_only,

+ 4 - 3
toolchain/lower/file_context.cpp

@@ -715,12 +715,13 @@ auto FileContext::GetLocForDI(SemIR::InstId inst_id) -> LocForDI {
   SemIR::AbsoluteNodeId resolved =
       GetAbsoluteNodeId(sem_ir_, SemIR::LocId(inst_id)).back();
   const auto& tree_and_subtrees =
-      (*tree_and_subtrees_getters_for_debug_info_)[resolved.check_ir_id
+      (*tree_and_subtrees_getters_for_debug_info_)[resolved.check_ir_id()
                                                        .index]();
   const auto& tokens = tree_and_subtrees.tree().tokens();
 
-  if (resolved.node_id.has_value()) {
-    auto token = tree_and_subtrees.GetSubtreeTokenRange(resolved.node_id).begin;
+  if (resolved.node_id().has_value()) {
+    auto token =
+        tree_and_subtrees.GetSubtreeTokenRange(resolved.node_id()).begin;
     return {.filename = tokens.source().filename(),
             .line_number = tokens.GetLineNumber(token),
             .column_number = tokens.GetColumnNumber(token)};

+ 22 - 18
toolchain/sem_ir/absolute_node_id.cpp

@@ -8,14 +8,19 @@
 
 namespace Carbon::SemIR {
 
-// Notes an import on the diagnostic and updates cursors to point at the
-// imported IR.
+// Notes an import on the diagnostic. For `Cpp` imports, returns true. Otherwise
+// updates cursors to point at the imported IR and returns false.
 static auto FollowImportRef(
     llvm::SmallVector<AbsoluteNodeId>& absolute_node_ids,
     const File*& cursor_ir, InstId& cursor_inst_id,
-    ImportIRInstId import_ir_inst_id) -> void {
+    ImportIRInstId import_ir_inst_id) -> bool {
   auto import_ir_inst = cursor_ir->import_ir_insts().Get(import_ir_inst_id);
-  CARBON_CHECK(import_ir_inst.ir_id() != ImportIRId::Cpp);
+  if (import_ir_inst.ir_id() == ImportIRId::Cpp) {
+    absolute_node_ids.push_back(
+        AbsoluteNodeId(import_ir_inst.clang_source_loc_id()));
+    return true;
+  }
+
   const auto& import_ir = cursor_ir->import_irs().Get(import_ir_inst.ir_id());
   CARBON_CHECK(import_ir.decl_id.has_value(),
                "If we get `None` locations here, we may need to more "
@@ -37,9 +42,8 @@ static auto FollowImportRef(
           implicit_import_ir_inst.inst_id());
       CARBON_CHECK(implicit_loc_id.kind() == LocId::Kind::NodeId,
                    "Should only be one layer of implicit imports");
-      absolute_node_ids.push_back(
-          {.check_ir_id = implicit_ir.sem_ir->check_ir_id(),
-           .node_id = implicit_loc_id.node_id()});
+      absolute_node_ids.push_back(AbsoluteNodeId(
+          implicit_ir.sem_ir->check_ir_id(), implicit_loc_id.node_id()));
       break;
     }
 
@@ -48,14 +52,15 @@ static auto FollowImportRef(
 
     case LocId::Kind::NodeId: {
       // For imports in the current file, the location is simple.
-      absolute_node_ids.push_back({.check_ir_id = cursor_ir->check_ir_id(),
-                                   .node_id = import_loc_id.node_id()});
+      absolute_node_ids.push_back(
+          AbsoluteNodeId(cursor_ir->check_ir_id(), import_loc_id.node_id()));
       break;
     }
   }
 
   cursor_ir = import_ir.sem_ir;
   cursor_inst_id = import_ir_inst.inst_id();
+  return false;
 }
 
 // Returns true if this is the final parse node location. If the location is an
@@ -65,15 +70,14 @@ static auto HandleLocId(llvm::SmallVector<AbsoluteNodeId>& absolute_node_ids,
                         LocId loc_id) -> bool {
   switch (loc_id.kind()) {
     case LocId::Kind::ImportIRInstId: {
-      FollowImportRef(absolute_node_ids, cursor_ir, cursor_inst_id,
-                      loc_id.import_ir_inst_id());
-      return false;
+      return FollowImportRef(absolute_node_ids, cursor_ir, cursor_inst_id,
+                             loc_id.import_ir_inst_id());
     }
 
     case LocId::Kind::NodeId: {
       // Parse nodes always refer to the current IR.
-      absolute_node_ids.push_back({.check_ir_id = cursor_ir->check_ir_id(),
-                                   .node_id = loc_id.node_id()});
+      absolute_node_ids.push_back(
+          AbsoluteNodeId(cursor_ir->check_ir_id(), loc_id.node_id()));
       return true;
     }
 
@@ -115,8 +119,8 @@ static auto GetAbsoluteNodeIdImpl(
   }
 
   // `None` parse node but not an import; just nothing to point at.
-  absolute_node_ids.push_back({.check_ir_id = cursor_ir->check_ir_id(),
-                               .node_id = Parse::NodeId::None});
+  absolute_node_ids.push_back(
+      AbsoluteNodeId(cursor_ir->check_ir_id(), Parse::NodeId::None));
 }
 
 auto GetAbsoluteNodeId(const File* sem_ir, LocId loc_id)
@@ -124,8 +128,8 @@ auto GetAbsoluteNodeId(const File* sem_ir, LocId loc_id)
   llvm::SmallVector<AbsoluteNodeId> absolute_node_ids;
   switch (loc_id.kind()) {
     case LocId::Kind::None:
-      absolute_node_ids.push_back({.check_ir_id = sem_ir->check_ir_id(),
-                                   .node_id = Parse::NodeId::None});
+      absolute_node_ids.push_back(
+          AbsoluteNodeId(sem_ir->check_ir_id(), Parse::NodeId::None));
       break;
 
     case LocId::Kind::InstId:

+ 43 - 4
toolchain/sem_ir/absolute_node_id.h

@@ -11,10 +11,49 @@
 
 namespace Carbon::SemIR {
 
-// A specific node location in a file.
-struct AbsoluteNodeId {
-  CheckIRId check_ir_id;
-  Parse::NodeId node_id;
+// A specific node location in a file. Can refer to a Clang source location
+// within imported C++ code.
+class AbsoluteNodeId {
+ public:
+  // A speicifc node location in a file.
+  explicit AbsoluteNodeId(CheckIRId check_ir_id, Parse::NodeId node_id)
+      : check_ir_id_(check_ir_id), node_id_(node_id) {
+    CARBON_CHECK(check_ir_id != CheckIRId::Cpp);
+  }
+
+  // A Clang source location within imported C++ code.
+  explicit AbsoluteNodeId(ClangSourceLocId clang_source_loc_id)
+      : check_ir_id_(CheckIRId::Cpp),
+        clang_source_loc_id_(clang_source_loc_id) {}
+
+  // For a specific node location in a file, the ID of the IR.
+  // For Clang source location, this returns `Cpp`.
+  auto check_ir_id() const -> CheckIRId { return check_ir_id_; }
+
+  // The specific node location in a file. Must be called only if
+  // `check_ir_id()` doesn't return `Cpp`.
+  auto node_id() const -> Parse::NodeId {
+    CARBON_CHECK(check_ir_id() != CheckIRId::Cpp);
+    return node_id_;
+  }
+
+  // The Clang source location. Must be called only if `check_ir_id()` returns
+  // `Cpp`.
+  auto clang_source_loc_id() const -> ClangSourceLocId {
+    CARBON_CHECK(check_ir_id() == CheckIRId::Cpp);
+    return clang_source_loc_id_;
+  }
+
+ private:
+  // See `check_ir_id()`.
+  CheckIRId check_ir_id_;
+
+  union {
+    // See `node_id()`.
+    Parse::NodeId node_id_;
+    // See `clang_source_loc_id()`.
+    ClangSourceLocId clang_source_loc_id_;
+  };
 };
 
 // Resolves the `LocId` to a series of `NodeId`s, which may be in different

+ 6 - 0
toolchain/sem_ir/ids.h

@@ -272,9 +272,15 @@ struct FunctionId : public IdBase<FunctionId> {
 // check execution.
 struct CheckIRId : public IdBase<CheckIRId> {
   static constexpr llvm::StringLiteral Label = "check_ir";
+
+  // Used when referring to the imported C++.
+  static const CheckIRId Cpp;
+
   using IdBase::IdBase;
 };
 
+constexpr CheckIRId CheckIRId::Cpp = CheckIRId(NoneIndex - 1);
+
 // The ID of a class.
 struct ClassId : public IdBase<ClassId> {
   static constexpr llvm::StringLiteral Label = "class";