Always create a DefaultArgumentInitializer for a parameter with a default argument.

As with pattern binding initializer contexts, we were trying to
optimize away these contexts, leading to an unpredictable AST.

Author: DougGregor

include/swift/AST/ASTContext.h

@@ -549,14 +549,6 @@ class ASTContext {
     addCleanup([&object]{ object.~T(); });
   }
 
-  /// Create a context for the initializer of the nth default argument
-  /// of the given function.  To reduce memory usage, if the context
-  /// goes unused, it should be returned to the ASTContext with
-  /// destroyDefaultArgumentContext.
-  DefaultArgumentInitializer *createDefaultArgumentContext(DeclContext *fn,
-                                                           unsigned index);
-  void destroyDefaultArgumentContext(DefaultArgumentInitializer *DC);
-
   //===--------------------------------------------------------------------===//
   // Diagnostics Helper functions
   //===--------------------------------------------------------------------===//

include/swift/AST/Decl.h

@@ -4275,8 +4275,13 @@ class ParamDecl : public VarDecl {
   SourceLoc ArgumentNameLoc;
   SourceLoc LetVarInOutLoc;
 
+  struct StoredDefaultArgument {
+    Expr *DefaultArg = nullptr;
+    Initializer *InitContext = nullptr;
+  };
+
   /// The default value, if any, along with whether this is varargs.
-  llvm::PointerIntPair<Expr *, 1> DefaultValueAndIsVariadic;
+  llvm::PointerIntPair<StoredDefaultArgument *, 1> DefaultValueAndIsVariadic;
 
   /// True if the type is implicitly specified in the source, but this has an
   /// apparently valid typeRepr.  This is used in accessors, which look like:
@@ -4322,13 +4327,22 @@ class ParamDecl : public VarDecl {
     defaultArgumentKind = K;
   }
 
-  void setDefaultValue(Expr *E) {
-    DefaultValueAndIsVariadic.setPointer(E);
-  }
   Expr *getDefaultValue() const {
-    return DefaultValueAndIsVariadic.getPointer();
+    if (auto stored = DefaultValueAndIsVariadic.getPointer())
+      return stored->DefaultArg;
+    return nullptr;
   }
 
+  void setDefaultValue(Expr *E);
+
+  Initializer *getDefaultArgumentInitContext() const {
+    if (auto stored = DefaultValueAndIsVariadic.getPointer())
+      return stored->InitContext;
+    return nullptr;
+  }
+
+  void setDefaultArgumentInitContext(Initializer *initContext);
+
   /// Whether or not this parameter is varargs.
   bool isVariadic() const { return DefaultValueAndIsVariadic.getInt(); }
   void setVariadic(bool value = true) {DefaultValueAndIsVariadic.setInt(value);}

include/swift/AST/Expr.h

@@ -539,11 +539,6 @@ class alignas(8) Expr {
   /// walk into the body of it (unless it is single-expression).
   void forEachChildExpr(const std::function<Expr*(Expr*)> &callback);
 
-  /// findExistingInitializerContext - Given that this expression is
-  /// an initializer that belongs in some sort of Initializer
-  /// context, look through it for any existing context object.
-  Initializer *findExistingInitializerContext();
-
   /// Determine whether this expression refers to a type by name.
   ///
   /// This distinguishes static references to types, like Int, from metatype

include/swift/AST/Initializer.h

@@ -155,11 +155,8 @@ class DefaultArgumentInitializer : public Initializer {
   /// Change the parent of this context.  This is necessary because
   /// the function signature is parsed before the function
   /// declaration/expression itself is built.
-  void changeFunction(DeclContext *parent) {
-    assert(parent->isLocalContext());
-    setParent(parent);
-  }
-
+  void changeFunction(AbstractFunctionDecl *parent);
+  
   static bool classof(const DeclContext *DC) {
     if (auto init = dyn_cast<Initializer>(DC))
       return classof(init);

include/swift/Parse/Parser.h

@@ -924,7 +924,7 @@ class Parser {
 
     /// Set the parsed context for all the initializers to the given
     /// function.
-    void setFunctionContext(DeclContext *DC);
+    void setFunctionContext(AbstractFunctionDecl *AFD);
 
     DefaultArgumentInfo(bool inTypeContext) {
       NextIndex = inTypeContext ? 1 : 0;

lib/AST/ASTContext.cpp

@@ -258,9 +258,6 @@ struct ASTContext::Implementation {
   llvm::DenseMap<Decl *, std::pair<LazyMemberLoader *, uint64_t>>
     ConformanceLoaders;
 
-  /// \brief A cached unused default-argument initializer context.
-  DefaultArgumentInitializer *UnusedDefaultArgumentContext = nullptr;
-
   /// Mapping from archetypes with lazily-resolved nested types to the
   /// archetype builder and potential archetype corresponding to that
   /// archetype.
@@ -1559,22 +1556,6 @@ void ValueDecl::setLocalDiscriminator(unsigned index) {
   getASTContext().Impl.LocalDiscriminators.insert({this, index});
 }
 
-DefaultArgumentInitializer *
-ASTContext::createDefaultArgumentContext(DeclContext *fn, unsigned index) {
-  // Check for an existing context we can re-use.
-  if (auto existing = Impl.UnusedDefaultArgumentContext) {
-    Impl.UnusedDefaultArgumentContext = nullptr;
-    existing->reset(fn, index);
-    return existing;
-  }
-
-  return new (*this) DefaultArgumentInitializer(fn, index);
-}
-void ASTContext::destroyDefaultArgumentContext(DefaultArgumentInitializer *DC) {
-  // There isn't much value in caching more than one of these.
-  Impl.UnusedDefaultArgumentContext = DC;
-}
-
 NormalProtocolConformance *
 ASTContext::getBehaviorConformance(Type conformingType,
                                    Type conformingInterfaceType,

lib/AST/Decl.cpp

@@ -3681,7 +3681,7 @@ ParamDecl::ParamDecl(ParamDecl *PD)
     ArgumentName(PD->getArgumentName()),
     ArgumentNameLoc(PD->getArgumentNameLoc()),
     LetVarInOutLoc(PD->getLetVarInOutLoc()),
-    DefaultValueAndIsVariadic(PD->DefaultValueAndIsVariadic),
+    DefaultValueAndIsVariadic(nullptr, PD->DefaultValueAndIsVariadic.getInt()),
     IsTypeLocImplicit(PD->IsTypeLocImplicit),
     defaultArgumentKind(PD->defaultArgumentKind) {
   typeLoc = PD->getTypeLoc();
@@ -3851,6 +3851,39 @@ Type ParamDecl::getVarargBaseTy(Type VarArgT) {
   return T;
 }
 
+void ParamDecl::setDefaultValue(Expr *E) {
+  if (!DefaultValueAndIsVariadic.getPointer()) {
+    if (!E) return;
+
+    DefaultValueAndIsVariadic.setPointer(
+      getASTContext().Allocate<StoredDefaultArgument>());
+  }
+
+  DefaultValueAndIsVariadic.getPointer()->DefaultArg = E;
+}
+
+void ParamDecl::setDefaultArgumentInitContext(Initializer *initContext) {
+  assert(DefaultValueAndIsVariadic.getPointer());
+  DefaultValueAndIsVariadic.getPointer()->InitContext = initContext;
+}
+
+void DefaultArgumentInitializer::changeFunction(AbstractFunctionDecl *parent) {
+  assert(parent->isLocalContext());
+  setParent(parent);
+
+  unsigned offset = getIndex();
+  for (auto list : parent->getParameterLists()) {
+    if (offset < list->size()) {
+      auto param = list->get(offset);
+      if (param->getDefaultValue())
+        param->setDefaultArgumentInitContext(this);
+      return;
+    }
+
+    offset -= list->size();
+  }
+}
+
 /// Determine whether the given Swift type is an integral type, i.e.,
 /// a type that wraps a builtin integer.
 static bool isIntegralType(Type type) {

lib/AST/Expr.cpp

@@ -567,22 +567,6 @@ void Expr::forEachChildExpr(const std::function<Expr*(Expr*)> &callback) {
   this->walk(ChildWalker(callback));
 }
 
-Initializer *Expr::findExistingInitializerContext() {
-  struct FindExistingInitializer : ASTWalker {
-    Initializer *TheInitializer = nullptr;
-    std::pair<bool,Expr*> walkToExprPre(Expr *E) override {
-      assert(!TheInitializer && "continuing to walk after finding context?");
-      if (auto closure = dyn_cast<AbstractClosureExpr>(E)) {
-        TheInitializer = cast<Initializer>(closure->getParent());
-        return { false, nullptr };
-      }
-      return { true, E };
-    }
-  } finder;
-  walk(finder);
-  return finder.TheInitializer;
-}
-
 bool Expr::isTypeReference() const {
   // If the result isn't a metatype, there's nothing else to do.
   if (!getType()->is<AnyMetatypeType>())

lib/AST/Parameter.cpp

@@ -90,6 +90,8 @@ ParameterList *ParameterList::clone(const ASTContext &C,
 
   // Remap the ParamDecls inside of the ParameterList.
   for (auto &decl : params) {
+    bool hadDefaultArgument =decl->getDefaultValue() != nullptr;
+
     decl = new (C) ParamDecl(decl);
     if (options & Implicit)
       decl->setImplicit();
@@ -101,9 +103,8 @@ ParameterList *ParameterList::clone(const ASTContext &C,
       decl->setName(C.getIdentifier("argument"));
 
     // If we're inheriting a default argument, mark it as such.
-    if (decl->isDefaultArgument() && (options & Inherited)) {
+    if (hadDefaultArgument && (options & Inherited)) {
       decl->setDefaultArgumentKind(DefaultArgumentKind::Inherited);
-      decl->setDefaultValue(nullptr);
     }
   }
 

lib/Parse/ParsePattern.cpp

@@ -47,10 +47,9 @@ static DefaultArgumentKind getDefaultArgKind(Expr *init) {
   }
 }
 
-void Parser::DefaultArgumentInfo::setFunctionContext(DeclContext *DC) {
-  assert(DC->isLocalContext());
+void Parser::DefaultArgumentInfo::setFunctionContext(AbstractFunctionDecl *AFD){
   for (auto context : ParsedContexts) {
-    context->changeFunction(DC);
+    context->changeFunction(AFD);
   }
 }
 
@@ -64,19 +63,15 @@ static ParserStatus parseDefaultArgument(Parser &P,
   // Enter a fresh default-argument context with a meaningless parent.
   // We'll change the parent to the function later after we've created
   // that declaration.
-  auto initDC =
-    P.Context.createDefaultArgumentContext(P.CurDeclContext, argIndex);
+  auto initDC = new (P.Context) DefaultArgumentInitializer(P.CurDeclContext,
+                                                           argIndex);
   Parser::ParseFunctionBody initScope(P, initDC);
 
   ParserResult<Expr> initR = P.parseExpr(diag::expected_init_value);
 
-  // Give back the default-argument context if we didn't need it.
-  if (!initScope.hasClosures()) {
-    P.Context.destroyDefaultArgumentContext(initDC);
-
-  // Otherwise, record it if we're supposed to accept default
+  // Record the default-argument context if we're supposed to accept default
   // arguments here.
-  } else if (defaultArgs) {
+  if (defaultArgs) {
     defaultArgs->ParsedContexts.push_back(initDC);
   }
 

lib/Sema/TypeCheckStmt.cpp

@@ -1206,26 +1206,13 @@ static void checkDefaultArguments(TypeChecker &tc, ParameterList *params,
   assert(dc->isLocalContext());
 
   for (auto &param : *params) {
-    unsigned curArgIndex = nextArgIndex++;
+    ++nextArgIndex;
     if (!param->getDefaultValue() || !param->hasType() ||
         param->getType()->is<ErrorType>())
       continue;
 
     Expr *e = param->getDefaultValue();
-
-    // Re-use an existing initializer context if possible.
-    auto existingContext = e->findExistingInitializerContext();
-    DefaultArgumentInitializer *initContext;
-    if (existingContext) {
-      initContext = cast<DefaultArgumentInitializer>(existingContext);
-      assert(initContext->getIndex() == curArgIndex);
-      assert(initContext->getParent() == dc);
-
-    // Otherwise, allocate one temporarily.
-    } else {
-      initContext =
-        tc.Context.createDefaultArgumentContext(dc, curArgIndex);
-    }
+    auto initContext = param->getDefaultArgumentInitContext();
 
     // Type-check the initializer, then flag that we did so.
     if (!tc.typeCheckExpression(e, initContext,
@@ -1237,12 +1224,7 @@ static void checkDefaultArguments(TypeChecker &tc, ParameterList *params,
 
     // Walk the checked initializer and contextualize any closures
     // we saw there.
-    bool hasClosures = tc.contextualizeInitializer(initContext, e);
-
-    // If we created a new context and didn't run into any autoclosures
-    // during the walk, give the context back to the ASTContext.
-    if (!hasClosures && !existingContext)
-      tc.Context.destroyDefaultArgumentContext(initContext);
+    (void)tc.contextualizeInitializer(initContext, e);
   }
 }