Revert "Implement backward-compatible closure capture behavior with parser lookup disabled"

Author: compnerd

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"
+#include "swift/AST/NameLookup.h" // for DeclVisibilityKind
 #include "swift/AST/SimpleRequest.h"
 #include "swift/Basic/Compiler.h"
 #include "swift/Basic/Debug.h"
@@ -444,6 +444,7 @@ 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
@@ -1023,10 +1024,10 @@ class AbstractPatternEntryScope : public ASTScopeImpl {
 public:
   PatternBindingDecl *const decl;
   const unsigned patternEntryIndex;
-  const bool isLocalBinding;
+  const DeclVisibilityKind vis;
 
   AbstractPatternEntryScope(PatternBindingDecl *, unsigned entryIndex,
-                            bool);
+                            DeclVisibilityKind);
   virtual ~AbstractPatternEntryScope() {}
 
   const PatternBindingEntry &getPatternEntry() const;
@@ -1043,8 +1044,8 @@ class AbstractPatternEntryScope : public ASTScopeImpl {
 class PatternEntryDeclScope final : public AbstractPatternEntryScope {
 public:
   PatternEntryDeclScope(PatternBindingDecl *pbDecl, unsigned entryIndex,
-                        bool isLocalBinding)
-      : AbstractPatternEntryScope(pbDecl, entryIndex, isLocalBinding) {}
+                        DeclVisibilityKind vis)
+      : AbstractPatternEntryScope(pbDecl, entryIndex, vis) {}
   virtual ~PatternEntryDeclScope() {}
 
 protected:
@@ -1071,8 +1072,8 @@ class PatternEntryInitializerScope final : public AbstractPatternEntryScope {
 
 public:
   PatternEntryInitializerScope(PatternBindingDecl *pbDecl, unsigned entryIndex,
-                               bool isLocalBinding)
-      : AbstractPatternEntryScope(pbDecl, entryIndex, isLocalBinding),
+                               DeclVisibilityKind vis)
+      : AbstractPatternEntryScope(pbDecl, entryIndex, vis),
         initAsWrittenWhenCreated(pbDecl->getOriginalInit(entryIndex)) {}
   virtual ~PatternEntryInitializerScope() {}
 
@@ -1657,22 +1658,10 @@ 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:
-  BraceStmtScope(BraceStmt *e,
-                 SmallVector<ValueDecl *, 2> localFuncsAndTypes,
-                 SmallVector<VarDecl *, 2> localVars)
-      : stmt(e),
-        localFuncsAndTypes(localFuncsAndTypes),
-        localVars(localVars) {}
+  BraceStmt *const stmt;
+  BraceStmtScope(BraceStmt *e) : stmt(e) {}
   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,
+  virtual bool consume(ArrayRef<ValueDecl *> values, DeclVisibilityKind vis,
                        NullablePtr<DeclContext> baseDC = nullptr) = 0;
 
   /// Look for members of a nominal type or extension scope.
@@ -613,14 +613,6 @@ 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.
@@ -644,7 +636,7 @@ class ASTScopeDeclGatherer : public AbstractASTScopeDeclConsumer {
 public:
   virtual ~ASTScopeDeclGatherer() = default;
 
-  bool consume(ArrayRef<ValueDecl *> values,
+  bool consume(ArrayRef<ValueDecl *> values, DeclVisibilityKind vis,
                NullablePtr<DeclContext> baseDC = nullptr) override;
 
   /// Eventually this functionality should move into ASTScopeLookup

lib/AST/ASTScopeCreation.cpp

@@ -667,30 +667,10 @@ 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, std::move(localFuncsAndTypes), std::move(localVars));
+        scopeCreator.ifUniqueConstructExpandAndInsert<BraceStmtScope>(p, bs);
     if (auto *s = scopeCreator.getASTContext().Stats)
       ++s->getFrontendCounters().NumBraceStmtASTScopes;
-
     return maybeBraceScope.getPtrOr(p);
   }
 
@@ -701,24 +681,24 @@ 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, isLocalBinding)
+                  insertionPoint, patternBinding, i, vis)
               .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;
   }
 
@@ -991,7 +971,7 @@ PatternEntryDeclScope::expandAScopeThatCreatesANewInsertionPoint(
         "Original inits are always after the '='");
     scopeCreator
         .constructExpandAndInsertUncheckable<PatternEntryInitializerScope>(
-            this, decl, patternEntryIndex, isLocalBinding);
+            this, decl, patternEntryIndex, vis);
   }
 
   // Add accessors for the variables in this pattern.
@@ -1002,7 +982,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 (isLocalBinding)
+  if (vis == DeclVisibilityKind::LocalVariable)
     return {this, "All code that follows is inside this scope"};
 
   return {getParent().get(), "Global and type members do not introduce scopes"};
@@ -1378,9 +1358,8 @@ ASTScopeImpl *LabeledConditionalStmtScope::createNestedConditionalClauseScopes(
 
 AbstractPatternEntryScope::AbstractPatternEntryScope(
     PatternBindingDecl *declBeingScoped, unsigned entryIndex,
-    bool isLocalBinding)
-    : decl(declBeingScoped), patternEntryIndex(entryIndex),
-      isLocalBinding(isLocalBinding) {
+    DeclVisibilityKind vis)
+    : decl(declBeingScoped), patternEntryIndex(entryIndex), vis(vis) {
   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))
+  if (consumer.consume(bindings, DeclVisibilityKind::GenericParameter))
     return true;
   return false;
 }
@@ -289,26 +289,28 @@ PatternEntryInitializerScope::getLookupParent() const {
 
 bool GenericParamScope::lookupLocalsOrMembers(DeclConsumer consumer) const {
   auto *param = paramList->getParams()[index];
-  return consumer.consume({param});
+  return consumer.consume({param}, DeclVisibilityKind::GenericParameter);
 }
 
 bool PatternEntryDeclScope::lookupLocalsOrMembers(DeclConsumer consumer) const {
-  if (!isLocalBinding)
-    return false;
-  return lookupLocalBindingsInPattern(getPattern(), consumer);
+  if (vis != DeclVisibilityKind::LocalVariable)
+    return false; // look in self type will find this later
+  return lookupLocalBindingsInPattern(getPattern(), vis, consumer);
 }
 
 bool ForEachPatternScope::lookupLocalsOrMembers(DeclConsumer consumer) const {
-  return lookupLocalBindingsInPattern(stmt->getPattern(), consumer);
+  return lookupLocalBindingsInPattern(
+      stmt->getPattern(), DeclVisibilityKind::LocalVariable, consumer);
 }
 
 bool CaseLabelItemScope::lookupLocalsOrMembers(DeclConsumer consumer) const {
-  return lookupLocalBindingsInPattern(item.getPattern(), consumer);
+  return lookupLocalBindingsInPattern(
+      item.getPattern(), DeclVisibilityKind::LocalVariable, consumer);
 }
 
 bool CaseStmtBodyScope::lookupLocalsOrMembers(DeclConsumer consumer) const {
   for (auto *var : stmt->getCaseBodyVariablesOrEmptyArray())
-    if (consumer.consume({var}))
+    if (consumer.consume({var}, DeclVisibilityKind::LocalVariable))
         return true;
 
   return false;
@@ -318,12 +320,13 @@ bool FunctionBodyScope::lookupLocalsOrMembers(
     DeclConsumer consumer) const {
   if (auto *paramList = decl->getParameters()) {
     for (auto *paramDecl : *paramList)
-      if (consumer.consume({paramDecl}))
+      if (consumer.consume({paramDecl}, DeclVisibilityKind::FunctionParameter))
         return true;
   }
 
   if (decl->getDeclContext()->isTypeContext()) {
-    return consumer.consume({decl->getImplicitSelfDecl()});
+    return consumer.consume({decl->getImplicitSelfDecl()},
+                            DeclVisibilityKind::FunctionParameter);
   }
 
   // Consider \c var t: T { (did/will/)get/set { ... t }}
@@ -332,7 +335,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}))
+      if (consumer.consume({storage}, DeclVisibilityKind::LocalVariable))
         return true;
   }
 
@@ -343,7 +346,7 @@ bool SpecializeAttributeScope::lookupLocalsOrMembers(
     DeclConsumer consumer) const {
   if (auto *params = whatWasSpecialized->getGenericParams())
     for (auto *param : params->getParams())
-      if (consumer.consume({param}))
+      if (consumer.consume({param}, DeclVisibilityKind::GenericParameter))
         return true;
   return false;
 }
@@ -353,7 +356,7 @@ bool DifferentiableAttributeScope::lookupLocalsOrMembers(
   auto visitAbstractFunctionDecl = [&](AbstractFunctionDecl *afd) {
     if (auto *params = afd->getGenericParams())
       for (auto *param : params->getParams())
-        if (consumer.consume({param}))
+        if (consumer.consume({param}, DeclVisibilityKind::GenericParameter))
           return true;
     return false;
   };
@@ -368,10 +371,20 @@ bool DifferentiableAttributeScope::lookupLocalsOrMembers(
 }
 
 bool BraceStmtScope::lookupLocalsOrMembers(DeclConsumer consumer) const {
-  if (consumer.consume(localFuncsAndTypes))
-    return true;
-
-  if (consumer.consumePossiblyNotInScope(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.
+  //
+  // 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))
     return true;
 
   if (consumer.finishLookupInBraceStmt(stmt))
@@ -387,15 +400,18 @@ bool PatternEntryInitializerScope::lookupLocalsOrMembers(
       decl->getInitContext(0));
   if (initContext) {
     if (auto *selfParam = initContext->getImplicitSelfDecl()) {
-      return consumer.consume({selfParam});
+      return consumer.consume({selfParam},
+                              DeclVisibilityKind::FunctionParameter);
     }
   }
   return false;
 }
 
 bool CaptureListScope::lookupLocalsOrMembers(DeclConsumer consumer) const {
   for (auto &e : expr->getCaptureList()) {
-    if (consumer.consume({e.Var}))
+    if (consumer.consume(
+            {e.Var},
+            DeclVisibilityKind::LocalVariable)) // or FunctionParameter??
       return true;
   }
   return false;
@@ -404,24 +420,26 @@ bool CaptureListScope::lookupLocalsOrMembers(DeclConsumer consumer) const {
 bool ClosureParametersScope::lookupLocalsOrMembers(
     DeclConsumer consumer) const {
   for (auto param : *closureExpr->getParameters())
-    if (consumer.consume({param}))
+    if (consumer.consume({param}, DeclVisibilityKind::FunctionParameter))
       return true;
   return false;
 }
 
 bool ConditionalClausePatternUseScope::lookupLocalsOrMembers(
     DeclConsumer consumer) const {
-  return lookupLocalBindingsInPattern(pattern, consumer);
+  return lookupLocalBindingsInPattern(
+      pattern, DeclVisibilityKind::LocalVariable, 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});
+      isDone = consumer.consume({var}, vis);
   });
   return isDone;
 }

lib/AST/Decl.cpp

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