Skip to content

Commit 138ecf1

Browse files
authored
Remove verbose formatting of instructions on crash messages. (#4495)
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
1 parent be56ff8 commit 138ecf1

File tree

9 files changed

+32
-77
lines changed

9 files changed

+32
-77
lines changed

toolchain/check/BUILD

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,6 @@ cc_library(
205205
"//common:vlog",
206206
"//toolchain/parse:node_kind",
207207
"//toolchain/parse:tree",
208-
"//toolchain/sem_ir:formatter",
209208
"//toolchain/sem_ir:ids",
210209
"@llvm-project//llvm:Support",
211210
],

toolchain/check/context.cpp

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1314,12 +1314,11 @@ auto Context::PrintForStackDump(llvm::raw_ostream& output) const -> void {
13141314
// spaces then add a couple to indent past the Context label.
13151315
constexpr int Indent = 10;
13161316

1317-
SemIR::Formatter formatter(*tokens_, *parse_tree_, *sem_ir_);
1318-
node_stack_.PrintForStackDump(formatter, Indent, output);
1319-
inst_block_stack_.PrintForStackDump(formatter, Indent, output);
1320-
pattern_block_stack_.PrintForStackDump(formatter, Indent, output);
1321-
param_and_arg_refs_stack_.PrintForStackDump(formatter, Indent, output);
1322-
args_type_info_stack_.PrintForStackDump(formatter, Indent, output);
1317+
node_stack_.PrintForStackDump(Indent, output);
1318+
inst_block_stack_.PrintForStackDump(Indent, output);
1319+
pattern_block_stack_.PrintForStackDump(Indent, output);
1320+
param_and_arg_refs_stack_.PrintForStackDump(Indent, output);
1321+
args_type_info_stack_.PrintForStackDump(Indent, output);
13231322
}
13241323

13251324
auto Context::DumpFormattedFile() const -> void {

toolchain/check/inst_block_stack.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,17 +61,19 @@ auto InstBlockStack::PopAndDiscard() -> void {
6161
CARBON_VLOG("{0} PopAndDiscard {1}\n", name_, id_stack_.size());
6262
}
6363

64-
auto InstBlockStack::PrintForStackDump(SemIR::Formatter& formatter, int indent,
64+
auto InstBlockStack::PrintForStackDump(int indent,
6565
llvm::raw_ostream& output) const
6666
-> void {
6767
output.indent(indent);
6868
output << name_ << ":\n";
6969
for (const auto& [i, id] : llvm::enumerate(id_stack_)) {
7070
output.indent(indent + 2);
71-
output << i << ". " << id;
72-
formatter.PrintPartialTrailingCodeBlock(insts_stack_.PeekArrayAt(i),
73-
indent + 4, output);
74-
output << "\n";
71+
output << i << ".\t" << id << "\t{";
72+
llvm::ListSeparator sep;
73+
for (auto id : insts_stack_.PeekArrayAt(i)) {
74+
output << sep << id;
75+
}
76+
output << "}\n";
7577
}
7678
}
7779

toolchain/check/inst_block_stack.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,7 @@ class InstBlockStack {
7070
}
7171

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

7675
// Runs verification that the processing cleanly finished.
7776
auto VerifyOnFinish() const -> void {

toolchain/check/node_stack.cpp

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,19 +8,15 @@
88

99
namespace Carbon::Check {
1010

11-
auto NodeStack::PrintForStackDump(SemIR::Formatter& formatter, int indent,
12-
llvm::raw_ostream& output) const -> void {
11+
auto NodeStack::PrintForStackDump(int indent, llvm::raw_ostream& output) const
12+
-> void {
1313
auto print_id = [&]<Id::Kind Kind>(Id id) {
1414
if constexpr (Kind == Id::Kind::None) {
15-
output << "no value\n";
15+
output << "no value";
1616
} else if constexpr (Kind == Id::Kind::Invalid) {
1717
CARBON_FATAL("Should not be in node stack");
18-
} else if constexpr (Kind == Id::KindFor<SemIR::InstId>()) {
19-
output << "\n";
20-
formatter.PrintInst(id.As<Id::KindFor<SemIR::InstId>()>(), indent + 4,
21-
output);
2218
} else {
23-
output << id.As<Kind>() << "\n";
19+
output << id.As<Kind>();
2420
}
2521
};
2622

@@ -37,6 +33,7 @@ auto NodeStack::PrintForStackDump(SemIR::Formatter& formatter, int indent,
3733
break;
3834
#include "toolchain/parse/node_kind.def"
3935
}
36+
output << "\n";
4037
}
4138
}
4239

toolchain/check/node_stack.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
#include "toolchain/parse/node_kind.h"
1212
#include "toolchain/parse/tree.h"
1313
#include "toolchain/parse/typed_nodes.h"
14-
#include "toolchain/sem_ir/formatter.h"
1514
#include "toolchain/sem_ir/id_kind.h"
1615
#include "toolchain/sem_ir/ids.h"
1716

@@ -335,8 +334,7 @@ class NodeStack {
335334
}
336335

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

341339
auto empty() const -> bool { return stack_.empty(); }
342340
auto size() const -> size_t { return stack_.size(); }

toolchain/check/param_and_arg_refs_stack.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,8 @@ class ParamAndArgRefsStack {
7272
auto VerifyOnFinish() -> void { stack_.VerifyOnFinish(); }
7373

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

8079
private:

toolchain/sem_ir/formatter.cpp

Lines changed: 11 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -78,34 +78,6 @@ class FormatterImpl {
7878
out_ << "\n";
7979
}
8080

81-
// Prints a code block.
82-
auto FormatPartialTrailingCodeBlock(llvm::ArrayRef<SemIR::InstId> block)
83-
-> void {
84-
out_ << ' ';
85-
OpenBrace();
86-
constexpr int NumPrintedOnSkip = 9;
87-
// Avoid only skipping one item.
88-
if (block.size() > NumPrintedOnSkip + 1) {
89-
Indent();
90-
out_ << "... skipping " << (block.size() - NumPrintedOnSkip)
91-
<< " insts ...\n";
92-
block = block.take_back(NumPrintedOnSkip);
93-
}
94-
FormatCodeBlock(block);
95-
CloseBrace();
96-
}
97-
98-
// Prints a single instruction.
99-
auto FormatInst(InstId inst_id) -> void {
100-
if (!inst_id.is_valid()) {
101-
Indent();
102-
out_ << "invalid\n";
103-
return;
104-
}
105-
106-
FormatInst(inst_id, sem_ir_.insts().Get(inst_id));
107-
}
108-
10981
private:
11082
enum class AddSpace : bool { Before, After };
11183

@@ -560,6 +532,17 @@ class FormatterImpl {
560532
}
561533
}
562534

535+
// Prints a single instruction.
536+
auto FormatInst(InstId inst_id) -> void {
537+
if (!inst_id.is_valid()) {
538+
Indent();
539+
out_ << "invalid\n";
540+
return;
541+
}
542+
543+
FormatInst(inst_id, sem_ir_.insts().Get(inst_id));
544+
}
545+
563546
auto FormatInst(InstId inst_id, Inst inst) -> void {
564547
CARBON_KIND_SWITCH(inst) {
565548
#define CARBON_SEM_IR_INST_KIND(InstT) \
@@ -1180,17 +1163,4 @@ auto Formatter::Print(llvm::raw_ostream& out) -> void {
11801163
formatter.Format();
11811164
}
11821165

1183-
auto Formatter::PrintPartialTrailingCodeBlock(
1184-
llvm::ArrayRef<SemIR::InstId> block, int indent, llvm::raw_ostream& out)
1185-
-> void {
1186-
FormatterImpl formatter(sem_ir_, &inst_namer_, out, indent);
1187-
formatter.FormatPartialTrailingCodeBlock(block);
1188-
}
1189-
1190-
auto Formatter::PrintInst(SemIR::InstId inst_id, int indent,
1191-
llvm::raw_ostream& out) -> void {
1192-
FormatterImpl formatter(sem_ir_, &inst_namer_, out, indent);
1193-
formatter.FormatInst(inst_id);
1194-
}
1195-
11961166
} // namespace Carbon::SemIR

toolchain/sem_ir/formatter.h

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,6 @@ class Formatter {
2222

2323
// Prints the full IR.
2424
auto Print(llvm::raw_ostream& out) -> void;
25-
// Prints a single code block. Only prints the last several instructions of
26-
// large blocks.
27-
auto PrintPartialTrailingCodeBlock(llvm::ArrayRef<SemIR::InstId> block,
28-
int indent, llvm::raw_ostream& out)
29-
-> void;
30-
// Prints a single instruction.
31-
auto PrintInst(SemIR::InstId inst_id, int indent, llvm::raw_ostream& out)
32-
-> void;
3325

3426
private:
3527
const File& sem_ir_;

0 commit comments

Comments
 (0)