ASTScope: Collapse PureFunctionBodyScope and MethodBodyScope

This centralizes some invariants around the 'self' parameter.
While all ConstructorDecls and DestructorDecls have a 'self',
even if they're invalid because they're not nested inside a type,
we don't want to consider this as the base 'self' for lookups.

Eg, consider this invalid code:

class C {
  func f() {
    init() {
      x
    }
  }
}

The base for the lookup should be f.self, not f.init.self.

Author: slavapestov

include/swift/AST/ASTScope.h

@@ -1113,7 +1113,6 @@ class AbstractFunctionBodyScope : public ASTScopeImpl {
   }
   virtual NullablePtr<Decl> getDeclIfAny() const override { return decl; }
   Decl *getDecl() const { return decl; }
-  static bool isAMethod(const AbstractFunctionDecl *);
 
   NullablePtr<ASTScopeImpl> getParentOfASTAncestorScopesToBeRescued() override;
 
@@ -1128,25 +1127,15 @@ class AbstractFunctionBodyScope : public ASTScopeImpl {
   SourceRange sourceRangeForDeferredExpansion() const override;
 };
 
-/// Body of methods, functions in types.
-class MethodBodyScope final : public AbstractFunctionBodyScope {
+/// Body of functions and methods.
+class FunctionBodyScope final : public AbstractFunctionBodyScope {
 public:
-  MethodBodyScope(AbstractFunctionDecl *e) : AbstractFunctionBodyScope(e) {}
-  std::string getClassName() const override;
-  bool lookupLocalsOrMembers(ArrayRef<const ASTScopeImpl *>,
-                             DeclConsumer consumer) const override;
-
-  Optional<NullablePtr<DeclContext>> computeSelfDCForParent() const override;
-};
-
-/// Body of "pure" functions, functions without an implicit "self".
-class PureFunctionBodyScope final : public AbstractFunctionBodyScope {
-public:
-  PureFunctionBodyScope(AbstractFunctionDecl *e)
+  FunctionBodyScope(AbstractFunctionDecl *e)
       : AbstractFunctionBodyScope(e) {}
   std::string getClassName() const override;
   bool lookupLocalsOrMembers(ArrayRef<const ASTScopeImpl *>,
                              DeclConsumer consumer) const override;
+  Optional<NullablePtr<DeclContext>> computeSelfDCForParent() const override;
 };
 
 class DefaultArgumentInitializerScope final : public ASTScopeImpl {

lib/AST/ASTScope.cpp

@@ -118,20 +118,6 @@ Stmt *LabeledConditionalStmtScope::getStmt() const {
   return getLabeledConditionalStmt();
 }
 
-bool AbstractFunctionBodyScope::isAMethod(
-    const AbstractFunctionDecl *const afd) {
-  // What makes this interesting is that a method named "init" which is not
-  // in a nominal type or extension decl body still gets an implicit self
-  // parameter (even though the program is illegal).
-  // So when choosing between creating a MethodBodyScope and a
-  // PureFunctionBodyScope do we go by the enclosing Decl (i.e.
-  // "afd->getDeclContext()->isTypeContext()") or by
-  // "bool(afd->getImplicitSelfDecl())"?
-  //
-  // Since the code uses \c getImplicitSelfDecl, use that.
-  return afd->getImplicitSelfDecl();
-}
-
 #pragma mark getLabeledConditionalStmt
 LabeledConditionalStmt *IfStmtScope::getLabeledConditionalStmt() const {
   return stmt;
@@ -218,8 +204,7 @@ DEFINE_GET_CLASS_NAME(ASTSourceFileScope)
 DEFINE_GET_CLASS_NAME(GenericParamScope)
 DEFINE_GET_CLASS_NAME(AbstractFunctionDeclScope)
 DEFINE_GET_CLASS_NAME(ParameterListScope)
-DEFINE_GET_CLASS_NAME(MethodBodyScope)
-DEFINE_GET_CLASS_NAME(PureFunctionBodyScope)
+DEFINE_GET_CLASS_NAME(FunctionBodyScope)
 DEFINE_GET_CLASS_NAME(DefaultArgumentInitializerScope)
 DEFINE_GET_CLASS_NAME(AttachedPropertyWrapperScope)
 DEFINE_GET_CLASS_NAME(PatternEntryDeclScope)

lib/AST/ASTScopeCreation.cpp

@@ -1416,12 +1416,8 @@ void AbstractFunctionDeclScope::expandAScopeThatDoesNotCreateANewInsertionPoint(
   // We create body scopes when there is no body for source kit to complete
   // erroneous code in bodies.
   if (decl->getBodySourceRange().isValid()) {
-    if (AbstractFunctionBodyScope::isAMethod(decl))
-      scopeCreator.constructExpandAndInsertUncheckable<MethodBodyScope>(leaf,
+    scopeCreator.constructExpandAndInsertUncheckable<FunctionBodyScope>(leaf,
                                                                         decl);
-    else
-      scopeCreator.constructExpandAndInsertUncheckable<PureFunctionBodyScope>(
-          leaf, decl);
   }
 }
 

lib/AST/ASTScopeLookup.cpp

@@ -335,22 +335,15 @@ bool AbstractFunctionBodyScope::lookupLocalsOrMembers(
   return false;
 }
 
-bool MethodBodyScope::lookupLocalsOrMembers(
+bool FunctionBodyScope::lookupLocalsOrMembers(
     ArrayRef<const ASTScopeImpl *> history, DeclConsumer consumer) const {
-  ASTScopeAssert(isAMethod(decl), "Asking for members of a non-method.");
   if (AbstractFunctionBodyScope::lookupLocalsOrMembers(history, consumer))
     return true;
-  return consumer.consume({decl->getImplicitSelfDecl()},
-                          DeclVisibilityKind::FunctionParameter);
-}
 
-bool PureFunctionBodyScope::lookupLocalsOrMembers(
-    ArrayRef<const ASTScopeImpl *> history, DeclConsumer consumer) const {
-  ASTScopeAssert(
-      !isAMethod(decl),
-      "Should have called lookupLocalsOrMembers instead of this function.");
-  if (AbstractFunctionBodyScope::lookupLocalsOrMembers(history, consumer))
-    return true;
+  if (decl->getDeclContext()->isTypeContext()) {
+    return consumer.consume({decl->getImplicitSelfDecl()},
+                            DeclVisibilityKind::FunctionParameter);
+  }
 
   // Consider \c var t: T { (did/will/)get/set { ... t }}
   // Lookup needs to find t, but if the var is inside of a type the baseDC needs
@@ -361,6 +354,7 @@ bool PureFunctionBodyScope::lookupLocalsOrMembers(
       if (consumer.consume({storage}, DeclVisibilityKind::LocalVariable))
         return true;
   }
+
   return false;
 }
 
@@ -629,8 +623,10 @@ PatternEntryInitializerScope::computeSelfDCForParent() const {
 }
 
 Optional<NullablePtr<DeclContext>>
-MethodBodyScope::computeSelfDCForParent() const {
-  return NullablePtr<DeclContext>(decl);
+FunctionBodyScope::computeSelfDCForParent() const {
+  if (decl->getDeclContext()->isTypeContext())
+    return NullablePtr<DeclContext>(decl);
+  return None;
 }
 
 #pragma mark capturedSelfDC

lib/Sema/TypeCheckNameLookup.cpp

@@ -255,9 +255,7 @@ LookupResult TypeChecker::lookupUnqualified(DeclContext *dc, DeclNameRef name,
       if (!typeDC->isTypeContext()) {
         // If we don't have a type context this is an implicit 'self' reference.
         if (auto *CE = dyn_cast<ClosureExpr>(typeDC)) {
-          // If we found the result in a self capture, look through the capture.
-          assert(CE->getCapturedSelfDecl());
-          typeDC = found.getValueDecl()->getDeclContext();
+          typeDC = typeDC->getInnermostTypeContext();
         } else {
           // Otherwise, we must have the method context.
           typeDC = typeDC->getParent();

test/NameLookup/scope_map_top_level.swift

@@ -42,7 +42,7 @@ var i: Int = b.my_identity()
 // 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:                 `-PureFunctionBodyScope {{.*}}, [11:12 - 11:13]
+// CHECK-EXPANDED-NEXT:                 `-FunctionBodyScope {{.*}}, [11:12 - 11:13]
 // CHECK-EXPANDED-NEXT:                   `-BraceStmtScope {{.*}}, [11:12 - 11:13]
 // CHECK-EXPANDED-NEXT:             `-TopLevelCodeScope {{.*}}, [13:1 - 21:28]
 // CHECK-EXPANDED-NEXT:               `-BraceStmtScope {{.*}}, [13:1 - 21:28]
@@ -53,7 +53,7 @@ var i: Int = b.my_identity()
 // 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:                         `-MethodBodyScope {{.*}}, [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]