Просмотр исходного кода

Always dump summaries (single line output) on bulleted details lines (#7017)

The debugger dump format looks something like

```
id: summary
 - detail 1
 - detail 2
```

But if the detail is a full Dump of some other id, then the details
start to combine and get confusing. For instance if you Dump an
interface id as the detail, you get

```
id: summary
  - interface id: summary
  - complete: yes  <-- about the interface
  - detail 2  <-- not about the interface
```

This mixes the contents of multiple Dumps and is super confusing. So
introduce DumpFooSummary for everything that is dumped on a bulleted
details line, and always use the summary version in that situation.
Dana Jansens 1 месяц назад
Родитель
Сommit
562b423830
1 измененных файлов с 125 добавлено и 86 удалено
  1. 125 86
      toolchain/sem_ir/dump.cpp

+ 125 - 86
toolchain/sem_ir/dump.cpp

@@ -23,6 +23,48 @@ static auto DumpNameIfValid(const File& file, NameId name_id) -> std::string {
   return out.TakeStr();
 }
 
+static auto DumpLocSummary(const File& file, LocId loc_id) -> std::string {
+  RawStringOstream out;
+  // TODO: If the canonical location is None but the original is an InstId,
+  // should we dump the InstId anyway even though it has no location? Is that
+  // ever useful?
+  loc_id = file.insts().GetCanonicalLocId(loc_id);
+  switch (loc_id.kind()) {
+    case LocId::Kind::None: {
+      out << "LocId(<none>)";
+      break;
+    }
+
+    case LocId::Kind::ImportIRInstId: {
+      auto import_ir_id =
+          file.import_ir_insts().Get(loc_id.import_ir_inst_id()).ir_id();
+      const auto* import_file = file.import_irs().Get(import_ir_id).sem_ir;
+      out << "LocId(import from ";
+      if (import_file == nullptr) {
+        out << "unknown file";
+      } else {
+        out << "\"" << FormatEscaped(import_file->filename()) << "\"";
+      }
+      out << ")";
+      break;
+    }
+
+    case LocId::Kind::NodeId: {
+      auto token = file.parse_tree().node_token(loc_id.node_id());
+      auto line = file.parse_tree().tokens().GetLineNumber(token);
+      auto col = file.parse_tree().tokens().GetColumnNumber(token);
+      const char* implicit = loc_id.is_desugared() ? " implicit" : "";
+      out << "LocId(" << FormatEscaped(file.filename()) << ":" << line << ":"
+          << col << implicit << ")";
+      break;
+    }
+
+    case LocId::Kind::InstId:
+      CARBON_FATAL("unexpected LocId kind");
+  }
+  return out.TakeStr();
+}
+
 static auto DumpConstantSummary(const File& file, ConstantId const_id)
     -> std::string {
   RawStringOstream out;
@@ -40,15 +82,26 @@ static auto DumpConstantSummary(const File& file, ConstantId const_id)
   return out.TakeStr();
 }
 
-static auto DumpGenericSummary(const File& file, GenericId generic_id)
-    -> std::string {
+static auto DumpTypeSummary(const File& file, TypeId type_id) -> std::string {
   RawStringOstream out;
-  out << generic_id;
-  if (!generic_id.has_value()) {
+  out << type_id;
+  if (!type_id.has_value()) {
     return out.TakeStr();
   }
-  const auto& generic = file.generics().Get(generic_id);
-  out << ": " << generic << "\ndecl: " << Dump(file, generic.decl_id);
+
+  InstId inst_id = file.types().GetTypeInstId(type_id);
+  out << ": " << StringifyConstantInst(file, inst_id) << "; "
+      << file.insts().Get(inst_id);
+  auto const_id = file.types().GetConstantId(type_id);
+  if (const_id.is_symbolic()) {
+    if (file.constant_values().IsAttached(const_id)) {
+      out << " (attached symbolic)";
+    } else {
+      out << " (unattached symbolic)";
+    }
+  } else {
+    out << " (concrete)";
+  }
   return out.TakeStr();
 }
 
@@ -62,6 +115,42 @@ static auto DumpInstSummary(const File& file, InstId inst_id) -> std::string {
   return out.TakeStr();
 }
 
+static auto DumpGenericSummary(const File& file, GenericId generic_id)
+    -> std::string {
+  RawStringOstream out;
+  out << generic_id;
+  if (!generic_id.has_value()) {
+    return out.TakeStr();
+  }
+  const auto& generic = file.generics().Get(generic_id);
+  out << ": " << generic
+      << "\ndecl: " << DumpInstSummary(file, generic.decl_id);
+  return out.TakeStr();
+}
+
+static auto DumpInterfaceSummary(const File& file, InterfaceId interface_id)
+    -> std::string {
+  RawStringOstream out;
+  out << interface_id;
+  if (interface_id.has_value()) {
+    const auto& interface = file.interfaces().Get(interface_id);
+    out << ": " << interface << DumpNameIfValid(file, interface.name_id);
+  }
+  return out.TakeStr();
+}
+
+static auto DumpNamedConstraintSummary(const File& file,
+                                       NamedConstraintId named_constraint_id)
+    -> std::string {
+  RawStringOstream out;
+  out << named_constraint_id;
+  if (named_constraint_id.has_value()) {
+    const auto& constraint = file.named_constraints().Get(named_constraint_id);
+    out << ": " << constraint << DumpNameIfValid(file, constraint.name_id);
+  }
+  return out.TakeStr();
+}
+
 static auto DumpSpecificSummary(const File& file, SpecificId specific_id)
     -> std::string {
   RawStringOstream out;
@@ -72,6 +161,17 @@ static auto DumpSpecificSummary(const File& file, SpecificId specific_id)
   return out.TakeStr();
 }
 
+static auto DumpRequireImplsSummary(const File& file,
+                                    RequireImplsId require_impls_id)
+    -> std::string {
+  RawStringOstream out;
+  out << require_impls_id;
+  if (require_impls_id.has_value()) {
+    out << ": " << file.require_impls().Get(require_impls_id);
+  }
+  return out.TakeStr();
+}
+
 LLVM_DUMP_METHOD auto Dump(const File& file, ClassId class_id) -> std::string {
   RawStringOstream out;
   out << class_id;
@@ -92,10 +192,11 @@ LLVM_DUMP_METHOD auto Dump(const File& file, ConstantId const_id)
   if (const_id.is_symbolic()) {
     const auto& symbolic = file.constant_values().GetSymbolicConstant(const_id);
     out << ": " << symbolic << '\n'
-        << Dump(file, symbolic.inst_id) << '\n'
+        << DumpInstSummary(file, symbolic.inst_id) << '\n'
         << DumpGenericSummary(file, symbolic.generic_id);
   } else if (const_id.is_concrete()) {
-    out << ": " << Dump(file, file.constant_values().GetInstId(const_id));
+    out << ": "
+        << DumpInstSummary(file, file.constant_values().GetInstId(const_id));
   }
   return out.TakeStr();
 }
@@ -122,14 +223,14 @@ LLVM_DUMP_METHOD auto Dump(const File& file, FacetTypeId facet_type_id)
   const auto& facet_type = file.facet_types().Get(facet_type_id);
   out << ": " << facet_type;
   for (auto impls : facet_type.extend_constraints) {
-    out << "\n  - " << Dump(file, impls.interface_id);
+    out << "\n  - " << DumpInterfaceSummary(file, impls.interface_id);
     if (impls.specific_id.has_value()) {
       out << "; " << DumpSpecificSummary(file, impls.specific_id);
     }
     out << " (extend)";
   }
   for (auto impls : facet_type.self_impls_constraints) {
-    out << "\n  - " << Dump(file, impls.interface_id);
+    out << "\n  - " << DumpInterfaceSummary(file, impls.interface_id);
     if (impls.specific_id.has_value()) {
       out << "; " << DumpSpecificSummary(file, impls.specific_id);
     }
@@ -187,7 +288,7 @@ LLVM_DUMP_METHOD auto Dump(const File& file,
        llvm::enumerate(identified_facet_type.required_impls())) {
     auto [self, req_interface] = req_impl;
     // TODO: Dump the self too.
-    out << "\n  - " << Dump(file, req_interface.interface_id);
+    out << "\n  - " << DumpInterfaceSummary(file, req_interface.interface_id);
     if (req_interface.specific_id.has_value()) {
       out << "; " << DumpSpecificSummary(file, req_interface.specific_id);
     }
@@ -210,7 +311,8 @@ LLVM_DUMP_METHOD auto Dump(const File& file, ImplId impl_id) -> std::string {
   }
   const auto& impl = file.impls().Get(impl_id);
   out << ": " << impl << '\n'
-      << "  - interface_id: " << Dump(file, impl.interface.interface_id) << '\n'
+      << "  - interface_id: "
+      << DumpInterfaceSummary(file, impl.interface.interface_id) << '\n'
       << "  - specific_id: "
       << DumpSpecificSummary(file, impl.interface.specific_id);
   if (impl.interface.specific_id.has_value()) {
@@ -218,7 +320,7 @@ LLVM_DUMP_METHOD auto Dump(const File& file, ImplId impl_id) -> std::string {
         file.specifics().Get(impl.interface.specific_id).args_id;
     out << '\n' << Dump(file, inst_block_id);
   }
-  out << "\n  - witness loc: " << Dump(file, LocId(impl.witness_id));
+  out << "\n  - witness loc: " << DumpLocSummary(file, LocId(impl.witness_id));
   return out.TakeStr();
 }
 
@@ -253,7 +355,7 @@ LLVM_DUMP_METHOD auto Dump(const File& file, InstId inst_id) -> std::string {
   }
 
   if (inst.type_id().has_value()) {
-    out << "\n  - type: " << Dump(file, inst.type_id());
+    out << "\n  - type: " << DumpTypeSummary(file, inst.type_id());
   }
   ConstantId const_id = file.constant_values().GetAttached(inst_id);
   if (const_id.has_value()) {
@@ -265,62 +367,23 @@ LLVM_DUMP_METHOD auto Dump(const File& file, InstId inst_id) -> std::string {
       out << DumpConstantSummary(file, const_id);
     }
   }
-  out << "\n  - loc: " << Dump(file, LocId(inst_id));
+  out << "\n  - loc: " << DumpLocSummary(file, LocId(inst_id));
   return out.TakeStr();
 }
 
 LLVM_DUMP_METHOD auto Dump(const File& file, InterfaceId interface_id)
     -> std::string {
   RawStringOstream out;
-  out << interface_id;
+  out << DumpInterfaceSummary(file, interface_id);
   if (interface_id.has_value()) {
     const auto& interface = file.interfaces().Get(interface_id);
-    out << ": " << interface << DumpNameIfValid(file, interface.name_id);
     out << "\n  - complete: " << (interface.is_complete() ? "true" : "false");
   }
   return out.TakeStr();
 }
 
 LLVM_DUMP_METHOD auto Dump(const File& file, LocId loc_id) -> std::string {
-  RawStringOstream out;
-  // TODO: If the canonical location is None but the original is an InstId,
-  // should we dump the InstId anyway even though it has no location? Is that
-  // ever useful?
-  loc_id = file.insts().GetCanonicalLocId(loc_id);
-  switch (loc_id.kind()) {
-    case LocId::Kind::None: {
-      out << "LocId(<none>)";
-      break;
-    }
-
-    case LocId::Kind::ImportIRInstId: {
-      auto import_ir_id =
-          file.import_ir_insts().Get(loc_id.import_ir_inst_id()).ir_id();
-      const auto* import_file = file.import_irs().Get(import_ir_id).sem_ir;
-      out << "LocId(import from ";
-      if (import_file == nullptr) {
-        out << "unknown file";
-      } else {
-        out << "\"" << FormatEscaped(import_file->filename()) << "\"";
-      }
-      out << ")";
-      break;
-    }
-
-    case LocId::Kind::NodeId: {
-      auto token = file.parse_tree().node_token(loc_id.node_id());
-      auto line = file.parse_tree().tokens().GetLineNumber(token);
-      auto col = file.parse_tree().tokens().GetColumnNumber(token);
-      const char* implicit = loc_id.is_desugared() ? " implicit" : "";
-      out << "LocId(" << FormatEscaped(file.filename()) << ":" << line << ":"
-          << col << implicit << ")";
-      break;
-    }
-
-    case LocId::Kind::InstId:
-      CARBON_FATAL("unexpected LocId kind");
-  }
-  return out.TakeStr();
+  return DumpLocSummary(file, loc_id);
 }
 
 LLVM_DUMP_METHOD auto Dump(const File& file, NameId name_id) -> std::string {
@@ -372,10 +435,9 @@ LLVM_DUMP_METHOD auto Dump(const File& file,
                            NamedConstraintId named_constraint_id)
     -> std::string {
   RawStringOstream out;
-  out << named_constraint_id;
+  out << DumpNamedConstraintSummary(file, named_constraint_id);
   if (named_constraint_id.has_value()) {
     const auto& constraint = file.named_constraints().Get(named_constraint_id);
-    out << ": " << constraint << DumpNameIfValid(file, constraint.name_id);
     out << "\n  - complete: " << (constraint.is_complete() ? "true" : "false");
   }
   return out.TakeStr();
@@ -389,7 +451,7 @@ LLVM_DUMP_METHOD auto Dump(const File& file,
   if (require_impls_block_id.has_value()) {
     for (auto require_id :
          file.require_impls_blocks().Get(require_impls_block_id)) {
-      out << "\n  - " << Dump(file, require_id);
+      out << "\n  - " << DumpRequireImplsSummary(file, require_id);
     }
   }
   return out.TakeStr();
@@ -397,12 +459,7 @@ LLVM_DUMP_METHOD auto Dump(const File& file,
 
 LLVM_DUMP_METHOD auto Dump(const File& file, RequireImplsId require_impls_id)
     -> std::string {
-  RawStringOstream out;
-  out << require_impls_id;
-  if (require_impls_id.has_value()) {
-    out << ": " << file.require_impls().Get(require_impls_id);
-  }
-  return out.TakeStr();
+  return DumpRequireImplsSummary(file, require_impls_id);
 }
 
 static auto DumpEvalBlock(const File& file, llvm::StringRef region_name,
@@ -438,7 +495,8 @@ LLVM_DUMP_METHOD auto Dump(const File& file,
   RawStringOstream out;
   const auto& interface = file.specific_interfaces().Get(specific_interface_id);
   out << specific_interface_id << "\n"
-      << "  - interface: " << Dump(file, interface.interface_id) << "\n"
+      << "  - interface: " << DumpInterfaceSummary(file, interface.interface_id)
+      << "\n"
       << "  - specific_id: "
       << DumpSpecificSummary(file, interface.specific_id);
   return out.TakeStr();
@@ -463,26 +521,7 @@ LLVM_DUMP_METHOD auto Dump(const File& file,
 }
 
 LLVM_DUMP_METHOD auto Dump(const File& file, TypeId type_id) -> std::string {
-  RawStringOstream out;
-  out << type_id;
-  if (!type_id.has_value()) {
-    return out.TakeStr();
-  }
-
-  InstId inst_id = file.types().GetTypeInstId(type_id);
-  out << ": " << StringifyConstantInst(file, inst_id) << "; "
-      << file.insts().Get(inst_id);
-  auto const_id = file.types().GetConstantId(type_id);
-  if (const_id.is_symbolic()) {
-    if (file.constant_values().IsAttached(const_id)) {
-      out << " (attached symbolic)";
-    } else {
-      out << " (unattached symbolic)";
-    }
-  } else {
-    out << " (concrete)";
-  }
-  return out.TakeStr();
+  return DumpTypeSummary(file, type_id);
 }
 
 // Functions that can be used instead of the corresponding constructor, which is