Store argument labels + locations in directly in call and call-like expressions

lib/AST/ASTDumper.cpp

@@ -2093,6 +2093,12 @@ class PrintExpr : public ExprVisitor<PrintExpr> {
       OS << " super";
     if (E->isThrowsSet())
       OS << (E->throws() ? " throws" : " nothrow");
+    if (auto call = dyn_cast<CallExpr>(E)) {
+      OS << "  arg_labels=";
+      for (auto label : call->getArgumentLabels()) 
+        OS << (label.empty() ? "_" : label.str()) << ":";
+    }
+
     OS << '\n';
     printRec(E->getFn());
     OS << '\n';

lib/Parse/ParseExpr.cpp

@@ -845,7 +845,8 @@ ParserResult<Expr> Parser::parseExprSuper() {
       return makeParserCodeCompletionResult<Expr>();
     if (idx.isNull())
       return nullptr;
-    return makeParserResult(new (Context) SubscriptExpr(superRef, idx.get()));
+    return makeParserResult(
+             SubscriptExpr::create(Context, superRef, idx.get()));
   }
 
   if (Tok.is(tok::code_complete)) {
@@ -1231,10 +1232,9 @@ ParserResult<Expr> Parser::parseExprPostfix(Diag<> ID, bool isExprBasic) {
     DeclNameLoc NameLoc;
 
     if (Tok.is(tok::code_complete)) {
-      auto Expr = new (Context) UnresolvedMemberExpr(
-                                  DotLoc,
-                                  DeclNameLoc(DotLoc.getAdvancedLoc(1)),
-                                  Context.getIdentifier("_"), nullptr);
+      auto Expr = UnresolvedMemberExpr::create(
+                    Context, DotLoc, DeclNameLoc(DotLoc.getAdvancedLoc(1)),
+                    Context.getIdentifier("_"), /*implicit=*/false);
       Result = makeParserResult(Expr);
       if (CodeCompletion) {
         std::vector<StringRef> Identifiers;
@@ -1279,8 +1279,9 @@ ParserResult<Expr> Parser::parseExprPostfix(Diag<> ID, bool isExprBasic) {
 
     // Handle .foo by just making an AST node.
     Result = makeParserResult(
-               new (Context) UnresolvedMemberExpr(DotLoc, NameLoc, Name,
-                                                  Arg.getPtrOrNull()));
+               UnresolvedMemberExpr::create(Context, DotLoc, NameLoc, Name,
+                                            Arg.getPtrOrNull(),
+                                            /*implicit=*/false));
     break;
   }
 
@@ -1529,7 +1530,7 @@ ParserResult<Expr> Parser::parseExprPostfix(Diag<> ID, bool isExprBasic) {
       if (Idx.isNull() || Result.isNull())
         return nullptr;
       Result = makeParserResult(
-          new (Context) SubscriptExpr(Result.get(), Idx.get()));
+                 SubscriptExpr::create(Context, Result.get(), Idx.get()));
       continue;
     }
 
@@ -1557,12 +1558,14 @@ ParserResult<Expr> Parser::parseExprPostfix(Diag<> ID, bool isExprBasic) {
       if (auto call = dyn_cast<CallExpr>(Result.get())) {
         // When a closure follows a call, it becomes the last argument of
         // that call.
+        // FIXME: Wasteful to allocate another CallExpr here. Delay the
+        // allocation.
         Expr *arg = addTrailingClosureToArgument(Context, call->getArg(),
                                                  closure.get());
-        call->setArg(arg);
-
-        if (closure.hasCodeCompletion())
-          Result.setHasCodeCompletion();
+        Result = makeParserResult(
+                   ParserStatus(closure),
+                   CallExpr::create(Context, call->getFn(), arg,
+                                    call->isImplicit()));
       } else {
         // Otherwise, the closure implicitly forms a call.
         Expr *arg = createArgWithTrailingClosure(Context, SourceLoc(), { },
@@ -2643,8 +2646,8 @@ Parser::parseExprObjectLiteral(ObjectLiteralExpr::LiteralKind LitKind,
   }
 
   return makeParserResult(
-    new (Context) ObjectLiteralExpr(PoundLoc, LitKind, Arg.get(),
-                                    /*implicit=*/false));
+    ObjectLiteralExpr::create(Context, PoundLoc, LitKind, Arg.get(),
+                              /*implicit=*/false));
 }
 
 /// \brief Parse an expression call suffix.

lib/Sema/CSApply.cpp

@@ -1180,13 +1180,16 @@ namespace {
     /// the call. Otherwise, a specific level describing which parameter level
     /// we're applying.
     /// \param argLabels The argument labels provided for the call.
+    /// \param hasTrailingClosure Whether the last argument is a trailing
+    /// closure.
     /// \param locator Locator used to describe where in this expression we are.
     ///
     /// \returns the coerced expression, which will have type \c ToType.
     Expr *
     coerceCallArguments(Expr *arg, Type paramType,
                         llvm::PointerUnion<ApplyExpr *, LevelTy> applyOrLevel,
                         ArrayRef<Identifier> argLabels,
+                        bool hasTrailingClosure,
                         ConstraintLocatorBuilder locator);
 
     /// \brief Coerce the given object argument (e.g., for the base of a
@@ -1215,6 +1218,7 @@ namespace {
     /// \param isImplicit Whether this is an implicit subscript.
     Expr *buildSubscript(Expr *base, Expr *index,
                          ArrayRef<Identifier> argLabels,
+                         bool hasTrailingClosure,
                          ConstraintLocatorBuilder locator,
                          bool isImplicit, AccessSemantics semantics) {
       // Determine the declaration selected for this subscript operation.
@@ -1259,6 +1263,7 @@ namespace {
       // Coerce the index argument.
       index = coerceCallArguments(
           index, indexTy, LevelTy(1), argLabels,
+          hasTrailingClosure,
           locator.withPathElement(ConstraintLocator::SubscriptIndex));
       if (!index)
         return nullptr;
@@ -1274,11 +1279,10 @@ namespace {
           return nullptr;
 
         // TODO: diagnose if semantics != AccessSemantics::Ordinary?
-        auto subscriptExpr = new (tc.Context) DynamicSubscriptExpr(base,
-                                                                   index,
-                                                                   subscript);
+        auto subscriptExpr = DynamicSubscriptExpr::create(tc.Context, base,
+                                                          index, subscript,
+                                                          isImplicit);
         subscriptExpr->setType(resultTy);
-        subscriptExpr->setImplicit(isImplicit);
         Expr *result = subscriptExpr;
         closeExistential(result);
         return result;
@@ -1310,12 +1314,11 @@ namespace {
 
         // Form the generic subscript expression.
         auto subscriptExpr
-          = new (tc.Context) SubscriptExpr(base, index,
-                                           ConcreteDeclRef(tc.Context,
-                                                           subscript,
-                                                           substitutions),
-                                           isImplicit,
-                                           semantics);
+          = SubscriptExpr::create(tc.Context, base, index,
+                                  ConcreteDeclRef(tc.Context, subscript,
+                                                  substitutions),
+                                  isImplicit,
+                                  semantics);
         subscriptExpr->setType(resultTy);
         subscriptExpr->setIsSuper(isSuper);
 
@@ -1337,8 +1340,8 @@ namespace {
 
       // Form a normal subscript.
       auto *subscriptExpr
-        = new (tc.Context) SubscriptExpr(base, index, subscript,
-                                         isImplicit, semantics);
+        = SubscriptExpr::create(tc.Context, base, index, subscript,
+                                isImplicit, semantics);
       subscriptExpr->setType(resultTy);
       subscriptExpr->setIsSuper(isSuper);
       Expr *result = subscriptExpr;
@@ -2603,9 +2606,9 @@ namespace {
     }
 
     Expr *visitSubscriptExpr(SubscriptExpr *expr) {
-      SmallVector<Identifier, 2> argLabelsScratch;
       return buildSubscript(expr->getBase(), expr->getIndex(),
-                            expr->getArgumentLabels(argLabelsScratch),
+                            expr->getArgumentLabels(),
+                            expr->hasTrailingClosure(),
                             cs.getConstraintLocator(expr),
                             expr->isImplicit(),
                             expr->getAccessSemantics());
@@ -2747,9 +2750,9 @@ namespace {
     }
 
     Expr *visitDynamicSubscriptExpr(DynamicSubscriptExpr *expr) {
-      SmallVector<Identifier, 2> argLabelsScratch;
       return buildSubscript(expr->getBase(), expr->getIndex(),
-                            expr->getArgumentLabels(argLabelsScratch),
+                            expr->getArgumentLabels(),
+                            expr->hasTrailingClosure(),
                             cs.getConstraintLocator(expr),
                             expr->isImplicit(), AccessSemantics::Ordinary);
     }
@@ -4638,6 +4641,7 @@ Expr *ExprRewriter::coerceCallArguments(
     Expr *arg, Type paramType,
     llvm::PointerUnion<ApplyExpr *, LevelTy> applyOrLevel,
     ArrayRef<Identifier> argLabels,
+    bool hasTrailingClosure,
     ConstraintLocatorBuilder locator) {
 
   bool allParamsMatch = arg->getType()->isEqual(paramType);
@@ -4704,7 +4708,7 @@ Expr *ExprRewriter::coerceCallArguments(
 
   SmallVector<ParamBinding, 4> parameterBindings;
   bool failed = constraints::matchCallArguments(args, params,
-                                                hasTrailingClosure(locator),
+                                                hasTrailingClosure,
                                                 /*allowFixes=*/false, listener,
                                                 parameterBindings);
   assert(!failed && "Call arguments did not match up?");
@@ -5922,9 +5926,12 @@ Expr *ExprRewriter::finishApply(ApplyExpr *apply, Type openedType,
   SmallVector<Identifier, 2> argLabelsScratch;
   if (auto fnType = fn->getType()->getAs<FunctionType>()) {
     auto origArg = apply->getArg();
+    bool hasTrailingClosure =
+      isa<CallExpr>(apply) && cast<CallExpr>(apply)->hasTrailingClosure();
     Expr *arg = coerceCallArguments(origArg, fnType->getInput(),
                                     apply,
                                     apply->getArgumentLabels(argLabelsScratch),
+                                    hasTrailingClosure,
                                     locator.withPathElement(
                                       ConstraintLocator::ApplyArgument));
     if (!arg) {

lib/Sema/CSDiag.cpp

@@ -4577,8 +4577,7 @@ bool FailureDiagnosis::visitSubscriptExpr(SubscriptExpr *SE) {
   for (unsigned i = 0, e = calleeInfo.size(); i != e; ++i)
     --calleeInfo.candidates[i].level;
 
-  SmallVector<Identifier, 2> argLabelsScratch;
-  ArrayRef<Identifier> argLabels = SE->getArgumentLabels(argLabelsScratch);
+  ArrayRef<Identifier> argLabels = SE->getArgumentLabels();
   if (diagnoseParameterErrors(calleeInfo, SE, indexExpr, argLabels))
     return true;
 

lib/Sema/CSSimplify.cpp

@@ -469,45 +469,53 @@ matchCallArguments(ArrayRef<CallArgParam> args,
 
 /// Find the callee declaration and uncurry level for a given call
 /// locator.
-static std::tuple<ValueDecl *, unsigned, ArrayRef<Identifier>>
+static std::tuple<ValueDecl *, unsigned, ArrayRef<Identifier>, bool>
 getCalleeDeclAndArgs(ConstraintSystem &cs,
                      ConstraintLocatorBuilder callLocator,
                      SmallVectorImpl<Identifier> &argLabelsScratch) {
   ArrayRef<Identifier> argLabels;
+  bool hasTrailingClosure = false;
 
-  // If the call locator is not just a call expression, there's
-  // nothing to do.
+  // Break down the call.
   SmallVector<LocatorPathElt, 2> path;
   auto callExpr = callLocator.getLocatorParts(path);
-  if (!callExpr) return std::make_tuple(nullptr, 0, argLabels);
+  if (!callExpr) 
+    return std::make_tuple(nullptr, 0, argLabels, hasTrailingClosure);
 
   // Our remaining path can only be 'ApplyArgument' or 'SubscriptIndex'.
   if (!path.empty() &&
       !(path.size() == 1 &&
         (path.back().getKind() == ConstraintLocator::ApplyArgument ||
          path.back().getKind() == ConstraintLocator::SubscriptIndex)))
-    return std::make_tuple(nullptr, 0, argLabels);
+    return std::make_tuple(nullptr, 0, argLabels, hasTrailingClosure);
 
   // Dig out the callee.
   Expr *calleeExpr;
   if (auto call = dyn_cast<CallExpr>(callExpr)) {
     calleeExpr = call->getDirectCallee();
-    argLabels = call->getArgumentLabels(argLabelsScratch);
+    argLabels = call->getArgumentLabels();
+    hasTrailingClosure = call->hasTrailingClosure();
   } else if (auto unresolved = dyn_cast<UnresolvedMemberExpr>(callExpr)) {
     calleeExpr = callExpr;
-    argLabels = unresolved->getArgumentLabels(argLabelsScratch);
+    argLabels = unresolved->getArgumentLabels();
+    hasTrailingClosure = unresolved->hasTrailingClosure();
   } else if (auto subscript = dyn_cast<SubscriptExpr>(callExpr)) {
     calleeExpr = callExpr;
-    argLabels = subscript->getArgumentLabels(argLabelsScratch);
+    argLabels = subscript->getArgumentLabels();
+    hasTrailingClosure = subscript->hasTrailingClosure();
   } else if (auto dynSubscript = dyn_cast<DynamicSubscriptExpr>(callExpr)) {
     calleeExpr = callExpr;
-    argLabels = dynSubscript->getArgumentLabels(argLabelsScratch);
+    argLabels = dynSubscript->getArgumentLabels();
+    hasTrailingClosure = dynSubscript->hasTrailingClosure();
   } else {
-    if (auto apply = dyn_cast<ApplyExpr>(callExpr))
+    if (auto apply = dyn_cast<ApplyExpr>(callExpr)) {
       argLabels = apply->getArgumentLabels(argLabelsScratch);
-    else if (auto objectLiteral = dyn_cast<ObjectLiteralExpr>(callExpr))
-      argLabels = objectLiteral->getArgumentLabels(argLabelsScratch);
-    return std::make_tuple(nullptr, 0, argLabels);
+      assert(!apply->hasTrailingClosure());
+    } else if (auto objectLiteral = dyn_cast<ObjectLiteralExpr>(callExpr)) {
+      argLabels = objectLiteral->getArgumentLabels();
+      hasTrailingClosure = objectLiteral->hasTrailingClosure();
+    }
+    return std::make_tuple(nullptr, 0, argLabels, hasTrailingClosure);
   }
 
   // Determine the target locator.
@@ -551,7 +559,8 @@ getCalleeDeclAndArgs(ConstraintSystem &cs,
   }
 
   // If we didn't find any matching overloads, we're done.
-  if (!choice) return std::make_tuple(nullptr, 0, argLabels);
+  if (!choice)
+    return std::make_tuple(nullptr, 0, argLabels, hasTrailingClosure);
 
   // If there's a declaration, return it.
   if (choice->isDecl()) {
@@ -570,10 +579,10 @@ getCalleeDeclAndArgs(ConstraintSystem &cs,
       }
     }
 
-    return std::make_tuple(decl, level, argLabels);
+    return std::make_tuple(decl, level, argLabels, hasTrailingClosure);
   }
 
-  return std::make_tuple(nullptr, 0, argLabels);
+  return std::make_tuple(nullptr, 0, argLabels, hasTrailingClosure);
 }
 
 // Match the argument of a call to the parameter.
@@ -604,7 +613,8 @@ matchCallArguments(ConstraintSystem &cs, TypeMatchKind kind,
   unsigned calleeLevel;
   ArrayRef<Identifier> argLabels;
   SmallVector<Identifier, 2> argLabelsScratch;
-  std::tie(callee, calleeLevel, argLabels) =
+  bool hasTrailingClosure = false;
+  std::tie(callee, calleeLevel, argLabels, hasTrailingClosure) =
     getCalleeDeclAndArgs(cs, locator, argLabelsScratch);
   auto params = decomposeParamType(paramType, callee, calleeLevel);
 
@@ -614,8 +624,7 @@ matchCallArguments(ConstraintSystem &cs, TypeMatchKind kind,
   // Match up the call arguments to the parameters.
   MatchCallArgumentListener listener;
   SmallVector<ParamBinding, 4> parameterBindings;
-  if (constraints::matchCallArguments(args, params,
-                                      hasTrailingClosure(locator),
+  if (constraints::matchCallArguments(args, params, hasTrailingClosure,
                                       cs.shouldAttemptFixes(), listener,
                                       parameterBindings))
     return ConstraintSystem::SolutionKind::Error;

lib/Sema/CodeSynthesis.cpp

@@ -354,9 +354,11 @@ static Expr *buildArgumentForwardingExpr(ArrayRef<ParamDecl*> params,
     labelLocs.push_back(SourceLoc());
   }
 
-  // A single unlabelled value is not a tuple.
-  if (args.size() == 1 && labels[0].empty())
-    return args[0];
+  // A single unlabeled value is not a tuple.
+  if (args.size() == 1 && labels[0].empty()) {
+    return new (ctx) ParenExpr(SourceLoc(), args[0], SourceLoc(),
+                               /*hasTrailingClosure=*/false);
+  }
 
   return TupleExpr::create(ctx, SourceLoc(), args, labels, labelLocs,
                            SourceLoc(), false, IsImplicit);
@@ -477,8 +479,8 @@ static Expr *buildStorageReference(
 
   if (auto subscript = dyn_cast<SubscriptDecl>(storage)) {
     Expr *indices = referenceContext.getIndexRefExpr(ctx, subscript);
-    return new (ctx) SubscriptExpr(selfDRE, indices, storage,
-                                   IsImplicit, semantics);
+    return SubscriptExpr::create(ctx, selfDRE, indices, storage,
+                                 IsImplicit, semantics);
   }
 
   // This is a potentially polymorphic access, which is unnecessary;

lib/Sema/ConstraintSystem.h

@@ -2129,11 +2129,6 @@ static inline bool computeTupleShuffle(TupleType *fromTuple,
                              sources, variadicArgs);
 }
 
-
-/// \brief Return whether the function argument indicated by `locator` has a
-/// trailing closure.
-bool hasTrailingClosure(const ConstraintLocatorBuilder &locator);
-
 /// Describes the arguments to which a parameter binds.
 /// FIXME: This is an awful data structure. We want the equivalent of a
 /// TinyPtrVector for unsigned values.

lib/Sema/PlaygroundTransform.cpp

@@ -1009,8 +1009,11 @@ class Instrumenter {
 
     LoggerRef->setImplicit(true);
 
+    SmallVector<Identifier, 4> ArgLabels(ArgsWithSourceRange.size(),
+                                         Identifier());
     ApplyExpr *LoggerCall = CallExpr::createImplicit(Context, LoggerRef,
-                                                     ArgsWithSourceRange, { });
+                                                     ArgsWithSourceRange,
+                                                     ArgLabels);
     Added<ApplyExpr*> AddedLogger(LoggerCall);
 
     if (!doTypeCheck(Context, TypeCheckDC, AddedLogger)) {