Skip to content

Commit d890b8a

Browse files
CodaFiCodaFi
committed
Remove some save-and-restores
An awful pattern we use throughout the compiler is to save and restore global flags just for little things. In this case, it was just to turn on some extra options in AST printing for type variables. The kicker is that the ASTDumper doesn't even respect this flag. Add this as a PrintOption and remove the offending save-and-restores. This doesn't quite get them all: we appear to have productized this pattern in the REPL.
1 parent f4d333d commit d890b8a

13 files changed

+55
-57
lines changed

‎include/swift/AST/PrintOptions.h

+4
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,10 @@ struct PrintOptions {
270270
/// Whether to skip keywords with a prefix of underscore such as __consuming.
271271
bool SkipUnderscoredKeywords = false;
272272

273+
/// Prints type variables and unresolved types in an expanded notation suitable
274+
/// for debugging.
275+
bool PrintTypesForDebugging = false;
276+
273277
/// How to print opaque return types.
274278
enum class OpaqueReturnTypePrintingMode {
275279
/// 'some P1 & P2'.

‎lib/AST/ASTDumper.cpp

+5-23
Original file line numberDiff line numberDiff line change
@@ -1262,15 +1262,6 @@ void ParameterList::dump() const {
12621262
}
12631263

12641264
void ParameterList::dump(raw_ostream &OS, unsigned Indent) const {
1265-
llvm::Optional<llvm::SaveAndRestore<bool>> X;
1266-
1267-
// Make sure to print type variables if we can get to ASTContext.
1268-
if (size() != 0 && get(0)) {
1269-
auto &ctx = get(0)->getASTContext();
1270-
X.emplace(llvm::SaveAndRestore<bool>(ctx.LangOpts.DebugConstraintSolver,
1271-
true));
1272-
}
1273-
12741265
PrintDecl(OS, Indent).printParameterList(this);
12751266
llvm::errs() << '\n';
12761267
}
@@ -1293,9 +1284,6 @@ void Decl::dump(const char *filename) const {
12931284
}
12941285

12951286
void Decl::dump(raw_ostream &OS, unsigned Indent) const {
1296-
// Make sure to print type variables.
1297-
llvm::SaveAndRestore<bool> X(getASTContext().LangOpts.DebugConstraintSolver,
1298-
true);
12991287
PrintDecl(OS, Indent).visit(const_cast<Decl *>(this));
13001288
OS << '\n';
13011289
}
@@ -1407,8 +1395,6 @@ void SourceFile::dump() const {
14071395
}
14081396

14091397
void SourceFile::dump(llvm::raw_ostream &OS) const {
1410-
llvm::SaveAndRestore<bool> X(getASTContext().LangOpts.DebugConstraintSolver,
1411-
true);
14121398
PrintDecl(OS).visitSourceFile(*this);
14131399
llvm::errs() << '\n';
14141400
}
@@ -1826,13 +1812,17 @@ class PrintExpr : public ExprVisitor<PrintExpr> {
18261812
}
18271813

18281814
raw_ostream &printCommon(Expr *E, const char *C) {
1815+
PrintOptions PO;
1816+
PO.PrintTypesForDebugging = true;
1817+
18291818
OS.indent(Indent);
18301819
PrintWithColorRAII(OS, ParenthesisColor) << '(';
18311820
PrintWithColorRAII(OS, ExprColor) << C;
18321821

18331822
if (E->isImplicit())
18341823
PrintWithColorRAII(OS, ExprModifierColor) << " implicit";
1835-
PrintWithColorRAII(OS, TypeColor) << " type='" << GetTypeOfExpr(E) << '\'';
1824+
PrintWithColorRAII(OS, TypeColor) << " type='";
1825+
PrintWithColorRAII(OS, TypeColor) << GetTypeOfExpr(E).getString(PO) << '\'';
18361826

18371827
// If we have a source range and an ASTContext, print the source range.
18381828
if (auto Ty = GetTypeOfExpr(E)) {
@@ -3749,14 +3739,10 @@ namespace {
37493739
} // end anonymous namespace
37503740

37513741
void Type::dump() const {
3752-
// Make sure to print type variables.
37533742
dump(llvm::errs());
37543743
}
37553744

37563745
void Type::dump(raw_ostream &os, unsigned indent) const {
3757-
// Make sure to print type variables.
3758-
llvm::SaveAndRestore<bool> X(getPointer()->getASTContext().LangOpts.
3759-
DebugConstraintSolver, true);
37603746
PrintType(os, indent).visit(*this, "");
37613747
os << "\n";
37623748
}
@@ -3767,10 +3753,6 @@ void TypeBase::dump() const {
37673753
}
37683754

37693755
void TypeBase::dump(raw_ostream &os, unsigned indent) const {
3770-
auto &ctx = const_cast<TypeBase*>(this)->getASTContext();
3771-
3772-
// Make sure to print type variables.
3773-
llvm::SaveAndRestore<bool> X(ctx.LangOpts.DebugConstraintSolver, true);
37743756
Type(const_cast<TypeBase *>(this)).dump(os, indent);
37753757
}
37763758

‎lib/AST/ASTPrinter.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -3595,7 +3595,7 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
35953595
}
35963596

35973597
void visitUnresolvedType(UnresolvedType *T) {
3598-
if (T->getASTContext().LangOpts.DebugConstraintSolver)
3598+
if (Options.PrintTypesForDebugging)
35993599
Printer << "<<unresolvedtype>>";
36003600
else
36013601
Printer << "_";
@@ -4331,7 +4331,7 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
43314331
#include "swift/AST/ReferenceStorage.def"
43324332

43334333
void visitTypeVariableType(TypeVariableType *T) {
4334-
if (T->getASTContext().LangOpts.DebugConstraintSolver) {
4334+
if (Options.PrintTypesForDebugging) {
43354335
Printer << "$T" << T->getID();
43364336
return;
43374337
}

‎lib/Sema/CSStep.cpp

+4-2
Original file line numberDiff line numberDiff line change
@@ -415,13 +415,15 @@ StepResult ComponentStep::finalize(bool isSuccess) {
415415
void TypeVariableStep::setup() {
416416
++CS.solverState->NumTypeVariablesBound;
417417
if (isDebugMode()) {
418+
PrintOptions PO;
419+
PO.PrintTypesForDebugging = true;
418420
auto &log = getDebugLogger();
419421

420422
log << "Initial bindings: ";
421423
interleave(InitialBindings.begin(), InitialBindings.end(),
422424
[&](const Binding &binding) {
423-
log << TypeVar->getString()
424-
<< " := " << binding.BindingType->getString();
425+
log << TypeVar->getString(PO)
426+
<< " := " << binding.BindingType->getString(PO);
425427
},
426428
[&log] { log << ", "; });
427429

‎lib/Sema/CSStep.h

+3-1
Original file line numberDiff line numberDiff line change
@@ -601,7 +601,9 @@ class TypeVariableStep final : public BindingStep<TypeVarBindingProducer> {
601601
StepResult resume(bool prevFailed) override;
602602

603603
void print(llvm::raw_ostream &Out) override {
604-
Out << "TypeVariableStep for " << TypeVar->getString() << " with #"
604+
PrintOptions PO;
605+
PO.PrintTypesForDebugging = true;
606+
Out << "TypeVariableStep for " << TypeVar->getString(PO) << " with #"
605607
<< InitialBindings.size() << " initial bindings\n";
606608
}
607609

‎lib/Sema/Constraint.cpp

+10-10
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
#include "swift/Basic/Compiler.h"
2222
#include "llvm/Support/Compiler.h"
2323
#include "llvm/Support/raw_ostream.h"
24-
#include "llvm/Support/SaveAndRestore.h"
2524
#include <algorithm>
2625

2726
using namespace swift;
@@ -265,6 +264,10 @@ Constraint *Constraint::clone(ConstraintSystem &cs) const {
265264
}
266265

267266
void Constraint::print(llvm::raw_ostream &Out, SourceManager *sm) const {
267+
// Print all type variables as $T0 instead of _ here.
268+
PrintOptions PO;
269+
PO.PrintTypesForDebugging = true;
270+
268271
if (Kind == ConstraintKind::Disjunction) {
269272
Out << "disjunction";
270273
if (shouldRememberChoice())
@@ -286,7 +289,7 @@ void Constraint::print(llvm::raw_ostream &Out, SourceManager *sm) const {
286289
return;
287290
}
288291

289-
getFirstType()->print(Out);
292+
Out << getFirstType()->getString(PO);
290293

291294
bool skipSecond = false;
292295

@@ -315,17 +318,17 @@ void Constraint::print(llvm::raw_ostream &Out, SourceManager *sm) const {
315318
case ConstraintKind::OneWayEqual: Out << " one-way bind to "; break;
316319
case ConstraintKind::KeyPath:
317320
Out << " key path from ";
318-
getSecondType()->print(Out);
321+
Out << getSecondType()->getString(PO);
319322
Out << " -> ";
320-
getThirdType()->print(Out);
323+
Out << getThirdType()->getString(PO);
321324
skipSecond = true;
322325
break;
323326

324327
case ConstraintKind::KeyPathApplication:
325328
Out << " key path projecting ";
326-
getSecondType()->print(Out);
329+
Out << getSecondType()->getString(PO);
327330
Out << " -> ";
328-
getThirdType()->print(Out);
331+
Out << getThirdType()->getString(PO);
329332
skipSecond = true;
330333
break;
331334
case ConstraintKind::OptionalObject:
@@ -396,7 +399,7 @@ void Constraint::print(llvm::raw_ostream &Out, SourceManager *sm) const {
396399
}
397400

398401
if (!skipSecond)
399-
getSecondType()->print(Out);
402+
Out << getSecondType()->getString(PO);
400403

401404
if (auto restriction = getRestriction()) {
402405
Out << ' ' << getName(*restriction);
@@ -420,9 +423,6 @@ void Constraint::dump(SourceManager *sm) const {
420423
}
421424

422425
void Constraint::dump(ConstraintSystem *CS) const {
423-
// Print all type variables as $T0 instead of _ here.
424-
llvm::SaveAndRestore<bool> X(CS->getASTContext().LangOpts.
425-
DebugConstraintSolver, true);
426426
// Disable MSVC warning: only for use within the debugger.
427427
#if SWIFT_COMPILER_IS_MSVC
428428
#pragma warning(push)

‎lib/Sema/ConstraintGraph.cpp

+3-2
Original file line numberDiff line numberDiff line change
@@ -1319,9 +1319,10 @@ void ConstraintGraph::incrementConstraintsPerContractionCounter() {
13191319

13201320
#pragma mark Debugging output
13211321

1322-
void ConstraintGraphNode::print(llvm::raw_ostream &out, unsigned indent) const {
1322+
void ConstraintGraphNode::print(llvm::raw_ostream &out, unsigned indent,
1323+
PrintOptions PO) const {
13231324
out.indent(indent);
1324-
TypeVar->print(out);
1325+
Type(TypeVar).print(out, PO);
13251326
out << ":\n";
13261327

13271328
// Print constraints.

‎lib/Sema/ConstraintGraph.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,8 @@ class ConstraintGraphNode {
160160
mutable SmallVector<TypeVariableType *, 2> EquivalenceClass;
161161

162162
/// Print this graph node.
163-
void print(llvm::raw_ostream &out, unsigned indent) const;
163+
void print(llvm::raw_ostream &out, unsigned indent,
164+
PrintOptions PO = PrintOptions()) const;
164165

165166
SWIFT_DEBUG_DUMP;
166167

‎lib/Sema/ConstraintLocator.cpp

+4-1
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,9 @@ void ConstraintLocator::dump(ConstraintSystem *CS) const {
261261

262262

263263
void ConstraintLocator::dump(SourceManager *sm, raw_ostream &out) const {
264+
PrintOptions PO;
265+
PO.PrintTypesForDebugging = true;
266+
264267
out << "locator@" << (void*) this << " [";
265268

266269
if (anchor) {
@@ -295,7 +298,7 @@ void ConstraintLocator::dump(SourceManager *sm, raw_ostream &out) const {
295298
switch (elt.getKind()) {
296299
case GenericParameter: {
297300
auto gpElt = elt.castTo<LocatorPathElt::GenericParameter>();
298-
out << "generic parameter '" << gpElt.getType()->getString() << "'";
301+
out << "generic parameter '" << gpElt.getType()->getString(PO) << "'";
299302
break;
300303
}
301304
case ApplyArgument:

‎lib/Sema/ConstraintSystem.cpp

+4-2
Original file line numberDiff line numberDiff line change
@@ -2242,11 +2242,13 @@ void ConstraintSystem::resolveOverload(ConstraintLocator *locator,
22422242
}
22432243

22442244
if (getASTContext().TypeCheckerOpts.DebugConstraintSolver) {
2245+
PrintOptions PO;
2246+
PO.PrintTypesForDebugging = true;
22452247
auto &log = getASTContext().TypeCheckerDebug->getStream();
22462248
log.indent(solverState ? solverState->depth * 2 : 2)
22472249
<< "(overload set choice binding "
2248-
<< boundType->getString() << " := "
2249-
<< refType->getString() << ")\n";
2250+
<< boundType->getString(PO) << " := "
2251+
<< refType->getString(PO) << ")\n";
22502252
}
22512253

22522254
// If this overload is disfavored, note that.

‎lib/Sema/ConstraintSystem.h

+7-6
Original file line numberDiff line numberDiff line change
@@ -3432,13 +3432,12 @@ class ConstraintSystem {
34323432
if (NumDefaultableBindings > 0)
34333433
out << "#defaultable_bindings=" << NumDefaultableBindings << " ";
34343434

3435+
PrintOptions PO;
3436+
PO.PrintTypesForDebugging = true;
34353437
out << "bindings={";
34363438
interleave(Bindings,
34373439
[&](const PotentialBinding &binding) {
34383440
auto type = binding.BindingType;
3439-
auto &ctx = type->getASTContext();
3440-
llvm::SaveAndRestore<bool> debugConstraints(
3441-
ctx.LangOpts.DebugConstraintSolver, true);
34423441
switch (binding.Kind) {
34433442
case AllowedBindingKind::Exact:
34443443
break;
@@ -3454,7 +3453,7 @@ class ConstraintSystem {
34543453
if (binding.DefaultedProtocol)
34553454
out << "(default from "
34563455
<< binding.DefaultedProtocol->getName() << ") ";
3457-
out << type.getString();
3456+
out << type.getString(PO);
34583457
},
34593458
[&]() { out << "; "; });
34603459
out << "}";
@@ -4188,8 +4187,10 @@ class TypeVariableBinding {
41884187
bool attempt(ConstraintSystem &cs) const;
41894188

41904189
void print(llvm::raw_ostream &Out, SourceManager *) const {
4191-
Out << "type variable " << TypeVar->getString()
4192-
<< " := " << Binding.BindingType->getString();
4190+
PrintOptions PO;
4191+
PO.PrintTypesForDebugging = true;
4192+
Out << "type variable " << TypeVar->getString(PO)
4193+
<< " := " << Binding.BindingType->getString(PO);
41934194
}
41944195
};
41954196

‎lib/Sema/SourceLoader.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,8 @@ ModuleDecl *SourceLoader::loadModule(SourceLoc importLoc,
106106
/*isSystem=*/false);
107107

108108
// Turn off debugging while parsing other modules.
109-
llvm::SaveAndRestore<bool> turnOffDebug(Ctx.LangOpts.DebugConstraintSolver,
110-
false);
109+
llvm::SaveAndRestore<bool>
110+
turnOffDebug(Ctx.TypeCheckerOpts.DebugConstraintSolver, false);
111111

112112
unsigned bufferID;
113113
if (auto BufID =

‎lib/Sema/TypeCheckConstraints.cpp

+5-5
Original file line numberDiff line numberDiff line change
@@ -3686,7 +3686,7 @@ void ConstraintSystem::print(raw_ostream &out) const {
36863686
out << "Type Variables:\n";
36873687
for (auto tv : getTypeVariables()) {
36883688
out.indent(2);
3689-
out << tv->getString(PO);
3689+
Type(tv).print(out, PO);
36903690
if (tv->getImpl().canBindToLValue())
36913691
out << " [lvalue allowed]";
36923692
if (tv->getImpl().canBindToInOut())
@@ -3703,7 +3703,7 @@ void ConstraintSystem::print(raw_ostream &out) const {
37033703
}
37043704
} else {
37053705
out << " equivalent to ";
3706-
out << rep->getString(PO);
3706+
Type(rep).print(out, PO);
37073707
}
37083708

37093709
if (auto *locator = tv->getImpl().getLocator()) {
@@ -3751,10 +3751,10 @@ void ConstraintSystem::print(raw_ostream &out) const {
37513751
case OverloadChoiceKind::DeclViaBridge:
37523752
case OverloadChoiceKind::DeclViaUnwrappedOptional:
37533753
if (choice.getBaseType())
3754-
out << choice.getBaseType()->getString() << ".";
3754+
out << choice.getBaseType()->getString(PO) << ".";
37553755
out << choice.getDecl()->getBaseName() << ": "
3756-
<< resolved->BoundType->getString() << " == "
3757-
<< resolved->ImpliedType->getString() << "\n";
3756+
<< resolved->BoundType->getString(PO) << " == "
3757+
<< resolved->ImpliedType->getString(PO) << "\n";
37583758
break;
37593759

37603760
case OverloadChoiceKind::BaseType:

0 commit comments

Comments
 (0)