[ASTPrinter] Switch to new ParameterTypeFlags

Switch printing off of using Function's ExtInfo for autoclosure and
escaping, and onto the ParameterTypeFlags, which let us do precise and
accurate context-sensitive printing of these parameter type
attributes. This fixes a huge list of issues where we were printing
@escaping for things like optional ObjC completion handlers, among
many others. We now correctly print @escaping in more places, and
don't print it when it's not correct.

Also updates the dumper to be consistent and give a good view of the
AST as represented in memory. Tests updated, more involved testing
coming soon.

Author: milseman

include/swift/AST/PrintOptions.h

@@ -244,6 +244,9 @@ struct PrintOptions {
   /// protocol requirements.
   bool SkipOverrides = false;
 
+  /// Whether to skip parameter type attributes
+  bool SkipParameterTypeAttributes = false;
+
   /// Whether to print a long attribute like '\@available' on a separate line
   /// from the declaration or other attributes.
   bool PrintLongAttrsOnSeparateLines = false;

lib/AST/ASTDumper.cpp

@@ -2621,6 +2621,15 @@ namespace {
       return OS;
     }
 
+    void dumpParameterFlags(ParameterTypeFlags paramFlags) {
+      if (paramFlags.isVariadic())
+        printFlag("vararg");
+      if (paramFlags.isAutoClosure())
+        printFlag("autoclosure");
+      if (paramFlags.isEscaping())
+        printFlag("escaping");
+    }
+
   public:
     PrintType(raw_ostream &os, unsigned indent) : OS(os), Indent(indent) { }
 
@@ -2684,6 +2693,7 @@ namespace {
 
     void visitParenType(ParenType *T, StringRef label) {
       printCommon(T, label, "paren_type");
+      dumpParameterFlags(T->getParameterFlags());
       printRec(T->getUnderlyingType());
       OS << ")";
     }
@@ -2698,9 +2708,7 @@ namespace {
         PrintWithColorRAII(OS, TypeFieldColor) << "tuple_type_elt";
         if (elt.hasName())
           printField("name", elt.getName().str());
-        if (elt.isVararg())
-          printFlag("vararg");
-
+        dumpParameterFlags(elt.getParameterFlags());
         printRec(elt.getType());
         OS << ")";
       }
@@ -2911,10 +2919,7 @@ namespace {
       }
 
       printFlag(T->isAutoClosure(), "autoclosure");
-
-      // Dump out either @noescape or @escaping
-      printFlag(!T->isNoEscape(), "@escaping");
-
+      printFlag(!T->isNoEscape(), "escaping");
       printFlag(T->throws(), "throws");
 
       printRec("input", T->getInput());

lib/AST/ASTPrinter.cpp

@@ -992,6 +992,20 @@ void StreamPrinter::printText(StringRef Text) {
   OS << Text;
 }
 
+/// Whether we will be printing a TypeLoc by using the TypeRepr printer
+static bool willUseTypeReprPrinting(TypeLoc tyLoc,
+                                    const PrintOptions &options) {
+  // Special case for when transforming archetypes
+  if (options.TransformContext && tyLoc.getType() &&
+      options.TransformContext->transform(tyLoc.getType()))
+    return false;
+
+  // Otherwise, whether we have a type repr and prefer that
+  return ((options.PreferTypeRepr && tyLoc.hasLocation()) ||
+          tyLoc.getType().isNull()) &&
+         tyLoc.getTypeRepr();
+}
+
 namespace {
 /// \brief AST pretty-printer.
 class PrintAST : public ASTVisitor<PrintAST> {
@@ -1187,8 +1201,11 @@ class PrintAST : public ASTVisitor<PrintAST> {
     // is null.
     if ((Options.PreferTypeRepr && TL.hasLocation()) ||
         TL.getType().isNull()) {
-      if (auto repr = TL.getTypeRepr())
+      if (auto repr = TL.getTypeRepr()) {
+        llvm::SaveAndRestore<bool> SPTA(Options.SkipParameterTypeAttributes,
+                                        true);
         repr->print(Printer, Options);
+      }
       return;
     }
 
@@ -1232,10 +1249,10 @@ class PrintAST : public ASTVisitor<PrintAST> {
   /// \returns true if anything was printed.
   bool printASTNodes(const ArrayRef<ASTNode> &Elements, bool NeedIndent = true);
 
-  void printOneParameter(const ParamDecl *param, bool Curried,
-                         bool ArgNameIsAPIByDefault);
+  void printOneParameter(const ParamDecl *param, ParameterTypeFlags paramFlags,
+                         bool Curried, bool ArgNameIsAPIByDefault);
 
-  void printParameterList(ParameterList *PL, bool isCurried,
+  void printParameterList(ParameterList *PL, Type paramListTy, bool isCurried,
                           std::function<bool()> isAPINameByDefault);
 
   /// \brief Print the function parameters in curried or selector style,
@@ -2500,6 +2517,14 @@ static bool isStructOrClassContext(DeclContext *dc) {
   return false;
 }
 
+static void printParameterFlags(ASTPrinter &printer, PrintOptions options,
+                                ParameterTypeFlags flags) {
+  if (!options.excludeAttrKind(TAK_autoclosure) && flags.isAutoClosure())
+    printer << "@autoclosure ";
+  if (!options.excludeAttrKind(TAK_escaping) && flags.isEscaping())
+    printer << "@escaping ";
+}
+
 void PrintAST::visitVarDecl(VarDecl *decl) {
   printDocumentationComment(decl);
   // Print @sil_stored when the attribute is not already
@@ -2536,7 +2561,8 @@ void PrintAST::visitParamDecl(ParamDecl *decl) {
   visitVarDecl(decl);
 }
 
-void PrintAST::printOneParameter(const ParamDecl *param, bool Curried,
+void PrintAST::printOneParameter(const ParamDecl *param,
+                                 ParameterTypeFlags paramFlags, bool Curried,
                                  bool ArgNameIsAPIByDefault) {
   Printer.callPrintStructurePre(PrintStructureKind::FunctionParameter, param);
   SWIFT_DEFER {
@@ -2590,7 +2616,25 @@ void PrintAST::printOneParameter(const ParamDecl *param, bool Curried,
   // Set and restore in-parameter-position printing of types
   {
     llvm::SaveAndRestore<bool> savePrintParam(Options.PrintAsInParamType, true);
+    // FIXME: don't do if will be using type repr printing
+    printParameterFlags(Printer, Options, paramFlags);
+
+    // Special case, if we're not going to use the type repr printing, peek
+    // through the paren types so that we don't print excessive @escapings
+    unsigned numParens = 0;
+    if (!willUseTypeReprPrinting(TheTypeLoc, Options)) {
+      while (auto parenTy =
+                 dyn_cast<ParenType>(TheTypeLoc.getType().getPointer())) {
+        ++numParens;
+        TheTypeLoc = TypeLoc::withoutLoc(parenTy->getUnderlyingType());
+      }
+    }
+
+    for (unsigned i = 0; i < numParens; ++i)
+      Printer << "(";
     printTypeLoc(TheTypeLoc);
+    for (unsigned i = 0; i < numParens; ++i)
+      Printer << ")";
   }
 
   if (param->isVariadic())
@@ -2621,28 +2665,68 @@ void PrintAST::printOneParameter(const ParamDecl *param, bool Curried,
   }
 }
 
-void PrintAST::printParameterList(ParameterList *PL, bool isCurried,
-                            std::function<bool()> isAPINameByDefault) {
+void PrintAST::printParameterList(ParameterList *PL, Type paramListTy,
+                                  bool isCurried,
+                                  std::function<bool()> isAPINameByDefault) {
+  SmallVector<ParameterTypeFlags, 4> paramFlags;
+  if (paramListTy) {
+    if (auto parenTy = dyn_cast<ParenType>(paramListTy.getPointer())) {
+      paramFlags.push_back(parenTy->getParameterFlags());
+    } else if (auto tupleTy = paramListTy->getAs<TupleType>()) {
+      for (auto elt : tupleTy->getElements())
+        paramFlags.push_back(elt.getParameterFlags());
+    } else {
+      paramFlags.push_back({});
+    }
+  } else {
+    // Malformed AST, just use default flags
+    paramFlags.resize(PL->size());
+  }
+
   Printer << "(";
   for (unsigned i = 0, e = PL->size(); i != e; ++i) {
     if (i > 0)
       Printer << ", ";
 
-    printOneParameter(PL->get(i), isCurried, isAPINameByDefault());
+    printOneParameter(PL->get(i), paramFlags[i], isCurried,
+                      isAPINameByDefault());
   }
   Printer << ")";
 }
 
 void PrintAST::printFunctionParameters(AbstractFunctionDecl *AFD) {
   auto BodyParams = AFD->getParameterLists();
+  auto curTy = AFD->hasType() ? AFD->getType() : nullptr;
 
   // Skip over the implicit 'self'.
-  if (AFD->getImplicitSelfDecl())
+  if (AFD->getImplicitSelfDecl()) {
     BodyParams = BodyParams.slice(1);
+    if (curTy)
+      if (auto funTy = curTy->getAs<AnyFunctionType>())
+        curTy = funTy->getResult();
+  }
+
+  SmallVector<Type, 4> parameterListTypes;
+  for (auto i = 0; i < BodyParams.size(); ++i) {
+    if (curTy) {
+      if (auto funTy = curTy->getAs<AnyFunctionType>()) {
+        parameterListTypes.push_back(funTy->getInput());
+        if (i < BodyParams.size() - 1)
+          curTy = funTy->getResult();
+      } else {
+        parameterListTypes.push_back(curTy);
+      }
+    }
+  }
 
   for (unsigned CurrPattern = 0, NumPatterns = BodyParams.size();
        CurrPattern != NumPatterns; ++CurrPattern) {
-    printParameterList(BodyParams[CurrPattern], /*Curried=*/CurrPattern > 0,
+    // Be extra careful in the event of printing mal-formed ASTs
+    auto paramListType = parameterListTypes.size() > CurrPattern
+                             ? parameterListTypes[CurrPattern]
+                             : nullptr;
+    printParameterList(BodyParams[CurrPattern], paramListType,
+                       /*Curried=*/CurrPattern > 0,
                        [&]()->bool {
       return CurrPattern > 0 || AFD->argumentNameIsAPIByDefault();
     });
@@ -2889,7 +2973,8 @@ void PrintAST::visitSubscriptDecl(SubscriptDecl *decl) {
   recordDeclLoc(decl, [&]{
     Printer << "subscript";
   }, [&] { // Parameters
-    printParameterList(decl->getIndices(), /*Curried=*/false,
+    printParameterList(decl->getIndices(), decl->getIndicesType(),
+                       /*Curried=*/false,
                        /*isAPINameByDefault*/[]()->bool{return false;});
   });
   Printer << " -> ";
@@ -3609,6 +3694,7 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
 
   void visitParenType(ParenType *T) {
     Printer << "(";
+    printParameterFlags(Printer, Options, T->getParameterFlags());
     visit(T->getUnderlyingType());
     Printer << ")";
   }
@@ -3638,8 +3724,10 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
       if (TD.isVararg()) {
         visit(TD.getVarargBaseTy());
         Printer << "...";
-      } else
+      } else {
+        printParameterFlags(Printer, Options, TD.getParameterFlags());
         visit(EltType);
+      }
     }
     Printer << ")";
   }
@@ -3765,15 +3853,6 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
     if (Options.SkipAttributes)
       return;
 
-    if (info.isAutoClosure() && !Options.excludeAttrKind(TAK_autoclosure)) {
-      Printer.printSimpleAttr("@autoclosure");
-      Printer << " ";
-    }
-    if (inParameterPrinting && !info.isNoEscape() &&
-        !Options.excludeAttrKind(TAK_escaping)) {
-      Printer.printSimpleAttr("@escaping");
-      Printer << " ";
-    }
 
     if (Options.PrintFunctionRepresentationAttrs &&
         !Options.excludeAttrKind(TAK_convention)) {

lib/AST/TypeRepr.cpp

@@ -283,13 +283,11 @@ void AttributedTypeRepr::printAttrs(ASTPrinter &Printer,
     return Attrs.has(K);
   };
 
-  if (hasAttr(TAK_autoclosure)) {
-    Printer.printSimpleAttr("@autoclosure");
-    Printer << " ";
-  }
-  if (hasAttr(TAK_escaping)) {
-    Printer.printSimpleAttr("@escaping");
-    Printer << " ";
+  if (!Options.SkipParameterTypeAttributes) {
+    if (hasAttr(TAK_autoclosure))
+      Printer.printSimpleAttr("@autoclosure") << " ";
+    if (hasAttr(TAK_escaping))
+      Printer.printSimpleAttr("@escaping") << " ";
   }
 
   if (hasAttr(TAK_thin)) {

test/Constraints/diagnostics.swift

@@ -60,7 +60,7 @@ f1(
    )
 
 f3(
-   f2 // expected-error {{cannot convert value of type '((@escaping (Int) -> Int)) -> Int' to expected argument type '(@escaping (Int) -> Float) -> Int'}}
+   f2 // expected-error {{cannot convert value of type '(@escaping ((Int) -> Int)) -> Int' to expected argument type '(@escaping (Int) -> Float) -> Int'}}
    )
 
 f4(i, d) // expected-error {{extra argument in call}}
@@ -672,7 +672,7 @@ func overloadSetResultType(_ a : Int, b : Int) -> Int {
   // https://twitter.com/_jlfischer/status/712337382175952896
   // TODO: <rdar://problem/27391581> QoI: Nonsensical "binary operator '&&' cannot be applied to two 'Bool' operands"
   return a == b && 1 == 2  // expected-error {{binary operator '&&' cannot be applied to two 'Bool' operands}}
-  // expected-note @-1 {{expected an argument list of type '(Bool, @autoclosure () throws -> Bool)'}}
+  // expected-note @-1 {{expected an argument list of type '(Bool, () throws -> Bool)'}}
 }
 
 // <rdar://problem/21523291> compiler error message for mutating immutable field is incorrect
@@ -736,7 +736,7 @@ extension Foo23752537 {
     // TODO: <rdar://problem/27391581> QoI: Nonsensical "binary operator '&&' cannot be applied to two 'Bool' operands"
     // expected-error @+1 {{binary operator '&&' cannot be applied to two 'Bool' operands}}
     return (self.title != other.title && self.message != other.message)
-    // expected-note @-1 {{expected an argument list of type '(Bool, @autoclosure () throws -> Bool)'}}
+    // expected-note @-1 {{expected an argument list of type '(Bool, () throws -> Bool)'}}
   }
 }