ASTScope: FunctionBodyScope should not be nested inside ParameterListScope

Author: slavapestov

include/swift/AST/ASTScope.h

@@ -298,9 +298,6 @@ class ASTScopeImpl {
   bool verifyThatChildrenAreContainedWithin(SourceRange) const;
   bool verifyThatThisNodeComeAfterItsPriorSibling() const;
 
-  virtual SourceRange
-  getSourceRangeOfEnclosedParamsOfASTNode(bool omitAssertions) const;
-
 private:
   bool checkSourceRangeAfterExpansion(const ASTContext &) const;
 
@@ -991,13 +988,6 @@ class AbstractFunctionDeclScope final : public ASTScopeImpl {
 
   NullablePtr<const void> getReferrent() const override;
 
-protected:
-  SourceRange
-  getSourceRangeOfEnclosedParamsOfASTNode(bool omitAssertions) const override;
-
-private:
-  static SourceLoc getParmsSourceLocOfAFD(AbstractFunctionDecl *);
-
 protected:
   NullablePtr<const GenericParamList> genericParams() const override;
 };
@@ -1507,9 +1497,6 @@ class SubscriptDeclScope final : public ASTScopeImpl {
   NullablePtr<const void> getReferrent() const override;
 
 protected:
-  SourceRange
-  getSourceRangeOfEnclosedParamsOfASTNode(bool omitAssertions) const override;
-
   NullablePtr<const GenericParamList> genericParams() const override;
   NullablePtr<AbstractStorageDecl>
   getEnclosingAbstractStorageDecl() const override {
@@ -1566,10 +1553,6 @@ class EnumElementScope : public ASTScopeImpl {
   NullablePtr<Decl> getDeclIfAny() const override { return decl; }
   Decl *getDecl() const { return decl; }
 
-protected:
-  SourceRange
-  getSourceRangeOfEnclosedParamsOfASTNode(bool omitAssertions) const override;
-
 private:
   void expandAScopeThatDoesNotCreateANewInsertionPoint(ScopeCreator &);
 };

lib/AST/ASTScopeCreation.cpp

@@ -1298,24 +1298,23 @@ void AbstractFunctionDeclScope::expandAScopeThatDoesNotCreateANewInsertionPoint(
             .ifUniqueConstructExpandAndInsert<DifferentiableAttributeScope>(
                 this, diffAttr, decl);
       });
+
   // Create scopes for generic and ordinary parameters.
   // For a subscript declaration, the generic and ordinary parameters are in an
   // ancestor scope, so don't make them here.
   ASTScopeImpl *leaf = this;
+
   if (!isa<AccessorDecl>(decl)) {
     leaf = scopeCreator.addNestedGenericParamScopesToTree(
         decl, decl->getGenericParams(), leaf);
-    if (isLocalizable(decl) && getParmsSourceLocOfAFD(decl).isValid()) {
-      // swift::createDesignatedInitOverride just clones the parameters, so they
-      // end up with a bogus SourceRange, maybe *before* the start of the
-      // function.
-      if (!decl->isImplicit()) {
-        leaf = scopeCreator
-                   .constructExpandAndInsertUncheckable<ParameterListScope>(
-                       leaf, decl->getParameters(), nullptr);
-      }
+
+    auto *params = decl->getParameters();
+    if (params->size() > 0) {
+      scopeCreator.constructExpandAndInsertUncheckable<ParameterListScope>(
+          leaf, params, nullptr);
     }
   }
+
   // Create scope for the body.
   // We create body scopes when there is no body for source kit to complete
   // erroneous code in bodies.

lib/AST/ASTScopeSourceRange.cpp

@@ -362,8 +362,7 @@ SourceRange AbstractFunctionDeclScope::getSourceRangeOfThisASTNode(
 
 SourceRange ParameterListScope::getSourceRangeOfThisASTNode(
     const bool omitAssertions) const {
-  const auto rangeForGoodInput =
-      getSourceRangeOfEnclosedParamsOfASTNode(omitAssertions);
+  auto rangeForGoodInput = params->getSourceRange();
   auto r = SourceRange(rangeForGoodInput.Start,
                        fixupEndForBadInput(rangeForGoodInput));
   ASTScopeAssert(getSourceManager().rangeContains(
@@ -672,52 +671,6 @@ void ASTScopeImpl::widenSourceRangeForIgnoredASTNode(const ASTNode n) {
     sourceRangeOfIgnoredASTNodes.widen(r);
 }
 
-#pragma mark getSourceRangeOfEnclosedParamsOfASTNode
-
-SourceRange ASTScopeImpl::getSourceRangeOfEnclosedParamsOfASTNode(
-    const bool omitAssertions) const {
-  return getParent().get()->getSourceRangeOfEnclosedParamsOfASTNode(
-      omitAssertions);
-}
-
-SourceRange EnumElementScope::getSourceRangeOfEnclosedParamsOfASTNode(
-    bool omitAssertions) const {
-  auto *pl = decl->getParameterList();
-  return pl ? pl->getSourceRange() : SourceRange();
-}
-
-SourceRange SubscriptDeclScope::getSourceRangeOfEnclosedParamsOfASTNode(
-    const bool omitAssertions) const {
-  auto r = SourceRange(decl->getIndices()->getLParenLoc(), decl->getEndLoc());
-  // Because of "subscript(x: MyStruct#^PARAM_1^#) -> Int { return 0 }"
-  // Cannot just use decl->getEndLoc()
-  r.widen(decl->getIndices()->getRParenLoc());
-  return r;
-}
-
-SourceRange AbstractFunctionDeclScope::getSourceRangeOfEnclosedParamsOfASTNode(
-    const bool omitAssertions) const {
-  const auto s = getParmsSourceLocOfAFD(decl);
-  const auto e = getSourceRangeOfThisASTNode(omitAssertions).End;
-  return s.isInvalid() || e.isInvalid() ? SourceRange() : SourceRange(s, e);
-}
-
-SourceLoc
-AbstractFunctionDeclScope::getParmsSourceLocOfAFD(AbstractFunctionDecl *decl) {
-  if (auto *c = dyn_cast<ConstructorDecl>(decl))
-    return c->getParameters()->getLParenLoc();
-
-  if (auto *dd = dyn_cast<DestructorDecl>(decl))
-    return dd->getNameLoc();
-
-  auto *fd = cast<FuncDecl>(decl);
-  // clang-format off
-  return isa<AccessorDecl>(fd) ? fd->getLoc()
-       : fd->isDeferBody()     ? fd->getNameLoc()
-       :                         fd->getParameters()->getLParenLoc();
-  // clang-format on
-}
-
 SourceLoc getLocAfterExtendedNominal(const ExtensionDecl *const ext) {
   const auto *const etr = ext->getExtendedTypeRepr();
   if (!etr)

test/NameLookup/scope_map_top_level.swift

@@ -8,7 +8,7 @@ let a: Int? = 1
 guard let b = a else {
 }
 
-func foo() {} // to interrupt the TopLevelCodeDecl
+func foo(x: Int) {} // to interrupt the TopLevelCodeDecl
 
 let c = b
 
@@ -40,10 +40,10 @@ var i: Int = b.my_identity()
 // CHECK-EXPANDED-NEXT:             `-ConditionalClausePatternUseScope, [8:22 - 8:22] let b{{\??}}
 // CHECK-EXPANDED-NEXT:           |-BraceStmtScope {{.*}}, [8:22 - 9:1]
 // CHECK-EXPANDED-NEXT:           `-LookupParentDiversionScope, [9:1 - 21:28]
-// CHECK-EXPANDED-NEXT:             |-AbstractFunctionDeclScope {{.*}}, [11:1 - 11:13] 'foo()'
-// CHECK-EXPANDED-NEXT:               `-ParameterListScope {{.*}}, [11:9 - 11:13]
-// CHECK-EXPANDED-NEXT:                 `-FunctionBodyScope {{.*}}, [11:12 - 11:13]
-// CHECK-EXPANDED-NEXT:                   `-BraceStmtScope {{.*}}, [11:12 - 11:13]
+// CHECK-EXPANDED-NEXT:             |-AbstractFunctionDeclScope {{.*}}, [11:1 - 11:19] 'foo(x:)'
+// CHECK-EXPANDED-NEXT:               |-ParameterListScope {{.*}}, [11:9 - 11:16]
+// CHECK-EXPANDED-NEXT:               `-FunctionBodyScope {{.*}}, [11:18 - 11:19]
+// 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'
@@ -52,9 +52,8 @@ var i: Int = b.my_identity()
 // 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()'
-// CHECK-EXPANDED-NEXT:                       `-ParameterListScope {{.*}}, [18:19 - 18:43]
-// CHECK-EXPANDED-NEXT:                         `-FunctionBodyScope {{.*}}, [18:29 - 18:43]
-// CHECK-EXPANDED-NEXT:                           `-BraceStmtScope {{.*}}, [18:29 - 18:43]
+// CHECK-EXPANDED-NEXT:                       `-FunctionBodyScope {{.*}}, [18:29 - 18:43]
+// CHECK-EXPANDED-NEXT:                         `-BraceStmtScope {{.*}}, [18:29 - 18:43]
 // CHECK-EXPANDED-NEXT:                 `-TopLevelCodeScope {{.*}}, [21:1 - 21:28]
 // CHECK-EXPANDED-NEXT:                   `-BraceStmtScope {{.*}}, [21:1 - 21:28]
 // CHECK-EXPANDED-NEXT:                     `-PatternEntryDeclScope {{.*}}, [21:5 - 21:28] entry 0 'i'