Merge pull request swiftlang#34038 from slavapestov/local-pbd-scopes

ASTScope: Take start location into account when modeling local pattern bindings

Author: slavapestov

include/swift/AST/ASTScope.h

@@ -344,9 +344,6 @@ class ASTScopeImpl {
   virtual NullablePtr<ASTScopeImpl> insertionPointForDeferredExpansion();
   virtual SourceRange sourceRangeForDeferredExpansion() const;
 
-public:
-  bool isATypeDeclScope() const;
-
 private:
   virtual ScopeCreator &getScopeCreator();
 
@@ -435,6 +432,7 @@ class ASTScopeImpl {
     return p->getParent().isNonNull() ? p : nullptr;
   }
 
+public:
   /// The tree is organized by source location and for most nodes this is also
   /// what obtaines for scoping. However, guards are different. The scope after
   /// the guard else must hop into the innermoset scope of the guard condition.
@@ -1079,6 +1077,7 @@ class PatternEntryDeclScope final : public AbstractPatternEntryScope {
 
 protected:
   bool lookupLocalsOrMembers(DeclConsumer) const override;
+  bool isLabeledStmtLookupTerminator() const override;
 };
 
 class PatternEntryInitializerScope final : public AbstractPatternEntryScope {
@@ -1093,6 +1092,7 @@ class PatternEntryInitializerScope final : public AbstractPatternEntryScope {
 
 protected:
   ASTScopeImpl *expandSpecifically(ScopeCreator &scopeCreator) override;
+  NullablePtr<const ASTScopeImpl> getLookupParent() const override;
 
 private:
   void expandAScopeThatDoesNotCreateANewInsertionPoint(ScopeCreator &);

lib/AST/ASTScopeCreation.cpp

@@ -38,11 +38,6 @@
 using namespace swift;
 using namespace ast_scope;
 
-/// If true, nest scopes so a variable is out of scope before its declaration
-/// Does not handle capture rules for local functions properly.
-/// If false don't push uses down into subscopes after decls.
-static const bool handleUseBeforeDef = false;
-
 #pragma mark source range utilities
 static bool rangeableIsIgnored(const Decl *d) { return d->isImplicit(); }
 static bool rangeableIsIgnored(const Expr *d) {
@@ -746,11 +741,11 @@ class NodeAdder
     if (auto *var = patternBinding->getSingleVar())
       scopeCreator.addChildrenForKnownAttributes(var, parentScope);
 
-    const bool isInTypeDecl = parentScope->isATypeDeclScope();
+    const bool isLocalBinding = patternBinding->getDeclContext()->isLocalContext();
 
     const DeclVisibilityKind vis =
-        isInTypeDecl ? DeclVisibilityKind::MemberOfCurrentNominal
-                     : DeclVisibilityKind::LocalVariable;
+        isLocalBinding ? DeclVisibilityKind::LocalVariable
+                       : DeclVisibilityKind::MemberOfCurrentNominal;
     auto *insertionPoint = parentScope;
     for (auto i : range(patternBinding->getNumPatternEntries())) {
       insertionPoint =
@@ -759,9 +754,12 @@ class NodeAdder
                   insertionPoint, patternBinding, i, vis)
               .getPtrOr(insertionPoint);
     }
-    // If in a type decl, the type search will find these,
-    // but if in a brace stmt, must continue under the last binding.
-    return isInTypeDecl ? parentScope : insertionPoint;
+
+    ASTScopeAssert(isLocalBinding || insertionPoint == parentScope,
+                   "Bindings at the top-level or members of types should "
+                   "not change the insertion point");
+
+    return insertionPoint;
   }
 
   NullablePtr<ASTScopeImpl> visitEnumElementDecl(EnumElementDecl *eed,
@@ -1041,11 +1039,13 @@ PatternEntryDeclScope::expandAScopeThatCreatesANewInsertionPoint(
     scopeCreator.addChildrenForAllLocalizableAccessorsInSourceOrder(var, this);
   });
 
-  ASTScopeAssert(!handleUseBeforeDef,
-                 "next line is wrong otherwise; would need a use scope");
+  // 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)
+    return {this, "All code that follows is inside this scope"};
 
-  return {getParent().get(), "When not handling use-before-def, succeeding "
-                             "code just goes in the same scope as this one"};
+  return {getParent().get(), "Global and type members do not introduce scopes"};
 }
 
 void
@@ -1424,11 +1424,6 @@ AbstractPatternEntryScope::AbstractPatternEntryScope(
                  "out of bounds");
 }
 
-bool ASTScopeImpl::isATypeDeclScope() const {
-  Decl *const pd = getDeclIfAny().getPtrOrNull();
-  return pd && (isa<NominalTypeDecl>(pd) || isa<ExtensionDecl>(pd));
-}
-
 #pragma mark new operators
 void *ASTScopeImpl::operator new(size_t bytes, const ASTContext &ctx,
                                  unsigned alignment) {

lib/AST/ASTScopeLookup.cpp

@@ -271,6 +271,24 @@ bool GenericTypeOrExtensionScope::areMembersVisibleFromWhereClause() const {
   return isa<ProtocolDecl>(decl) || isa<ExtensionDecl>(decl);
 }
 
+#pragma mark custom lookup parent behavior
+
+NullablePtr<const ASTScopeImpl>
+PatternEntryInitializerScope::getLookupParent() const {
+  auto parent = getParent().get();
+  assert(parent->getClassName() == "PatternEntryDeclScope");
+
+  // Lookups from inside a pattern binding initializer skip the parent
+  // scope that introduces bindings bound by the pattern, since we
+  // want this to work:
+  //
+  // func f(x: Int) {
+  //   let x = x
+  //   print(x)
+  // }
+  return parent->getLookupParent();
+}
+
 #pragma mark looking in locals or members - locals
 
 bool GenericParamScope::lookupLocalsOrMembers(DeclConsumer consumer) const {
@@ -510,6 +528,10 @@ bool CaseStmtBodyScope::isLabeledStmtLookupTerminator() const {
   return false;
 }
 
+bool PatternEntryDeclScope::isLabeledStmtLookupTerminator() const {
+  return false;
+}
+
 llvm::SmallVector<LabeledStmt *, 4>
 ASTScopeImpl::lookupLabeledStmts(SourceFile *sourceFile, SourceLoc loc) {
   // Find the innermost scope from which to start our search.

test/NameLookup/scope_map_top_level.swift

@@ -31,8 +31,8 @@ var i: Int = b.my_identity()
 // CHECK-EXPANDED-NEXT:   `-NominalTypeBodyScope {{.*}}, [4:11 - 4:13]
 // CHECK-EXPANDED-NEXT: `-TopLevelCodeScope {{.*}}, [6:1 - 21:28]
 // CHECK-EXPANDED-NEXT:   `-BraceStmtScope {{.*}}, [6:1 - 21:28]
-// CHECK-EXPANDED-NEXT:     |-PatternEntryDeclScope {{.*}}, [6:5 - 6:15] entry 0 'a'
-// CHECK-EXPANDED-NEXT:       `-PatternEntryInitializerScope {{.*}}, [6:15 - 6:15] entry 0 'a'
+// CHECK-EXPANDED-NEXT:     `-PatternEntryDeclScope {{.*}}, [6:5 - 21:28] entry 0 'a'
+// CHECK-EXPANDED-NEXT:       |-PatternEntryInitializerScope {{.*}}, [6:15 - 6:15] entry 0 'a'
 // CHECK-EXPANDED-NEXT:     `-TopLevelCodeScope {{.*}}, [8:1 - 21:28]
 // CHECK-EXPANDED-NEXT:       `-BraceStmtScope {{.*}}, [8:1 - 21:28]
 // CHECK-EXPANDED-NEXT:         `-GuardStmtScope {{.*}}, [8:1 - 21:28]
@@ -46,9 +46,9 @@ var i: Int = b.my_identity()
 // CHECK-EXPANDED-NEXT:                 `-BraceStmtScope {{.*}}, [11:18 - 11:19]
 // CHECK-EXPANDED-NEXT:             `-TopLevelCodeScope {{.*}}, [13:1 - 21:28]
 // CHECK-EXPANDED-NEXT:               `-BraceStmtScope {{.*}}, [13:1 - 21:28]
-// CHECK-EXPANDED-NEXT:                 |-PatternEntryDeclScope {{.*}}, [13:5 - 13:9] entry 0 'c'
-// CHECK-EXPANDED-NEXT:                   `-PatternEntryInitializerScope {{.*}}, [13:9 - 13:9] entry 0 'c'
-// CHECK-EXPANDED-NEXT:                 |-TypeAliasDeclScope {{.*}}, [15:1 - 15:15]
+// CHECK-EXPANDED-NEXT:                 `-PatternEntryDeclScope {{.*}}, [13:5 - 21:28] entry 0 'c'
+// CHECK-EXPANDED-NEXT:                   |-PatternEntryInitializerScope {{.*}}, [13:9 - 13:9] entry 0 'c'
+// CHECK-EXPANDED-NEXT:                   |-TypeAliasDeclScope {{.*}}, [15:1 - 15:15]
 // CHECK-EXPANDED-NEXT:                 |-ExtensionDeclScope {{.*}}, [17:14 - 19:1]
 // CHECK-EXPANDED-NEXT:                   `-ExtensionBodyScope {{.*}}, [17:15 - 19:1]
 // CHECK-EXPANDED-NEXT:                     `-AbstractFunctionDeclScope {{.*}}, [18:3 - 18:43] 'my_identity()'