Skip to content

Improve indentation in type checker debugging output #62053

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 30, 2022
+193 −144
Merged

Improve indentation in type checker debugging output #62053

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3c401a1
Fix unbalanced braces in debugging output
abdulowork Nov 17, 2022
3f36694
Improve indentation in debugging output
abdulowork Nov 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions include/swift/Sema/ConstraintSystem.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1708,7 +1708,7 @@ class Solution {
SWIFT_DEBUG_DUMP;

/// Dump this solution.
void dump(raw_ostream &OS) const LLVM_ATTRIBUTE_USED;
void dump(raw_ostream &OS, unsigned indent) const LLVM_ATTRIBUTE_USED;
};

/// Describes the differences between several solutions to the same
Expand Down Expand Up @@ -6433,9 +6433,10 @@ class DisjunctionChoice {
bool isSymmetricOperator() const;
bool isUnaryOperator() const;

void print(llvm::raw_ostream &Out, SourceManager *SM, unsigned indent = 0) const {
void print(llvm::raw_ostream &Out, SourceManager *SM,
unsigned indent = 0) const {
Out << "disjunction choice ";
Choice->print(Out, SM);
Choice->print(Out, SM, indent);
}

operator Constraint *() { return Choice; }
Expand Down
8 changes: 5 additions & 3 deletions lib/Sema/BuilderTransform.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2370,9 +2370,10 @@ Optional<BraceStmt *> TypeChecker::applyResultBuilderBodyTransform(
}

if (cs.isDebugMode()) {
auto &log = llvm::errs();
auto indent = cs.solverState ? cs.solverState->getCurrentIndent() : 0;
auto &log = llvm::errs().indent(indent);
log << "--- Applying Solution ---\n";
solutions.front().dump(log);
solutions.front().dump(log, indent);
log << '\n';
}

Expand All @@ -2387,7 +2388,8 @@ Optional<BraceStmt *> TypeChecker::applyResultBuilderBodyTransform(
auto *body = result->getFunctionBody();

if (cs.isDebugMode()) {
auto &log = llvm::errs();
auto indent = cs.solverState ? cs.solverState->getCurrentIndent() : 0;
auto &log = llvm::errs().indent(indent);
log << "--- Type-checked function body ---\n";
body->dump(log);
log << '\n';
Expand Down
20 changes: 11 additions & 9 deletions lib/Sema/CSRanking.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ void ConstraintSystem::increaseScore(ScoreKind kind, unsigned value) {
if (isDebugMode() && value > 0) {
if (solverState)
llvm::errs().indent(solverState->getCurrentIndent());
llvm::errs() << "(increasing '" << Score::getNameFor(kind) << "' score by " << value
<< ")\n";
llvm::errs() << "(increasing '" << Score::getNameFor(kind) << "' score by "
<< value << ")\n";
}

unsigned index = static_cast<unsigned>(kind);
Expand All @@ -73,7 +73,7 @@ bool ConstraintSystem::worseThanBestSolution() const {

if (isDebugMode()) {
llvm::errs().indent(solverState->getCurrentIndent())
<< "(solution is worse than the best solution)\n";
<< "(solution is worse than the best solution)\n";
}

return true;
Expand Down Expand Up @@ -804,7 +804,7 @@ SolutionCompareResult ConstraintSystem::compareSolutions(
const SolutionDiff &diff, unsigned idx1, unsigned idx2) {
if (cs.isDebugMode()) {
llvm::errs().indent(cs.solverState->getCurrentIndent())
<< "comparing solutions " << idx1 << " and " << idx2 <<"\n";
<< "comparing solutions " << idx1 << " and " << idx2 << "\n";
}

// Whether the solutions are identical.
Expand Down Expand Up @@ -1360,13 +1360,15 @@ ConstraintSystem::findBestSolution(SmallVectorImpl<Solution> &viable,
return 0;

if (isDebugMode()) {
llvm::errs().indent(solverState->getCurrentIndent())
<< "Comparing " << viable.size() << " viable solutions\n";
auto indent = solverState->getCurrentIndent();
auto &log = llvm::errs();

log.indent(indent) << "Comparing " << viable.size()
<< " viable solutions\n";
for (unsigned i = 0, n = viable.size(); i != n; ++i) {
llvm::errs().indent(solverState->getCurrentIndent())
<< "\n--- Solution #" << i << " ---\n";
viable[i].dump(llvm::errs().indent(solverState->getCurrentIndent()));
log << "\n";
log.indent(indent) << "--- Solution #" << i << " ---\n";
viable[i].dump(llvm::errs(), indent);
}
}

Expand Down
82 changes: 53 additions & 29 deletions lib/Sema/CSSolver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,17 +353,21 @@ bool ConstraintSystem::simplify() {
auto *constraint = &ActiveConstraints.front();
deactivateConstraint(constraint);

auto isSimplifiable =
constraint->getKind() != ConstraintKind::Disjunction &&
constraint->getKind() != ConstraintKind::Conjunction;

if (isDebugMode()) {
auto &log = llvm::errs();
log.indent(solverState->getCurrentIndent());
log << "(considering -> ";
constraint->print(log, &getASTContext().SourceMgr);
constraint->print(log, &getASTContext().SourceMgr,
solverState->getCurrentIndent());
log << "\n";

// {Dis, Con}junction are returned unsolved in \c simplifyConstraint() and
// handled separately by solver steps.
if (constraint->getKind() != ConstraintKind::Disjunction &&
constraint->getKind() != ConstraintKind::Conjunction) {
if (isSimplifiable) {
log.indent(solverState->getCurrentIndent() + 2)
<< "(simplification result:\n";
}
Expand All @@ -375,7 +379,9 @@ bool ConstraintSystem::simplify() {
retireFailedConstraint(constraint);
if (isDebugMode()) {
auto &log = llvm::errs();
log.indent(solverState->getCurrentIndent() + 2) << ")\n";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would print an unbalanced brace if constrain was ConstraintKind::Disjunction || ConstraintKind::Conjunction

Screenshot 2022-11-11 at 17 43 54

if (isSimplifiable) {
log.indent(solverState->getCurrentIndent() + 2) << ")\n";
}
log.indent(solverState->getCurrentIndent() + 2) << "(outcome: error)\n";
}
break;
Expand All @@ -386,7 +392,9 @@ bool ConstraintSystem::simplify() {
retireConstraint(constraint);
if (isDebugMode()) {
auto &log = llvm::errs();
log.indent(solverState->getCurrentIndent() + 2) << ")\n";
if (isSimplifiable) {
log.indent(solverState->getCurrentIndent() + 2) << ")\n";
}
log.indent(solverState->getCurrentIndent() + 2)
<< "(outcome: simplified)\n";
}
Expand All @@ -397,7 +405,9 @@ bool ConstraintSystem::simplify() {
++solverState->NumUnsimplifiedConstraints;
if (isDebugMode()) {
auto &log = llvm::errs();
log.indent(solverState->getCurrentIndent() + 2) << ")\n";
if (isSimplifiable) {
log.indent(solverState->getCurrentIndent() + 2) << ")\n";
}
log.indent(solverState->getCurrentIndent() + 2)
<< "(outcome: unsolved)\n";
}
Expand Down Expand Up @@ -489,7 +499,8 @@ ConstraintSystem::SolverState::SolverState(
if (tyOpts.DebugConstraintSolverAttempt &&
tyOpts.DebugConstraintSolverAttempt == SolutionAttempt) {
CS.Options |= ConstraintSystemFlags::DebugConstraints;
llvm::errs() << "---Constraint system #" << SolutionAttempt << "---\n";
llvm::errs().indent(CS.solverState->getCurrentIndent())
<< "---Constraint system #" << SolutionAttempt << "---\n";
CS.print(llvm::errs());
}
}
Expand Down Expand Up @@ -793,7 +804,8 @@ bool ConstraintSystem::Candidate::solve(
auto &ctx = cs.getASTContext();
if (cs.isDebugMode()) {
auto &log = llvm::errs();
log << "--- Solving candidate for shrinking at ";
auto indent = cs.solverState ? cs.solverState->getCurrentIndent() : 0;
log.indent(indent) << "--- Solving candidate for shrinking at ";
auto R = E->getSourceRange();
if (R.isValid()) {
R.print(log, ctx.SourceMgr, /*PrintText=*/ false);
Expand All @@ -802,7 +814,7 @@ bool ConstraintSystem::Candidate::solve(
}
log << " ---\n";

E->dump(log);
E->dump(log, indent);
log << '\n';
cs.print(log);
}
Expand Down Expand Up @@ -830,14 +842,18 @@ bool ConstraintSystem::Candidate::solve(

if (cs.isDebugMode()) {
auto &log = llvm::errs();
auto indent = cs.solverState ? cs.solverState->getCurrentIndent() : 0;
if (solutions.empty()) {
log << "--- No Solutions ---\n";
log << "\n";
log.indent(indent) << "--- No Solutions ---\n";
} else {
log << "--- Solutions ---\n";
log << "\n";
log.indent(indent) << "--- Solutions ---\n";
for (unsigned i = 0, n = solutions.size(); i != n; ++i) {
auto &solution = solutions[i];
log << "\n--- Solution #" << i << " ---\n";
solution.dump(log);
log << "\n";
log.indent(indent) << "--- Solution #" << i << " ---\n";
solution.dump(log, indent);
}
}
}
Expand Down Expand Up @@ -1320,14 +1336,18 @@ Optional<std::vector<Solution>> ConstraintSystem::solve(
auto dumpSolutions = [&](const SolutionResult &result) {
// Debug-print the set of solutions.
if (isDebugMode()) {
auto &log = llvm::errs();
auto indent = solverState ? solverState->getCurrentIndent() : 0;
if (result.getKind() == SolutionResult::Success) {
llvm::errs() << "\n---Solution---\n";
result.getSolution().dump(llvm::errs());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solutions where printed unindented:

Screenshot 2022-11-11 at 17 02 34

log << "\n";
log.indent(indent) << "---Solution---\n";
result.getSolution().dump(llvm::errs(), indent);
} else if (result.getKind() == SolutionResult::Ambiguous) {
auto solutions = result.getAmbiguousSolutions();
for (unsigned i : indices(solutions)) {
llvm::errs() << "\n--- Solution #" << i << " ---\n";
solutions[i].dump(llvm::errs());
log << "\n";
log.indent(indent) << "--- Solution #" << i << " ---\n";
solutions[i].dump(llvm::errs(), indent);
}
}
}
Expand Down Expand Up @@ -1596,10 +1616,12 @@ void ConstraintSystem::solveForCodeCompletion(

if (isDebugMode()) {
auto &log = llvm::errs();
log << "--- Discovered " << solutions.size() << " solutions ---\n";
auto indent = solverState ? solverState->getCurrentIndent() : 0;
log.indent(indent) << "--- Discovered " << solutions.size()
<< " solutions ---\n";
for (const auto &solution : solutions) {
log << "--- Solution ---\n";
solution.dump(log);
log.indent(indent) << "--- Solution ---\n";
solution.dump(log, indent);
}
}

Expand All @@ -1621,7 +1643,8 @@ bool ConstraintSystem::solveForCodeCompletion(

if (isDebugMode()) {
auto &log = llvm::errs();
log << "--- Code Completion ---\n";
log.indent(solverState ? solverState->getCurrentIndent() : 0)
<< "--- Code Completion ---\n";
}

if (generateConstraints(target, FreeTypeVariableBinding::Disallow))
Expand Down Expand Up @@ -1665,10 +1688,10 @@ ConstraintSystem::filterDisjunction(
}

if (isDebugMode()) {
llvm::errs().indent(solverState ? solverState->getCurrentIndent() : 0)
<< "(disabled disjunction term ";
constraint->print(llvm::errs(), &ctx.SourceMgr);
llvm::errs() << ")\n";
auto indent = (solverState ? solverState->getCurrentIndent() : 0) + 4;
llvm::errs().indent(indent) << "(disabled disjunction term ";
constraint->print(llvm::errs(), &ctx.SourceMgr, indent);
llvm::errs().indent(indent) << ")\n";
}

if (restoreOnFail)
Expand Down Expand Up @@ -1724,10 +1747,11 @@ ConstraintSystem::filterDisjunction(
}

if (isDebugMode()) {
llvm::errs().indent(solverState ? solverState->getCurrentIndent(): 0)
<< "(introducing single enabled disjunction term ";
choice->print(llvm::errs(), &ctx.SourceMgr);
llvm::errs() << ")\n";
auto indent = (solverState ? solverState->getCurrentIndent() : 0) + 4;
llvm::errs().indent(indent)
<< "(introducing single enabled disjunction term ";
choice->print(llvm::errs(), &ctx.SourceMgr, indent);
llvm::errs().indent(indent) << ")\n";
}

simplifyDisjunctionChoice(choice);
Expand Down
36 changes: 20 additions & 16 deletions lib/Sema/CSStep.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,14 @@ void SplitterStep::computeFollowupSteps(

if (CS.isDebugMode()) {
auto &log = getDebugLogger();
auto indent = CS.solverState->getCurrentIndent();
// Verify that the constraint graph is valid.
CG.verify();

log << "---Constraint graph---\n";
log.indent(indent) << "---Constraint graph---\n";
CG.print(CS.getTypeVariables(), log);

log << "---Connected components---\n";
log.indent(indent) << "---Connected components---\n";
CG.printConnectedComponents(CS.getTypeVariables(), log);
}

Expand Down Expand Up @@ -361,12 +362,6 @@ StepResult ComponentStep::take(bool prevFailed) {
auto *disjunction = CS.selectDisjunction();

if (CS.isDebugMode()) {
if (!potentialBindings.empty()) {
auto &log = getDebugLogger();
log << "(Potential Binding(s): " << '\n';
log << potentialBindings;
}

SmallVector<Constraint *, 4> disjunctions;
CS.collectDisjunctions(disjunctions);
std::vector<std::string> overloadDisjunctions;
Expand All @@ -379,17 +374,24 @@ StepResult ComponentStep::take(bool prevFailed) {
overloadDisjunctions.push_back(
constraints[0]->getFirstType()->getString(PO));
}

if (!potentialBindings.empty() || !overloadDisjunctions.empty()) {
auto &log = getDebugLogger();
log << "(Potential Binding(s): " << '\n';
log << potentialBindings;
}

if (!overloadDisjunctions.empty()) {
auto &log = getDebugLogger();
log.indent(2);
log.indent(CS.solverState->getCurrentIndent() + 2);
log << "Disjunction(s) = [";
interleave(overloadDisjunctions, log, ", ");
log << "]\n";
}

if (!potentialBindings.empty() || !overloadDisjunctions.empty()) {
auto &log = getDebugLogger();
log << ")\n";
}
if (!potentialBindings.empty() || !overloadDisjunctions.empty()) {
auto &log = getDebugLogger();
log << ")\n";
}
}

Expand Down Expand Up @@ -428,7 +430,9 @@ StepResult ComponentStep::take(bool prevFailed) {

auto printConstraints = [&](const ConstraintList &constraints) {
for (auto &constraint : constraints)
constraint.print(getDebugLogger(), &CS.getASTContext().SourceMgr);
constraint.print(
getDebugLogger().indent(CS.solverState->getCurrentIndent()),
&CS.getASTContext().SourceMgr, CS.solverState->getCurrentIndent());
};

// If we don't have any disjunction or type variable choices left, we're done
Expand Down Expand Up @@ -670,8 +674,8 @@ bool DisjunctionStep::shouldSkip(const DisjunctionChoice &choice) const {
if (CS.isDebugMode()) {
auto &log = getDebugLogger();
log << "(skipping " + reason + " ";
choice.print(log, &ctx.SourceMgr);
log << '\n';
choice.print(log, &ctx.SourceMgr, CS.solverState->getCurrentIndent());
log << ")\n";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This brace was unbalanced

}

return true;
Expand Down
6 changes: 4 additions & 2 deletions lib/Sema/CSStep.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -707,7 +707,8 @@ class DisjunctionStep final : public BindingStep<DisjunctionChoiceProducer> {

void print(llvm::raw_ostream &Out) override {
Out << "DisjunctionStep for ";
Disjunction->print(Out, &CS.getASTContext().SourceMgr);
Disjunction->print(Out, &CS.getASTContext().SourceMgr,
CS.solverState->getCurrentIndent());
Out << '\n';
}

Expand Down Expand Up @@ -1013,7 +1014,8 @@ class ConjunctionStep : public BindingStep<ConjunctionElementProducer> {

void print(llvm::raw_ostream &Out) override {
Out << "ConjunctionStep for ";
Conjunction->print(Out, &CS.getASTContext().SourceMgr);
Conjunction->print(Out, &CS.getASTContext().SourceMgr,
CS.solverState->getCurrentIndent());
Out << '\n';
}

Expand Down
Loading