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.

Author: CodaFi

include/swift/AST/PrintOptions.h

@@ -270,6 +270,10 @@ struct PrintOptions {
   /// Whether to skip keywords with a prefix of underscore such as __consuming.
   bool SkipUnderscoredKeywords = false;
 
+  /// Prints type variables and unresolved types in an expanded notation suitable
+  /// for debugging.
+  bool PrintTypesForDebugging = false;
+  
   /// How to print opaque return types.
   enum class OpaqueReturnTypePrintingMode {
     /// 'some P1 & P2'.

lib/AST/ASTDumper.cpp

@@ -1262,15 +1262,6 @@ void ParameterList::dump() const {
 }
 
 void ParameterList::dump(raw_ostream &OS, unsigned Indent) const {
-  llvm::Optional<llvm::SaveAndRestore<bool>> X;
-  
-  // Make sure to print type variables if we can get to ASTContext.
-  if (size() != 0 && get(0)) {
-    auto &ctx = get(0)->getASTContext();
-    X.emplace(llvm::SaveAndRestore<bool>(ctx.LangOpts.DebugConstraintSolver,
-                                         true));
-  }
-  
   PrintDecl(OS, Indent).printParameterList(this);
   llvm::errs() << '\n';
 }
@@ -1293,9 +1284,6 @@ void Decl::dump(const char *filename) const {
 }
 
 void Decl::dump(raw_ostream &OS, unsigned Indent) const {
-  // Make sure to print type variables.
-  llvm::SaveAndRestore<bool> X(getASTContext().LangOpts.DebugConstraintSolver,
-                               true);
   PrintDecl(OS, Indent).visit(const_cast<Decl *>(this));
   OS << '\n';
 }
@@ -1407,8 +1395,6 @@ void SourceFile::dump() const {
 }
 
 void SourceFile::dump(llvm::raw_ostream &OS) const {
-  llvm::SaveAndRestore<bool> X(getASTContext().LangOpts.DebugConstraintSolver,
-                               true);
   PrintDecl(OS).visitSourceFile(*this);
   llvm::errs() << '\n';
 }
@@ -1826,13 +1812,17 @@ class PrintExpr : public ExprVisitor<PrintExpr> {
   }
 
   raw_ostream &printCommon(Expr *E, const char *C) {
+    PrintOptions PO;
+    PO.PrintTypesForDebugging = true;
+
     OS.indent(Indent);
     PrintWithColorRAII(OS, ParenthesisColor) << '(';
     PrintWithColorRAII(OS, ExprColor) << C;
 
     if (E->isImplicit())
       PrintWithColorRAII(OS, ExprModifierColor) << " implicit";
-    PrintWithColorRAII(OS, TypeColor) << " type='" << GetTypeOfExpr(E) << '\'';
+    PrintWithColorRAII(OS, TypeColor) << " type='";
+    PrintWithColorRAII(OS, TypeColor) << GetTypeOfExpr(E).getString(PO) << '\'';
 
     // If we have a source range and an ASTContext, print the source range.
     if (auto Ty = GetTypeOfExpr(E)) {
@@ -3749,14 +3739,10 @@ namespace {
 } // end anonymous namespace
 
 void Type::dump() const {
-  // Make sure to print type variables.
   dump(llvm::errs());
 }
 
 void Type::dump(raw_ostream &os, unsigned indent) const {
-  // Make sure to print type variables.
-  llvm::SaveAndRestore<bool> X(getPointer()->getASTContext().LangOpts.
-                               DebugConstraintSolver, true);
   PrintType(os, indent).visit(*this, "");
   os << "\n";
 }
@@ -3767,10 +3753,6 @@ void TypeBase::dump() const {
 }
 
 void TypeBase::dump(raw_ostream &os, unsigned indent) const {
-  auto &ctx = const_cast<TypeBase*>(this)->getASTContext();
-  
-  // Make sure to print type variables.
-  llvm::SaveAndRestore<bool> X(ctx.LangOpts.DebugConstraintSolver, true);
   Type(const_cast<TypeBase *>(this)).dump(os, indent);
 }
 

lib/AST/ASTPrinter.cpp

@@ -3595,7 +3595,7 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
   }
 
   void visitUnresolvedType(UnresolvedType *T) {
-    if (T->getASTContext().LangOpts.DebugConstraintSolver)
+    if (Options.PrintTypesForDebugging)
       Printer << "<<unresolvedtype>>";
     else
       Printer << "_";
@@ -4331,7 +4331,7 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
 #include "swift/AST/ReferenceStorage.def"
 
   void visitTypeVariableType(TypeVariableType *T) {
-    if (T->getASTContext().LangOpts.DebugConstraintSolver) {
+    if (Options.PrintTypesForDebugging) {
       Printer << "$T" << T->getID();
       return;
     }

lib/Sema/CSStep.cpp

@@ -415,13 +415,15 @@ StepResult ComponentStep::finalize(bool isSuccess) {
 void TypeVariableStep::setup() {
   ++CS.solverState->NumTypeVariablesBound;
   if (isDebugMode()) {
+    PrintOptions PO;
+    PO.PrintTypesForDebugging = true;
     auto &log = getDebugLogger();
 
     log << "Initial bindings: ";
     interleave(InitialBindings.begin(), InitialBindings.end(),
                [&](const Binding &binding) {
-                 log << TypeVar->getString()
-                     << " := " << binding.BindingType->getString();
+                 log << TypeVar->getString(PO)
+                     << " := " << binding.BindingType->getString(PO);
                },
                [&log] { log << ", "; });
 

lib/Sema/CSStep.h

@@ -601,7 +601,9 @@ class TypeVariableStep final : public BindingStep<TypeVarBindingProducer> {
   StepResult resume(bool prevFailed) override;
 
   void print(llvm::raw_ostream &Out) override {
-    Out << "TypeVariableStep for " << TypeVar->getString() << " with #"
+    PrintOptions PO;
+    PO.PrintTypesForDebugging = true;
+    Out << "TypeVariableStep for " << TypeVar->getString(PO) << " with #"
         << InitialBindings.size() << " initial bindings\n";
   }
 

lib/Sema/Constraint.cpp

@@ -21,7 +21,6 @@
 #include "swift/Basic/Compiler.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/raw_ostream.h"
-#include "llvm/Support/SaveAndRestore.h"
 #include <algorithm>
 
 using namespace swift;
@@ -265,6 +264,10 @@ Constraint *Constraint::clone(ConstraintSystem &cs) const {
 }
 
 void Constraint::print(llvm::raw_ostream &Out, SourceManager *sm) const {
+  // Print all type variables as $T0 instead of _ here.
+  PrintOptions PO;
+  PO.PrintTypesForDebugging = true;
+  
   if (Kind == ConstraintKind::Disjunction) {
     Out << "disjunction";
     if (shouldRememberChoice())
@@ -286,7 +289,7 @@ void Constraint::print(llvm::raw_ostream &Out, SourceManager *sm) const {
     return;
   }
 
-  getFirstType()->print(Out);
+  Out << getFirstType()->getString(PO);
 
   bool skipSecond = false;
 
@@ -315,17 +318,17 @@ void Constraint::print(llvm::raw_ostream &Out, SourceManager *sm) const {
   case ConstraintKind::OneWayEqual: Out << " one-way bind to "; break;
   case ConstraintKind::KeyPath:
       Out << " key path from ";
-      getSecondType()->print(Out);
+      Out << getSecondType()->getString(PO);
       Out << " -> ";
-      getThirdType()->print(Out);
+      Out << getThirdType()->getString(PO);
       skipSecond = true;
       break;
 
   case ConstraintKind::KeyPathApplication:
       Out << " key path projecting ";
-      getSecondType()->print(Out);
+      Out << getSecondType()->getString(PO);
       Out << " -> ";
-      getThirdType()->print(Out);
+      Out << getThirdType()->getString(PO);
       skipSecond = true;
       break;
   case ConstraintKind::OptionalObject:
@@ -396,7 +399,7 @@ void Constraint::print(llvm::raw_ostream &Out, SourceManager *sm) const {
   }
 
   if (!skipSecond)
-    getSecondType()->print(Out);
+    Out << getSecondType()->getString(PO);
 
   if (auto restriction = getRestriction()) {
     Out << ' ' << getName(*restriction);
@@ -420,9 +423,6 @@ void Constraint::dump(SourceManager *sm) const {
 }
 
 void Constraint::dump(ConstraintSystem *CS) const {
-  // Print all type variables as $T0 instead of _ here.
-  llvm::SaveAndRestore<bool> X(CS->getASTContext().LangOpts.
-                               DebugConstraintSolver, true);
   // Disable MSVC warning: only for use within the debugger.
 #if SWIFT_COMPILER_IS_MSVC
 #pragma warning(push)

lib/Sema/ConstraintGraph.cpp

@@ -1319,9 +1319,10 @@ void ConstraintGraph::incrementConstraintsPerContractionCounter() {
 
 #pragma mark Debugging output
 
-void ConstraintGraphNode::print(llvm::raw_ostream &out, unsigned indent) const {
+void ConstraintGraphNode::print(llvm::raw_ostream &out, unsigned indent,
+                                PrintOptions PO) const {
   out.indent(indent);
-  TypeVar->print(out);
+  Type(TypeVar).print(out, PO);
   out << ":\n";
 
   // Print constraints.

lib/Sema/ConstraintGraph.h

@@ -160,7 +160,8 @@ class ConstraintGraphNode {
   mutable SmallVector<TypeVariableType *, 2> EquivalenceClass;
 
   /// Print this graph node.
-  void print(llvm::raw_ostream &out, unsigned indent) const;
+  void print(llvm::raw_ostream &out, unsigned indent,
+             PrintOptions PO = PrintOptions()) const;
 
   SWIFT_DEBUG_DUMP;
 

lib/Sema/ConstraintLocator.cpp

@@ -261,6 +261,9 @@ void ConstraintLocator::dump(ConstraintSystem *CS) const {
 
 
 void ConstraintLocator::dump(SourceManager *sm, raw_ostream &out) const {
+  PrintOptions PO;
+  PO.PrintTypesForDebugging = true;
+  
   out << "locator@" << (void*) this << " [";
 
   if (anchor) {
@@ -295,7 +298,7 @@ void ConstraintLocator::dump(SourceManager *sm, raw_ostream &out) const {
     switch (elt.getKind()) {
     case GenericParameter: {
       auto gpElt = elt.castTo<LocatorPathElt::GenericParameter>();
-      out << "generic parameter '" << gpElt.getType()->getString() << "'";
+      out << "generic parameter '" << gpElt.getType()->getString(PO) << "'";
       break;
     }
     case ApplyArgument:

lib/Sema/ConstraintSystem.cpp

@@ -2242,11 +2242,13 @@ void ConstraintSystem::resolveOverload(ConstraintLocator *locator,
   }
 
   if (getASTContext().TypeCheckerOpts.DebugConstraintSolver) {
+    PrintOptions PO;
+    PO.PrintTypesForDebugging = true;
     auto &log = getASTContext().TypeCheckerDebug->getStream();
     log.indent(solverState ? solverState->depth * 2 : 2)
       << "(overload set choice binding "
-      << boundType->getString() << " := "
-      << refType->getString() << ")\n";
+      << boundType->getString(PO) << " := "
+      << refType->getString(PO) << ")\n";
   }
 
   // If this overload is disfavored, note that.