Merge pull request swiftlang#33989 from slavapestov/astscope-self-dc-cleanup

ASTScope: Redo 'selfDC' computation

Author: slavapestov

include/swift/AST/ASTScope.h

@@ -474,16 +474,6 @@ class ASTScopeImpl {
               NullablePtr<const GenericParamList> lastListSearched,
               DeclConsumer consumer) const;
 
-public:
-  /// Returns the SelfDC for parent (and possibly ancestor) scopes.
-  /// A return of None indicates that the previous child (in history) should be
-  /// asked.
-  virtual Optional<NullablePtr<DeclContext>> computeSelfDCForParent() const;
-
-  /// Returns the context that should be used when a nested scope (e.g. a
-  /// closure) captures self explicitly.
-  virtual NullablePtr<DeclContext> capturedSelfDC() const;
-
 protected:
   /// Find either locals or members (no scope has both)
   /// \param history The scopes visited since the start of lookup (including
@@ -691,27 +681,6 @@ class Portion {
     bool lookupMembersOf(const GenericTypeOrExtensionScope *scope,
                          ArrayRef<const ASTScopeImpl *>,
                          ASTScopeImpl::DeclConsumer consumer) const override;
-
-  private:
-    /// A client needs to know if a lookup result required the dynamic implicit
-    /// self value. It is required if the lookup originates from a method body
-    /// or a lazy pattern initializer. So, one approach would be to call the
-    /// consumer to find members right from those scopes. However, because
-    /// members aren't the first things searched, generics are, that approache
-    /// ends up duplicating code from the \c GenericTypeOrExtensionScope. So we
-    /// take the approach of doing those lookups there, and using this function
-    /// to compute the selfDC from the history.
-    static NullablePtr<DeclContext>
-    computeSelfDC(ArrayRef<const ASTScopeImpl *> history);
-    
-    /// If we find a lookup result that requires the dynamic implict self value,
-    /// we need to check the nested scopes to see if any closures explicitly
-    /// captured \c self. In that case, the appropriate selfDC is that of the
-    /// innermost closure which captures a \c self value from one of this type's
-    /// methods.
-    static NullablePtr<DeclContext>
-    checkNestedScopesForSelfCapture(ArrayRef<const ASTScopeImpl *> history,
-                                    size_t start);
 };
 
 /// Behavior specific to representing the trailing where clause of a
@@ -810,7 +779,6 @@ class GenericTypeOrExtensionScope : public ASTScopeImpl {
   virtual std::string declKindName() const = 0;
   virtual bool doesDeclHaveABody() const;
   const char *portionName() const { return portion->portionName; }
-  Optional<NullablePtr<DeclContext>> computeSelfDCForParent() const override;
 
 protected:
   Optional<bool> resolveIsCascadingUseForThisScope(
@@ -1037,8 +1005,6 @@ class AbstractFunctionDeclScope final : public ASTScopeImpl {
 
   NullablePtr<const void> getReferrent() const override;
 
-  static bool shouldCreateAccessorScope(const AccessorDecl *);
-
 protected:
   SourceRange
   getSourceRangeOfEnclosedParamsOfASTNode(bool omitAssertions) const override;
@@ -1115,7 +1081,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;
 
@@ -1130,21 +1095,10 @@ 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 *>,
@@ -1317,8 +1271,6 @@ class PatternEntryInitializerScope final : public AbstractPatternEntryScope {
   getSourceRangeOfThisASTNode(bool omitAssertions = false) const override;
   virtual NullablePtr<DeclContext> getDeclContext() const override;
 
-  Optional<NullablePtr<DeclContext>> computeSelfDCForParent() const override;
-
 protected:
   bool lookupLocalsOrMembers(ArrayRef<const ASTScopeImpl *>,
                              DeclConsumer) const override;
@@ -1430,13 +1382,6 @@ class ClosureParametersScope final : public ASTScopeImpl {
   std::string getClassName() const override;
   SourceRange
   getSourceRangeOfThisASTNode(bool omitAssertions = false) const override;
-  
-  /// Since explicit captures of \c self by closures enable the use of implicit
-  /// \c self, we need to make sure that the appropriate \c self is used as the
-  /// base decl for these uses (otherwise, the capture would be marked as
-  /// unused. \c ClosureParametersScope::capturedSelfDC() checks if we have such
-  ///  a capture of self.
-  NullablePtr<DeclContext> capturedSelfDC() const override;
 
   NullablePtr<ClosureExpr> getClosureIfClosureScope() const override {
     return closureExpr;

include/swift/AST/Decl.h

@@ -62,6 +62,7 @@ namespace swift {
   class DynamicSelfType;
   class Type;
   class Expr;
+  class CaptureListExpr;
   class DeclRefExpr;
   class ForeignAsyncConvention;
   class ForeignErrorConvention;
@@ -352,21 +353,13 @@ class alignas(1 << DeclAlignInBits) Decl {
     IsStatic : 1
   );
 
-  SWIFT_INLINE_BITFIELD(VarDecl, AbstractStorageDecl, 1+1+1+1+1+1+1+1,
+  SWIFT_INLINE_BITFIELD(VarDecl, AbstractStorageDecl, 1+1+1+1+1+1,
     /// Encodes whether this is a 'let' binding.
     Introducer : 1,
 
-    /// Whether this declaration was an element of a capture list.
-    IsCaptureList : 1,
-                        
     /// Whether this declaration captures the 'self' param under the same name.
     IsSelfParamCapture : 1,
 
-    /// Whether this vardecl has an initial value bound to it in a way
-    /// that isn't represented in the AST with an initializer in the pattern
-    /// binding.  This happens in cases like "for i in ...", switch cases, etc.
-    HasNonPatternBindingInit : 1,
-
     /// Whether this is a property used in expressions in the debugger.
     /// It is up to the debugger to instruct SIL how to access this variable.
     IsDebuggerVar : 1,
@@ -4892,16 +4885,19 @@ class VarDecl : public AbstractStorageDecl {
   };
 
 protected:
-  PointerUnion<PatternBindingDecl *, Stmt *, VarDecl *> Parent;
+  PointerUnion<PatternBindingDecl *,
+               Stmt *,
+               VarDecl *,
+               CaptureListExpr *> Parent;
 
   VarDecl(DeclKind kind, bool isStatic, Introducer introducer,
-          bool isCaptureList, SourceLoc nameLoc, Identifier name,
-          DeclContext *dc, StorageIsMutable_t supportsMutation);
+          SourceLoc nameLoc, Identifier name, DeclContext *dc,
+          StorageIsMutable_t supportsMutation);
 
 public:
-  VarDecl(bool isStatic, Introducer introducer, bool isCaptureList,
+  VarDecl(bool isStatic, Introducer introducer,
           SourceLoc nameLoc, Identifier name, DeclContext *dc)
-    : VarDecl(DeclKind::Var, isStatic, introducer, isCaptureList, nameLoc,
+    : VarDecl(DeclKind::Var, isStatic, introducer, nameLoc,
               name, dc, StorageIsMutable_t(introducer == Introducer::Var)) {}
 
   SourceRange getSourceRange() const;
@@ -5091,25 +5087,29 @@ class VarDecl : public AbstractStorageDecl {
     Bits.VarDecl.Introducer = uint8_t(value);
   }
 
+  CaptureListExpr *getParentCaptureList() const {
+    if (!Parent)
+      return nullptr;
+    return Parent.dyn_cast<CaptureListExpr *>();
+  }
+
+  /// Set \p v to be the pattern produced VarDecl that is the parent of this
+  /// var decl.
+  void setParentCaptureList(CaptureListExpr *expr) {
+    assert(expr != nullptr);
+    Parent = expr;
+  }
   /// Is this an element in a capture list?
-  bool isCaptureList() const { return Bits.VarDecl.IsCaptureList; }
+  bool isCaptureList() const {
+    return getParentCaptureList() != nullptr;
+  }
 
   /// Is this a capture of the self param?
   bool isSelfParamCapture() const { return Bits.VarDecl.IsSelfParamCapture; }
   void setIsSelfParamCapture(bool IsSelfParamCapture = true) {
       Bits.VarDecl.IsSelfParamCapture = IsSelfParamCapture;
   }
 
-  /// Return true if this vardecl has an initial value bound to it in a way
-  /// that isn't represented in the AST with an initializer in the pattern
-  /// binding.  This happens in cases like "for i in ...", switch cases, etc.
-  bool hasNonPatternBindingInit() const {
-    return Bits.VarDecl.HasNonPatternBindingInit;
-  }
-  void setHasNonPatternBindingInit(bool V = true) {
-    Bits.VarDecl.HasNonPatternBindingInit = V;
-  }
-
   /// Determines if this var has an initializer expression that should be
   /// exposed to clients.
   /// There's a very narrow case when we would: if the decl is an instance

include/swift/AST/Expr.h

@@ -3941,12 +3941,6 @@ class ClosureExpr : public AbstractClosureExpr {
   VarDecl *getCapturedSelfDecl() const {
     return CapturedSelfDecl;
   }
-  
-  /// Whether this closure captures the \c self param in its body in such a
-  /// way that implicit \c self is enabled within its body (i.e. \c self is
-  /// captured non-weakly).
-  bool capturesSelfEnablingImplictSelf() const;
-
 
   /// Get the type checking state of this closure's body.
   BodyState getBodyState() const {

include/swift/AST/NameLookup.h

@@ -633,7 +633,7 @@ class AbstractASTScopeDeclConsumer {
 
   /// Eventually this functionality should move into ASTScopeLookup
   virtual bool
-  lookInMembers(NullablePtr<DeclContext> selfDC, DeclContext *const scopeDC,
+  lookInMembers(DeclContext *const scopeDC,
                 NominalTypeDecl *const nominal,
                 function_ref<bool(Optional<bool>)> calculateIsCascadingUse) = 0;
 
@@ -656,7 +656,7 @@ class ASTScopeDeclGatherer : public AbstractASTScopeDeclConsumer {
                NullablePtr<DeclContext> baseDC = nullptr) override;
 
   /// Eventually this functionality should move into ASTScopeLookup
-  bool lookInMembers(NullablePtr<DeclContext>, DeclContext *const,
+  bool lookInMembers(DeclContext *const,
                      NominalTypeDecl *const,
                      function_ref<bool(Optional<bool>)>) override {
     return false;

include/swift/AST/Pattern.h

@@ -190,14 +190,6 @@ class alignas(8) Pattern {
 
   bool isNeverDefaultInitializable() const;
 
-  /// Mark all vardecls in this pattern as having non-pattern initial
-  /// values bound into them.
-  void markHasNonPatternBindingInit() {
-    forEachVariable([&](VarDecl *VD) {
-      VD->setHasNonPatternBindingInit();
-    });
-  }
-
   /// Mark all vardecls in this pattern as having an owning statement for
   /// the pattern.
   void markOwnedByStatement(Stmt *S) {

include/swift/AST/TypeAlignments.h

@@ -32,6 +32,7 @@ namespace swift {
   class ASTContext;
   class AttributeBase;
   class BraceStmt;
+  class CaptureListExpr;
   class Decl;
   class DeclContext;
   class Expr;
@@ -112,6 +113,7 @@ LLVM_DECLARE_TYPE_ALIGNMENT(swift::BraceStmt, swift::StmtAlignInBits)
 LLVM_DECLARE_TYPE_ALIGNMENT(swift::ASTContext, 2);
 LLVM_DECLARE_TYPE_ALIGNMENT(swift::DeclContext, swift::DeclContextAlignInBits)
 LLVM_DECLARE_TYPE_ALIGNMENT(swift::Expr, swift::ExprAlignInBits)
+LLVM_DECLARE_TYPE_ALIGNMENT(swift::CaptureListExpr, swift::ExprAlignInBits)
 LLVM_DECLARE_TYPE_ALIGNMENT(swift::AbstractClosureExpr, swift::ExprAlignInBits)
 LLVM_DECLARE_TYPE_ALIGNMENT(swift::OpaqueValueExpr, swift::ExprAlignInBits)
 LLVM_DECLARE_TYPE_ALIGNMENT(swift::ProtocolConformance, swift::DeclAlignInBits)

lib/AST/ASTDumper.cpp

@@ -842,8 +842,6 @@ namespace {
       printCommon(VD, "var_decl");
       if (VD->isLet())
         PrintWithColorRAII(OS, DeclModifierColor) << " let";
-      if (VD->hasNonPatternBindingInit())
-        PrintWithColorRAII(OS, DeclModifierColor) << " non_pattern_init";
       if (VD->getAttrs().hasAttribute<LazyAttr>())
         PrintWithColorRAII(OS, DeclModifierColor) << " lazy";
       printStorageImpl(VD);

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);
   }
 }
 
@@ -2004,11 +2000,6 @@ ASTScopeImpl::rescueASTAncestorScopesForReuseFromMe() {
   return astAncestorScopes;
 }
 
-bool AbstractFunctionDeclScope::shouldCreateAccessorScope(
-    const AccessorDecl *const ad) {
-  return isLocalizable(ad);
-}
-
 #pragma mark verification
 
 namespace {