[ASTGen] Better handle pattern binding entries

Generate all entries, and ensure we introduce a
PatternBindingInitializer context for non-local
cases. Also use this opportunity to cleanup
`PatternBindingDecl::create`.

Author: hamishknight

include/swift/AST/ASTBridging.h

@@ -265,6 +265,13 @@ namespace swift {
   BridgedPattern Bridged##Id##Pattern_asPattern(Bridged##Id##Pattern pattern);
 #include "swift/AST/PatternNodes.def"
 
+struct BridgedPatternBindingEntry {
+  BridgedPattern pattern;
+  BridgedSourceLoc equalLoc;
+  BridgedNullableExpr init;
+  BridgedNullablePatternBindingInitializer initContext;
+};
+
 //===----------------------------------------------------------------------===//
 // MARK: Diagnostic Engine
 //===----------------------------------------------------------------------===//
@@ -371,17 +378,32 @@ void BridgedDiagnostic_fixItReplace(BridgedDiagnostic cDiag,
 SWIFT_NAME("BridgedDiagnostic.finish(self:)")
 void BridgedDiagnostic_finish(BridgedDiagnostic cDiag);
 
+//===----------------------------------------------------------------------===//
+// MARK: DeclContexts
+//===----------------------------------------------------------------------===//
+
+SWIFT_NAME("getter:BridgedDeclContext.isLocalContext(self:)")
+bool BridgedDeclContext_isLocalContext(BridgedDeclContext cDeclContext);
+
+SWIFT_NAME("BridgedPatternBindingInitializer.create(declContext:)")
+BridgedPatternBindingInitializer
+BridgedPatternBindingInitializer_create(BridgedDeclContext cDeclContext);
+
+SWIFT_NAME("getter:BridgedPatternBindingInitializer.asDeclContext(self:)")
+BridgedDeclContext BridgedPatternBindingInitializer_asDeclContext(
+    BridgedPatternBindingInitializer cInit);
+
 //===----------------------------------------------------------------------===//
 // MARK: Decls
 //===----------------------------------------------------------------------===//
 
 SWIFT_NAME(
     "BridgedPatternBindingDecl.createParsed(_:declContext:bindingKeywordLoc:"
-    "pattern:initializer:isStatic:isLet:)")
+    "entries:isStatic:isLet:)")
 BridgedPatternBindingDecl BridgedPatternBindingDecl_createParsed(
     BridgedASTContext cContext, BridgedDeclContext cDeclContext,
-    BridgedSourceLoc cBindingKeywordLoc, BridgedPattern pattern,
-    BridgedExpr opaqueInitExpr, bool isStatic, bool isLet);
+    BridgedSourceLoc cBindingKeywordLoc, BridgedArrayRef cBindingEntries,
+    bool isStatic, bool isLet);
 
 SWIFT_NAME("BridgedParamDecl.createParsed(_:declContext:specifierLoc:firstName:"
            "firstNameLoc:secondName:secondNameLoc:type:defaultValue:)")

include/swift/AST/ASTBridgingWrappers.def

@@ -74,6 +74,7 @@ AST_BRIDGING_WRAPPER_NULLABLE(TypeRepr)
 AST_BRIDGING_WRAPPER_NULLABLE(GenericParamList)
 AST_BRIDGING_WRAPPER_NULLABLE(TrailingWhereClause)
 AST_BRIDGING_WRAPPER_NULLABLE(ParameterList)
+AST_BRIDGING_WRAPPER_NULLABLE(PatternBindingInitializer)
 AST_BRIDGING_WRAPPER_NONNULL(TypeAttributes)
 AST_BRIDGING_WRAPPER_NONNULL(CustomAttribute)
 

lib/AST/ASTBridging.cpp

@@ -20,6 +20,7 @@
 #include "swift/AST/Expr.h"
 #include "swift/AST/GenericParamList.h"
 #include "swift/AST/Identifier.h"
+#include "swift/AST/Initializer.h"
 #include "swift/AST/ParameterList.h"
 #include "swift/AST/ParseRequests.h"
 #include "swift/AST/Pattern.h"
@@ -324,36 +325,58 @@ void BridgedDiagnostic_finish(BridgedDiagnostic cDiag) {
   delete diag;
 }
 
+//===----------------------------------------------------------------------===//
+// MARK: DeclContexts
+//===----------------------------------------------------------------------===//
+
+bool BridgedDeclContext_isLocalContext(BridgedDeclContext cDeclContext) {
+  return cDeclContext.unbridged()->isLocalContext();
+}
+
+BridgedPatternBindingInitializer
+BridgedPatternBindingInitializer_create(BridgedDeclContext cDeclContext) {
+  auto *dc = cDeclContext.unbridged();
+  return new (dc->getASTContext()) PatternBindingInitializer(dc);
+}
+
+BridgedDeclContext BridgedPatternBindingInitializer_asDeclContext(
+    BridgedPatternBindingInitializer cInit) {
+  return cInit.unbridged();
+}
+
 //===----------------------------------------------------------------------===//
 // MARK: Decls
 //===----------------------------------------------------------------------===//
 
 BridgedPatternBindingDecl BridgedPatternBindingDecl_createParsed(
     BridgedASTContext cContext, BridgedDeclContext cDeclContext,
-    BridgedSourceLoc cBindingKeywordLoc, BridgedPattern cPattern,
-    BridgedExpr initExpr, bool isStatic, bool isLet) {
+    BridgedSourceLoc cBindingKeywordLoc, BridgedArrayRef cBindingEntries,
+    bool isStatic, bool isLet) {
   ASTContext &context = cContext.unbridged();
   DeclContext *declContext = cDeclContext.unbridged();
 
-  Pattern *pattern = cPattern.unbridged();
+  auto introducer = isLet ? VarDecl::Introducer::Let : VarDecl::Introducer::Var;
 
-  VarDecl::Introducer introducer =
-      isLet ? VarDecl::Introducer::Let : VarDecl::Introducer::Var;
+  SmallVector<PatternBindingEntry, 4> entries;
+  for (auto &entry : cBindingEntries.unbridged<BridgedPatternBindingEntry>()) {
+    auto *pattern = entry.pattern.unbridged();
+
+    // Configure all vars.
+    pattern->forEachVariable([&](VarDecl *VD) {
+      VD->setStatic(isStatic);
+      VD->setIntroducer(introducer);
+    });
+
+    entries.emplace_back(pattern, entry.equalLoc.unbridged(),
+                         entry.init.unbridged(), entry.initContext.unbridged());
+  }
 
-  // Configure all vars.
-  pattern->forEachVariable([&](VarDecl *VD) {
-    VD->setStatic(isStatic);
-    VD->setIntroducer(introducer);
-  });
-
-  return PatternBindingDecl::create(context,
-                                    /*StaticLoc=*/SourceLoc(),
-                                    // FIXME: 'class' spelling kind.
-                                    isStatic ? StaticSpellingKind::KeywordStatic
-                                             : StaticSpellingKind::None,
-                                    cBindingKeywordLoc.unbridged(), pattern,
-                                    /*EqualLoc=*/SourceLoc(), // FIXME
-                                    initExpr.unbridged(), declContext);
+  return PatternBindingDecl::create(
+      context,
+      /*StaticLoc=*/SourceLoc(),
+      // FIXME: 'class' spelling kind.
+      isStatic ? StaticSpellingKind::KeywordStatic : StaticSpellingKind::None,
+      cBindingKeywordLoc.unbridged(), entries, declContext);
 }
 
 BridgedParamDecl BridgedParamDecl_createParsed(

lib/AST/Decl.cpp

@@ -1919,26 +1919,26 @@ PatternBindingDecl::create(ASTContext &Ctx, SourceLoc StaticLoc,
                                                       /*ClangNode*/false);
   auto PBD = ::new (D) PatternBindingDecl(StaticLoc, StaticSpelling, VarLoc,
                                           PatternList.size(), Parent);
-
   // Set up the patterns.
-  auto entries = PBD->getMutablePatternList();
-  unsigned elt = 0U-1;
-  for (auto pe : PatternList) {
-    ++elt;
-    auto &newEntry = entries[elt];
-    newEntry = pe; // This should take care of initializer with flags
-    DeclContext *initContext = pe.getInitContext();
+  std::uninitialized_copy(PatternList.begin(), PatternList.end(),
+                          PBD->getTrailingObjects<PatternBindingEntry>());
+
+  for (auto idx : range(PBD->getNumPatternEntries())) {
+    auto *initContext =
+        cast_or_null<PatternBindingInitializer>(PBD->getInitContext(idx));
 
     // FIXME: We ought to reconsider this since it won't recontextualize any
     // closures/decls present in the initialization expr. This currently should
     // only affect implicit code though.
-    if (!initContext && !Parent->isLocalContext()) {
-      auto pbi = new (Ctx) PatternBindingInitializer(Parent);
-      pbi->setBinding(PBD, elt);
-      initContext = pbi;
-    }
+    if (!initContext && !Parent->isLocalContext())
+      initContext = new (Ctx) PatternBindingInitializer(Parent);
+
+    if (initContext)
+      initContext->setBinding(PBD, idx);
 
-    PBD->setPattern(elt, pe.getPattern(), initContext);
+    // We need to call setPattern to ensure the VarDecls in the pattern have
+    // the PatternBindingDecl set as their parent, and to setup the context.
+    PBD->setPattern(idx, PBD->getPattern(idx), initContext);
   }
   return PBD;
 }

lib/ASTGen/Sources/ASTGen/ASTGen.swift

@@ -212,11 +212,13 @@ extension ASTGenVisitor {
 extension ASTGenVisitor {
   /// Replaces the current declaration context with `declContext` for the duration of its execution, and calls `body`.
   @inline(__always)
-  func withDeclContext(_ declContext: BridgedDeclContext, _ body: () -> Void) {
+  func withDeclContext<T>(_ declContext: BridgedDeclContext, _ body: () -> T) -> T {
     let oldDeclContext = self.declContext
     self.declContext = declContext
-    body()
-    self.declContext = oldDeclContext
+    defer {
+      self.declContext = oldDeclContext
+    }
+    return body()
   }
 }
 
@@ -240,10 +242,6 @@ extension ASTGenVisitor {
     generate(decl: node.decl)
   }
 
-  func generate(initializerClause node: InitializerClauseSyntax) -> BridgedExpr {
-    generate(expr: node.value)
-  }
-
   func generate(conditionElement node: ConditionElementSyntax) -> ASTNode {
     // FIXME: returning ASTNode is wrong, non-expression conditions are not ASTNode.
     switch node.condition {

lib/ASTGen/Sources/ASTGen/Bridge.swift

@@ -33,6 +33,7 @@ extension BridgedNullablePattern: BridgedNullable {}
 extension BridgedNullableGenericParamList: BridgedNullable {}
 extension BridgedNullableTrailingWhereClause: BridgedNullable {}
 extension BridgedNullableParameterList: BridgedNullable {}
+extension BridgedNullablePatternBindingInitializer: BridgedNullable {}
 
 /// Protocol that declares that there's a "Nullable" variation of the type.
 ///
@@ -66,6 +67,9 @@ extension BridgedTrailingWhereClause: BridgedHasNullable {
 extension BridgedParameterList: BridgedHasNullable {
   typealias Nullable = BridgedNullableParameterList
 }
+extension BridgedPatternBindingInitializer: BridgedHasNullable {
+  typealias Nullable = BridgedNullablePatternBindingInitializer
+}
 
 public extension BridgedSourceLoc {
   /// Form a source location at the given absolute position in `buffer`.

lib/ASTGen/Sources/ASTGen/Decls.swift

@@ -288,19 +288,40 @@ extension ASTGenVisitor {
 // MARK: - AbstractStorageDecl
 
 extension ASTGenVisitor {
-  func generate(variableDecl node: VariableDeclSyntax) -> BridgedPatternBindingDecl {
-    let pattern = generate(pattern: node.bindings.first!.pattern)
-    let initializer = generate(initializerClause: node.bindings.first!.initializer!)
+  func generate(patternBinding binding: PatternBindingSyntax) -> BridgedPatternBindingEntry {
+    let pattern = generate(pattern: binding.pattern)
+    let equalLoc = generateSourceLoc(binding.initializer?.equal)
+
+    var initExpr: BridgedExpr?
+    var initContext: BridgedPatternBindingInitializer?
+    if let initializer = binding.initializer {
+      // Create a PatternBindingInitializer if we're not in a local context (this
+      // ensures that property initializers are correctly treated as being in a
+      // local context).
+      if !self.declContext.isLocalContext {
+        initContext = .create(declContext: self.declContext)
+      }
+      initExpr = withDeclContext(initContext?.asDeclContext ?? self.declContext) {
+        generate(expr: initializer.value)
+      }
+    }
+    return BridgedPatternBindingEntry(
+      pattern: pattern,
+      equalLoc: equalLoc,
+      init: initExpr.asNullable,
+      initContext: initContext.asNullable
+    )
+  }
 
+  func generate(variableDecl node: VariableDeclSyntax) -> BridgedPatternBindingDecl {
     let isStatic = false  // TODO: compute this
     let isLet = node.bindingSpecifier.keywordKind == .let
 
     return .createParsed(
       self.ctx,
       declContext: self.declContext,
       bindingKeywordLoc: self.generateSourceLoc(node.bindingSpecifier),
-      pattern: pattern,
-      initializer: initializer,
+      entries: node.bindings.lazy.map(self.generate).bridgedArray(in: self),
       isStatic: isStatic,
       isLet: isLet
     )

lib/Parse/ParseDecl.cpp

@@ -8364,12 +8364,6 @@ Parser::parseDeclVar(ParseDeclOptions Flags,
     auto *PBD = PatternBindingDecl::create(Context, StaticLoc, StaticSpelling,
                                            VarLoc, PBDEntries, BaseContext);
 
-    // Wire up any initializer contexts we needed.
-    for (unsigned i : indices(PBDEntries)) {
-      if (auto initContext = PBD->getInitContext(i))
-        cast<PatternBindingInitializer>(initContext)->setBinding(PBD, i);
-    }
-
     // If we're setting up a TopLevelCodeDecl, configure it by setting up the
     // body that holds PBD and we're done.  The TopLevelCodeDecl is already set
     // up in Decls to be returned to caller.

test/ASTGen/verify-parse.swift

@@ -98,6 +98,10 @@ func test9(_ value: Any) { }
 
 func test10<T>(t: T) where T: Proto1 {}
 
+func test11() {
+  var binding1 = 0, binding2 = ""
+}
+
 protocol Proto1 {}
 protocol Proto2 {}