Skip to content

Commit

Permalink
Remove verbose formatting of instructions on crash messages. (#4495)
Browse files Browse the repository at this point in the history
Undoes a chunk of #4125 because nobody's really in favor of keeping the
formatting, and it's occasionally caused a crash in Formatter to
dominate output (and even when working, it can be verbose; the source
location in (5) is often more helpful).

Basically goes back to:

```
4.	Check::Context
          NodeStack:
            0. LetIntroducer: no value
            1. BindingPattern: inst+15
            2. LetInitializer: no value
            3. StructLiteralStart: no value
          inst_block_stack_:
            0.	block<invalid>	{inst+0, inst+1, inst+6, inst+7, inst+8, inst+9, inst+10, inst+11, inst+12}
            1.	global_init	{}
          pattern_block_stack_:
            0.	block<invalid>	{}
          param_and_arg_refs_stack:
            0.	block<invalid>	{}
          args_type_info_stack_:
            0.	block<invalid>	{}
5.	alias_of_alias.carbon:15:12: checking StructLiteral
          let d: c = {};
                     ^~
```

Fixes #4145
  • Loading branch information
jonmeow authored Nov 7, 2024
1 parent be56ff8 commit 138ecf1
Show file tree
Hide file tree
Showing 9 changed files with 32 additions and 77 deletions.
1 change: 0 additions & 1 deletion toolchain/check/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,6 @@ cc_library(
"//common:vlog",
"//toolchain/parse:node_kind",
"//toolchain/parse:tree",
"//toolchain/sem_ir:formatter",
"//toolchain/sem_ir:ids",
"@llvm-project//llvm:Support",
],
Expand Down
11 changes: 5 additions & 6 deletions toolchain/check/context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1314,12 +1314,11 @@ auto Context::PrintForStackDump(llvm::raw_ostream& output) const -> void {
// spaces then add a couple to indent past the Context label.
constexpr int Indent = 10;

SemIR::Formatter formatter(*tokens_, *parse_tree_, *sem_ir_);
node_stack_.PrintForStackDump(formatter, Indent, output);
inst_block_stack_.PrintForStackDump(formatter, Indent, output);
pattern_block_stack_.PrintForStackDump(formatter, Indent, output);
param_and_arg_refs_stack_.PrintForStackDump(formatter, Indent, output);
args_type_info_stack_.PrintForStackDump(formatter, Indent, output);
node_stack_.PrintForStackDump(Indent, output);
inst_block_stack_.PrintForStackDump(Indent, output);
pattern_block_stack_.PrintForStackDump(Indent, output);
param_and_arg_refs_stack_.PrintForStackDump(Indent, output);
args_type_info_stack_.PrintForStackDump(Indent, output);
}

auto Context::DumpFormattedFile() const -> void {
Expand Down
12 changes: 7 additions & 5 deletions toolchain/check/inst_block_stack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,17 +61,19 @@ auto InstBlockStack::PopAndDiscard() -> void {
CARBON_VLOG("{0} PopAndDiscard {1}\n", name_, id_stack_.size());
}

auto InstBlockStack::PrintForStackDump(SemIR::Formatter& formatter, int indent,
auto InstBlockStack::PrintForStackDump(int indent,
llvm::raw_ostream& output) const
-> void {
output.indent(indent);
output << name_ << ":\n";
for (const auto& [i, id] : llvm::enumerate(id_stack_)) {
output.indent(indent + 2);
output << i << ". " << id;
formatter.PrintPartialTrailingCodeBlock(insts_stack_.PeekArrayAt(i),
indent + 4, output);
output << "\n";
output << i << ".\t" << id << "\t{";
llvm::ListSeparator sep;
for (auto id : insts_stack_.PeekArrayAt(i)) {
output << sep << id;
}
output << "}\n";
}
}

Expand Down
3 changes: 1 addition & 2 deletions toolchain/check/inst_block_stack.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,7 @@ class InstBlockStack {
}

// Prints the stack for a stack dump.
auto PrintForStackDump(SemIR::Formatter& formatter, int indent,
llvm::raw_ostream& output) const -> void;
auto PrintForStackDump(int indent, llvm::raw_ostream& output) const -> void;

// Runs verification that the processing cleanly finished.
auto VerifyOnFinish() const -> void {
Expand Down
13 changes: 5 additions & 8 deletions toolchain/check/node_stack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,15 @@

namespace Carbon::Check {

auto NodeStack::PrintForStackDump(SemIR::Formatter& formatter, int indent,
llvm::raw_ostream& output) const -> void {
auto NodeStack::PrintForStackDump(int indent, llvm::raw_ostream& output) const
-> void {
auto print_id = [&]<Id::Kind Kind>(Id id) {
if constexpr (Kind == Id::Kind::None) {
output << "no value\n";
output << "no value";
} else if constexpr (Kind == Id::Kind::Invalid) {
CARBON_FATAL("Should not be in node stack");
} else if constexpr (Kind == Id::KindFor<SemIR::InstId>()) {
output << "\n";
formatter.PrintInst(id.As<Id::KindFor<SemIR::InstId>()>(), indent + 4,
output);
} else {
output << id.As<Kind>() << "\n";
output << id.As<Kind>();
}
};

Expand All @@ -37,6 +33,7 @@ auto NodeStack::PrintForStackDump(SemIR::Formatter& formatter, int indent,
break;
#include "toolchain/parse/node_kind.def"
}
output << "\n";
}
}

Expand Down
4 changes: 1 addition & 3 deletions toolchain/check/node_stack.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#include "toolchain/parse/node_kind.h"
#include "toolchain/parse/tree.h"
#include "toolchain/parse/typed_nodes.h"
#include "toolchain/sem_ir/formatter.h"
#include "toolchain/sem_ir/id_kind.h"
#include "toolchain/sem_ir/ids.h"

Expand Down Expand Up @@ -335,8 +334,7 @@ class NodeStack {
}

// Prints the stack for a stack dump.
auto PrintForStackDump(SemIR::Formatter& formatter, int indent,
llvm::raw_ostream& output) const -> void;
auto PrintForStackDump(int indent, llvm::raw_ostream& output) const -> void;

auto empty() const -> bool { return stack_.empty(); }
auto size() const -> size_t { return stack_.size(); }
Expand Down
5 changes: 2 additions & 3 deletions toolchain/check/param_and_arg_refs_stack.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,8 @@ class ParamAndArgRefsStack {
auto VerifyOnFinish() -> void { stack_.VerifyOnFinish(); }

// Prints the stack for a stack dump.
auto PrintForStackDump(SemIR::Formatter& formatter, int indent,
llvm::raw_ostream& output) const -> void {
return stack_.PrintForStackDump(formatter, indent, output);
auto PrintForStackDump(int indent, llvm::raw_ostream& output) const -> void {
return stack_.PrintForStackDump(indent, output);
}

private:
Expand Down
52 changes: 11 additions & 41 deletions toolchain/sem_ir/formatter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,34 +78,6 @@ class FormatterImpl {
out_ << "\n";
}

// Prints a code block.
auto FormatPartialTrailingCodeBlock(llvm::ArrayRef<SemIR::InstId> block)
-> void {
out_ << ' ';
OpenBrace();
constexpr int NumPrintedOnSkip = 9;
// Avoid only skipping one item.
if (block.size() > NumPrintedOnSkip + 1) {
Indent();
out_ << "... skipping " << (block.size() - NumPrintedOnSkip)
<< " insts ...\n";
block = block.take_back(NumPrintedOnSkip);
}
FormatCodeBlock(block);
CloseBrace();
}

// Prints a single instruction.
auto FormatInst(InstId inst_id) -> void {
if (!inst_id.is_valid()) {
Indent();
out_ << "invalid\n";
return;
}

FormatInst(inst_id, sem_ir_.insts().Get(inst_id));
}

private:
enum class AddSpace : bool { Before, After };

Expand Down Expand Up @@ -560,6 +532,17 @@ class FormatterImpl {
}
}

// Prints a single instruction.
auto FormatInst(InstId inst_id) -> void {
if (!inst_id.is_valid()) {
Indent();
out_ << "invalid\n";
return;
}

FormatInst(inst_id, sem_ir_.insts().Get(inst_id));
}

auto FormatInst(InstId inst_id, Inst inst) -> void {
CARBON_KIND_SWITCH(inst) {
#define CARBON_SEM_IR_INST_KIND(InstT) \
Expand Down Expand Up @@ -1180,17 +1163,4 @@ auto Formatter::Print(llvm::raw_ostream& out) -> void {
formatter.Format();
}

auto Formatter::PrintPartialTrailingCodeBlock(
llvm::ArrayRef<SemIR::InstId> block, int indent, llvm::raw_ostream& out)
-> void {
FormatterImpl formatter(sem_ir_, &inst_namer_, out, indent);
formatter.FormatPartialTrailingCodeBlock(block);
}

auto Formatter::PrintInst(SemIR::InstId inst_id, int indent,
llvm::raw_ostream& out) -> void {
FormatterImpl formatter(sem_ir_, &inst_namer_, out, indent);
formatter.FormatInst(inst_id);
}

} // namespace Carbon::SemIR
8 changes: 0 additions & 8 deletions toolchain/sem_ir/formatter.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,6 @@ class Formatter {

// Prints the full IR.
auto Print(llvm::raw_ostream& out) -> void;
// Prints a single code block. Only prints the last several instructions of
// large blocks.
auto PrintPartialTrailingCodeBlock(llvm::ArrayRef<SemIR::InstId> block,
int indent, llvm::raw_ostream& out)
-> void;
// Prints a single instruction.
auto PrintInst(SemIR::InstId inst_id, int indent, llvm::raw_ostream& out)
-> void;

private:
const File& sem_ir_;
Expand Down

0 comments on commit 138ecf1

Please sign in to comment.