Merge pull request swiftlang#34135 from slavapestov/simulate-parser-lookup

Implement backward-compatible closure capture behavior with parser lookup disabled

Author: slavapestov

include/swift/AST/ASTScope.h

@@ -29,7 +29,7 @@
 #define SWIFT_AST_AST_SCOPE_H
 
 #include "swift/AST/ASTNode.h"
-#include "swift/AST/NameLookup.h" // for DeclVisibilityKind
+#include "swift/AST/NameLookup.h"
 #include "swift/AST/SimpleRequest.h"
 #include "swift/Basic/Compiler.h"
 #include "swift/Basic/Debug.h"
@@ -444,7 +444,6 @@ class ASTScopeImpl {
   // It is not an instance variable or inherited type.
 
   static bool lookupLocalBindingsInPattern(const Pattern *p,
-                                           DeclVisibilityKind vis,
                                            DeclConsumer consumer);
 
   /// When lookup must stop before the outermost scope, return the scope to stop
@@ -1024,10 +1023,10 @@ class AbstractPatternEntryScope : public ASTScopeImpl {
 public:
   PatternBindingDecl *const decl;
   const unsigned patternEntryIndex;
-  const DeclVisibilityKind vis;
+  const bool isLocalBinding;
 
   AbstractPatternEntryScope(PatternBindingDecl *, unsigned entryIndex,
-                            DeclVisibilityKind);
+                            bool);
   virtual ~AbstractPatternEntryScope() {}
 
   const PatternBindingEntry &getPatternEntry() const;
@@ -1044,8 +1043,8 @@ class AbstractPatternEntryScope : public ASTScopeImpl {
 class PatternEntryDeclScope final : public AbstractPatternEntryScope {
 public:
   PatternEntryDeclScope(PatternBindingDecl *pbDecl, unsigned entryIndex,
-                        DeclVisibilityKind vis)
-      : AbstractPatternEntryScope(pbDecl, entryIndex, vis) {}
+                        bool isLocalBinding)
+      : AbstractPatternEntryScope(pbDecl, entryIndex, isLocalBinding) {}
   virtual ~PatternEntryDeclScope() {}
 
 protected:
@@ -1072,8 +1071,8 @@ class PatternEntryInitializerScope final : public AbstractPatternEntryScope {
 
 public:
   PatternEntryInitializerScope(PatternBindingDecl *pbDecl, unsigned entryIndex,
-                               DeclVisibilityKind vis)
-      : AbstractPatternEntryScope(pbDecl, entryIndex, vis),
+                               bool isLocalBinding)
+      : AbstractPatternEntryScope(pbDecl, entryIndex, isLocalBinding),
         initAsWrittenWhenCreated(pbDecl->getOriginalInit(entryIndex)) {}
   virtual ~PatternEntryInitializerScope() {}
 
@@ -1658,10 +1657,22 @@ class CaseStmtBodyScope final : public ASTScopeImpl {
 };
 
 class BraceStmtScope final : public AbstractStmtScope {
+  BraceStmt *const stmt;
+
+  /// Declarations which are in scope from the beginning of the statement.
+  SmallVector<ValueDecl *, 2> localFuncsAndTypes;
+
+  /// Declarations that are normally in scope only after their
+  /// definition.
+  SmallVector<VarDecl *, 2> localVars;
 
 public:
-  BraceStmt *const stmt;
-  BraceStmtScope(BraceStmt *e) : stmt(e) {}
+  BraceStmtScope(BraceStmt *e,
+                 SmallVector<ValueDecl *, 2> localFuncsAndTypes,
+                 SmallVector<VarDecl *, 2> localVars)
+      : stmt(e),
+        localFuncsAndTypes(localFuncsAndTypes),
+        localVars(localVars) {}
   virtual ~BraceStmtScope() {}
 
 protected:

include/swift/AST/NameLookup.h

@@ -603,7 +603,7 @@ class AbstractASTScopeDeclConsumer {
   /// of type -vs- instance lookup results.
   ///
   /// \return true if the lookup should be stopped at this point.
-  virtual bool consume(ArrayRef<ValueDecl *> values, DeclVisibilityKind vis,
+  virtual bool consume(ArrayRef<ValueDecl *> values,
                        NullablePtr<DeclContext> baseDC = nullptr) = 0;
 
   /// Look for members of a nominal type or extension scope.
@@ -613,6 +613,14 @@ class AbstractASTScopeDeclConsumer {
   lookInMembers(DeclContext *const scopeDC,
                 NominalTypeDecl *const nominal) = 0;
 
+  /// Called for local VarDecls that might not yet be in scope.
+  ///
+  /// Note that the set of VarDecls visited here are going to be a
+  /// superset of those visited in consume().
+  virtual bool consumePossiblyNotInScope(ArrayRef<VarDecl *> values) {
+    return false;
+  }
+
   /// Called right before looking at the parent scope of a BraceStmt.
   ///
   /// \return true if the lookup should be stopped at this point.
@@ -636,7 +644,7 @@ class ASTScopeDeclGatherer : public AbstractASTScopeDeclConsumer {
 public:
   virtual ~ASTScopeDeclGatherer() = default;
 
-  bool consume(ArrayRef<ValueDecl *> values, DeclVisibilityKind vis,
+  bool consume(ArrayRef<ValueDecl *> values,
                NullablePtr<DeclContext> baseDC = nullptr) override;
 
   /// Eventually this functionality should move into ASTScopeLookup

lib/AST/ASTScopeCreation.cpp

@@ -667,10 +667,30 @@ class NodeAdder
 
   NullablePtr<ASTScopeImpl> visitBraceStmt(BraceStmt *bs, ASTScopeImpl *p,
                                            ScopeCreator &scopeCreator) {
+    SmallVector<ValueDecl *, 2> localFuncsAndTypes;
+    SmallVector<VarDecl *, 2> localVars;
+
+    // All types and functions are visible anywhere within a brace statement
+    // scope. When ordering matters (i.e. var decl) we will have split the brace
+    // statement into nested scopes.
+    for (auto braceElement : bs->getElements()) {
+      if (auto localBinding = braceElement.dyn_cast<Decl *>()) {
+        if (auto *vd = dyn_cast<ValueDecl>(localBinding)) {
+          if (isa<FuncDecl>(vd)  || isa<TypeDecl>(vd)) {
+            localFuncsAndTypes.push_back(vd);
+          } else if (auto *var = dyn_cast<VarDecl>(localBinding)) {
+            localVars.push_back(var);
+          }
+        }
+      }
+    }
+
     auto maybeBraceScope =
-        scopeCreator.ifUniqueConstructExpandAndInsert<BraceStmtScope>(p, bs);
+        scopeCreator.ifUniqueConstructExpandAndInsert<BraceStmtScope>(
+            p, bs, std::move(localFuncsAndTypes), std::move(localVars));
     if (auto *s = scopeCreator.getASTContext().Stats)
       ++s->getFrontendCounters().NumBraceStmtASTScopes;
+
     return maybeBraceScope.getPtrOr(p);
   }
 
@@ -681,23 +701,23 @@ class NodeAdder
     if (auto *var = patternBinding->getSingleVar())
       scopeCreator.addChildrenForKnownAttributes(var, parentScope);
 
-    const bool isLocalBinding = patternBinding->getDeclContext()->isLocalContext();
-
-    const DeclVisibilityKind vis =
-        isLocalBinding ? DeclVisibilityKind::LocalVariable
-                       : DeclVisibilityKind::MemberOfCurrentNominal;
     auto *insertionPoint = parentScope;
     for (auto i : range(patternBinding->getNumPatternEntries())) {
+      bool isLocalBinding = false;
+      if (auto *varDecl = patternBinding->getAnchoringVarDecl(i)) {
+        isLocalBinding = varDecl->getDeclContext()->isLocalContext();
+      }
+
       insertionPoint =
           scopeCreator
               .ifUniqueConstructExpandAndInsert<PatternEntryDeclScope>(
-                  insertionPoint, patternBinding, i, vis)
+                  insertionPoint, patternBinding, i, isLocalBinding)
               .getPtrOr(insertionPoint);
-    }
 
-    ASTScopeAssert(isLocalBinding || insertionPoint == parentScope,
-                   "Bindings at the top-level or members of types should "
-                   "not change the insertion point");
+      ASTScopeAssert(isLocalBinding || insertionPoint == parentScope,
+                     "Bindings at the top-level or members of types should "
+                     "not change the insertion point");
+    }
 
     return insertionPoint;
   }
@@ -971,7 +991,7 @@ PatternEntryDeclScope::expandAScopeThatCreatesANewInsertionPoint(
         "Original inits are always after the '='");
     scopeCreator
         .constructExpandAndInsertUncheckable<PatternEntryInitializerScope>(
-            this, decl, patternEntryIndex, vis);
+            this, decl, patternEntryIndex, isLocalBinding);
   }
 
   // Add accessors for the variables in this pattern.
@@ -982,7 +1002,7 @@ PatternEntryDeclScope::expandAScopeThatCreatesANewInsertionPoint(
   // In local context, the PatternEntryDeclScope becomes the insertion point, so
   // that all any bindings introduecd by the pattern are in scope for subsequent
   // lookups.
-  if (vis == DeclVisibilityKind::LocalVariable)
+  if (isLocalBinding)
     return {this, "All code that follows is inside this scope"};
 
   return {getParent().get(), "Global and type members do not introduce scopes"};
@@ -1358,8 +1378,9 @@ ASTScopeImpl *LabeledConditionalStmtScope::createNestedConditionalClauseScopes(
 
 AbstractPatternEntryScope::AbstractPatternEntryScope(
     PatternBindingDecl *declBeingScoped, unsigned entryIndex,
-    DeclVisibilityKind vis)
-    : decl(declBeingScoped), patternEntryIndex(entryIndex), vis(vis) {
+    bool isLocalBinding)
+    : decl(declBeingScoped), patternEntryIndex(entryIndex),
+      isLocalBinding(isLocalBinding) {
   ASTScopeAssert(entryIndex < declBeingScoped->getPatternList().size(),
                  "out of bounds");
 }

lib/AST/ASTScopeLookup.cpp

@@ -222,7 +222,7 @@ bool ASTScopeImpl::lookInGenericParametersOf(
   SmallVector<ValueDecl *, 32> bindings;
   for (auto *param : paramList.get()->getParams())
     bindings.push_back(param);
-  if (consumer.consume(bindings, DeclVisibilityKind::GenericParameter))
+  if (consumer.consume(bindings))
     return true;
   return false;
 }
@@ -289,28 +289,26 @@ PatternEntryInitializerScope::getLookupParent() const {
 
 bool GenericParamScope::lookupLocalsOrMembers(DeclConsumer consumer) const {
   auto *param = paramList->getParams()[index];
-  return consumer.consume({param}, DeclVisibilityKind::GenericParameter);
+  return consumer.consume({param});
 }
 
 bool PatternEntryDeclScope::lookupLocalsOrMembers(DeclConsumer consumer) const {
-  if (vis != DeclVisibilityKind::LocalVariable)
-    return false; // look in self type will find this later
-  return lookupLocalBindingsInPattern(getPattern(), vis, consumer);
+  if (!isLocalBinding)
+    return false;
+  return lookupLocalBindingsInPattern(getPattern(), consumer);
 }
 
 bool ForEachPatternScope::lookupLocalsOrMembers(DeclConsumer consumer) const {
-  return lookupLocalBindingsInPattern(
-      stmt->getPattern(), DeclVisibilityKind::LocalVariable, consumer);
+  return lookupLocalBindingsInPattern(stmt->getPattern(), consumer);
 }
 
 bool CaseLabelItemScope::lookupLocalsOrMembers(DeclConsumer consumer) const {
-  return lookupLocalBindingsInPattern(
-      item.getPattern(), DeclVisibilityKind::LocalVariable, consumer);
+  return lookupLocalBindingsInPattern(item.getPattern(), consumer);
 }
 
 bool CaseStmtBodyScope::lookupLocalsOrMembers(DeclConsumer consumer) const {
   for (auto *var : stmt->getCaseBodyVariablesOrEmptyArray())
-    if (consumer.consume({var}, DeclVisibilityKind::LocalVariable))
+    if (consumer.consume({var}))
         return true;
 
   return false;
@@ -320,13 +318,12 @@ bool FunctionBodyScope::lookupLocalsOrMembers(
     DeclConsumer consumer) const {
   if (auto *paramList = decl->getParameters()) {
     for (auto *paramDecl : *paramList)
-      if (consumer.consume({paramDecl}, DeclVisibilityKind::FunctionParameter))
+      if (consumer.consume({paramDecl}))
         return true;
   }
 
   if (decl->getDeclContext()->isTypeContext()) {
-    return consumer.consume({decl->getImplicitSelfDecl()},
-                            DeclVisibilityKind::FunctionParameter);
+    return consumer.consume({decl->getImplicitSelfDecl()});
   }
 
   // Consider \c var t: T { (did/will/)get/set { ... t }}
@@ -335,7 +332,7 @@ bool FunctionBodyScope::lookupLocalsOrMembers(
   // then t needs to be found as a local binding:
   if (auto *accessor = dyn_cast<AccessorDecl>(decl)) {
     if (auto *storage = accessor->getStorage())
-      if (consumer.consume({storage}, DeclVisibilityKind::LocalVariable))
+      if (consumer.consume({storage}))
         return true;
   }
 
@@ -346,7 +343,7 @@ bool SpecializeAttributeScope::lookupLocalsOrMembers(
     DeclConsumer consumer) const {
   if (auto *params = whatWasSpecialized->getGenericParams())
     for (auto *param : params->getParams())
-      if (consumer.consume({param}, DeclVisibilityKind::GenericParameter))
+      if (consumer.consume({param}))
         return true;
   return false;
 }
@@ -356,7 +353,7 @@ bool DifferentiableAttributeScope::lookupLocalsOrMembers(
   auto visitAbstractFunctionDecl = [&](AbstractFunctionDecl *afd) {
     if (auto *params = afd->getGenericParams())
       for (auto *param : params->getParams())
-        if (consumer.consume({param}, DeclVisibilityKind::GenericParameter))
+        if (consumer.consume({param}))
           return true;
     return false;
   };
@@ -371,20 +368,10 @@ bool DifferentiableAttributeScope::lookupLocalsOrMembers(
 }
 
 bool BraceStmtScope::lookupLocalsOrMembers(DeclConsumer consumer) const {
-  // All types and functions are visible anywhere within a brace statement
-  // scope. When ordering matters (i.e. var decl) we will have split the brace
-  // statement into nested scopes.
-  //
-  // Don't stop at the first one, there may be local funcs with same base name
-  // and want them all.
-  SmallVector<ValueDecl *, 32> localBindings;
-  for (auto braceElement : stmt->getElements()) {
-    if (auto localBinding = braceElement.dyn_cast<Decl *>()) {
-      if (auto *vd = dyn_cast<ValueDecl>(localBinding))
-        localBindings.push_back(vd);
-    }
-  }
-  if (consumer.consume(localBindings, DeclVisibilityKind::LocalVariable))
+  if (consumer.consume(localFuncsAndTypes))
+    return true;
+
+  if (consumer.consumePossiblyNotInScope(localVars))
     return true;
 
   if (consumer.finishLookupInBraceStmt(stmt))
@@ -400,18 +387,15 @@ bool PatternEntryInitializerScope::lookupLocalsOrMembers(
       decl->getInitContext(0));
   if (initContext) {
     if (auto *selfParam = initContext->getImplicitSelfDecl()) {
-      return consumer.consume({selfParam},
-                              DeclVisibilityKind::FunctionParameter);
+      return consumer.consume({selfParam});
     }
   }
   return false;
 }
 
 bool CaptureListScope::lookupLocalsOrMembers(DeclConsumer consumer) const {
   for (auto &e : expr->getCaptureList()) {
-    if (consumer.consume(
-            {e.Var},
-            DeclVisibilityKind::LocalVariable)) // or FunctionParameter??
+    if (consumer.consume({e.Var}))
       return true;
   }
   return false;
@@ -420,26 +404,24 @@ bool CaptureListScope::lookupLocalsOrMembers(DeclConsumer consumer) const {
 bool ClosureParametersScope::lookupLocalsOrMembers(
     DeclConsumer consumer) const {
   for (auto param : *closureExpr->getParameters())
-    if (consumer.consume({param}, DeclVisibilityKind::FunctionParameter))
+    if (consumer.consume({param}))
       return true;
   return false;
 }
 
 bool ConditionalClausePatternUseScope::lookupLocalsOrMembers(
     DeclConsumer consumer) const {
-  return lookupLocalBindingsInPattern(
-      pattern, DeclVisibilityKind::LocalVariable, consumer);
+  return lookupLocalBindingsInPattern(pattern, consumer);
 }
 
 bool ASTScopeImpl::lookupLocalBindingsInPattern(const Pattern *p,
-                                                DeclVisibilityKind vis,
                                                 DeclConsumer consumer) {
   if (!p)
     return false;
   bool isDone = false;
   p->forEachVariable([&](VarDecl *var) {
     if (!isDone)
-      isDone = consumer.consume({var}, vis);
+      isDone = consumer.consume({var});
   });
   return isDone;
 }

lib/AST/Decl.cpp

@@ -1447,7 +1447,8 @@ void PatternBindingEntry::setInit(Expr *E) {
 VarDecl *PatternBindingEntry::getAnchoringVarDecl() const {
   SmallVector<VarDecl *, 8> variables;
   getPattern()->collectVariables(variables);
-  assert(!variables.empty());
+  if (variables.empty())
+    return nullptr;
   return variables[0];
 }