Merge pull request swiftlang#33953 from slavapestov/astscope-cleanups

slavapestov · web-flow · commit 38db485c47e8 · 2020-09-16T09:28:31.000-04:00
A couple of ASTScope-related cleanups
diff --git a/include/swift/AST/ASTScope.h b/include/swift/AST/ASTScope.h
@@ -529,7 +529,7 @@ class ASTScopeImpl {
   // A local binding is a basically a local variable defined in that very scope
   // It is not an instance variable or inherited type.
 
-  static bool lookupLocalBindingsInPattern(Pattern *p,
+  static bool lookupLocalBindingsInPattern(const Pattern *p,
                                            DeclVisibilityKind vis,
                                            DeclConsumer consumer);
 
@@ -1933,6 +1933,42 @@ class ForEachPatternScope final : public ASTScopeImpl {
   bool isLabeledStmtLookupTerminator() const override;
 };
 
+/// The parent scope for a 'case' statement, consisting of zero or more
+/// CaseLabelItemScopes, followed by a CaseStmtBodyScope.
+///
+/// +------------------------------------------------------------------
+/// | CaseStmtScope
+/// +------------------------------------------------------------------
+/// |                               +--------------------------+
+/// |                               | CaseLabelItemScope:      |
+/// |                               +--------------------------+
+/// | case .foo(let x, let y) where | condition(x, y),         |
+/// |               ^------^--------------------^--^           |
+/// |               this guard expression sees first 'x'/'y'   |
+/// |                               +--------------------------+
+/// |
+/// |                               +--------------------------+
+/// |                               | CaseLabelItemScope:      |
+/// |                               +--------------------------+
+/// |      .foo(let x, let y) where | condition(x, y),         |
+/// |               ^------^--------------------^--^           |
+/// |               this guard expression sees second 'x'/'y'  |
+/// |                               +--------------------------+
+/// |
+/// |      .bar(let x, let y)
+/// |               this case label item doesn't have a guard, so no
+/// |               scope is created.
+/// |
+/// | +----------------------------------------------------------------
+/// | | CaseStmtBodyScope:
+/// | +----------------------------------------------------------------
+/// | | {
+/// | |    ... x, y  <-- body sees "joined" 'x'/'y' created by parser
+/// | | }
+/// | +----------------------------------------------------------------
+/// |
+/// +------------------------------------------------------------------
+
 class CaseStmtScope final : public AbstractStmtScope {
 public:
   CaseStmt *const stmt;
@@ -1945,17 +1981,66 @@ class CaseStmtScope final : public AbstractStmtScope {
 private:
   void expandAScopeThatDoesNotCreateANewInsertionPoint(ScopeCreator &);
 
+public:
+  std::string getClassName() const override;
+  Stmt *getStmt() const override { return stmt; }
+};
+
+/// The scope used for the guard expression in a case statement. Any
+/// variables bound by the case label item's pattern are visible in
+/// this scope.
+class CaseLabelItemScope final : public ASTScopeImpl {
+public:
+  CaseLabelItem item;
+  CaseLabelItemScope(const CaseLabelItem &item) : item(item) {}
+  virtual ~CaseLabelItemScope() {}
+
+protected:
+  ASTScopeImpl *expandSpecifically(ScopeCreator &scopeCreator) override;
+
+private:
+  void expandAScopeThatDoesNotCreateANewInsertionPoint(ScopeCreator &);
+
 public:
   std::string getClassName() const override;
   SourceRange
   getSourceRangeOfThisASTNode(bool omitAssertions = false) const override;
-  Stmt *getStmt() const override { return stmt; }
 
 protected:
   bool lookupLocalsOrMembers(ArrayRef<const ASTScopeImpl *>,
                              ASTScopeImpl::DeclConsumer) const override;
 };
 
+/// The scope used for the body of a 'case' statement.
+///
+/// If the 'case' statement has multiple case label items, each label
+/// item's pattern must bind the same variables; the parser creates
+/// "fake" variables to represent the join of the variables bound by
+/// each pattern.
+///
+/// These "fake" variables are visible in the 'case' statement body.
+class CaseStmtBodyScope final : public ASTScopeImpl {
+public:
+  CaseStmt *const stmt;
+  CaseStmtBodyScope(CaseStmt *e) : stmt(e) {}
+  virtual ~CaseStmtBodyScope() {}
+
+protected:
+  ASTScopeImpl *expandSpecifically(ScopeCreator &scopeCreator) override;
+
+private:
+  void expandAScopeThatDoesNotCreateANewInsertionPoint(ScopeCreator &);
+
+public:
+  std::string getClassName() const override;
+  SourceRange
+  getSourceRangeOfThisASTNode(bool omitAssertions = false) const override;
+protected:
+  bool lookupLocalsOrMembers(ArrayRef<const ASTScopeImpl *>,
+                             ASTScopeImpl::DeclConsumer) const override;
+  bool isLabeledStmtLookupTerminator() const override;
+};
+
 class BraceStmtScope final : public AbstractStmtScope {
 
 public:
diff --git a/include/swift/AST/Types.h b/include/swift/AST/Types.h
@@ -6035,10 +6035,8 @@ inline ArrayRef<AnyFunctionType::Param> AnyFunctionType::getParams() const {
   }
 }
 
-/// If this is a method in a type or extension thereof, compute
-/// and return a parameter to be used for the 'self' argument.  The type of
-/// the parameter is the empty Type() if no 'self' argument should exist. This
-/// can only be used after types have been resolved.
+/// If this is a method in a type or extension thereof, return the
+/// 'self' parameter.
 ///
 /// \param isInitializingCtor Specifies whether we're computing the 'self'
 /// type of an initializing constructor, which accepts an instance 'self'
diff --git a/lib/AST/ASTScope.cpp b/lib/AST/ASTScope.cpp
@@ -253,6 +253,8 @@ DEFINE_GET_CLASS_NAME(SwitchStmtScope)
 DEFINE_GET_CLASS_NAME(ForEachStmtScope)
 DEFINE_GET_CLASS_NAME(ForEachPatternScope)
 DEFINE_GET_CLASS_NAME(CaseStmtScope)
+DEFINE_GET_CLASS_NAME(CaseLabelItemScope)
+DEFINE_GET_CLASS_NAME(CaseStmtBodyScope)
 DEFINE_GET_CLASS_NAME(BraceStmtScope)
 
 #undef DEFINE_GET_CLASS_NAME
diff --git a/lib/AST/ASTScopeCreation.cpp b/lib/AST/ASTScopeCreation.cpp
@@ -1169,6 +1169,8 @@ NO_NEW_INSERTION_POINT(EnumElementScope)
 
 NO_NEW_INSERTION_POINT(CaptureListScope)
 NO_NEW_INSERTION_POINT(CaseStmtScope)
+NO_NEW_INSERTION_POINT(CaseLabelItemScope)
+NO_NEW_INSERTION_POINT(CaseStmtBodyScope)
 NO_NEW_INSERTION_POINT(ClosureBodyScope)
 NO_NEW_INSERTION_POINT(DefaultArgumentInitializerScope)
 NO_NEW_INSERTION_POINT(DoStmtScope)
@@ -1486,10 +1488,24 @@ void ForEachPatternScope::expandAScopeThatDoesNotCreateANewInsertionPoint(
 
 void CaseStmtScope::expandAScopeThatDoesNotCreateANewInsertionPoint(
     ScopeCreator &scopeCreator) {
-  for (auto &caseItem : stmt->getMutableCaseLabelItems())
-    scopeCreator.addToScopeTree(caseItem.getGuardExpr(), this);
+  for (auto &item : stmt->getCaseLabelItems()) {
+    if (item.getGuardExpr()) {
+      scopeCreator.constructExpandAndInsertUncheckable<CaseLabelItemScope>(
+        this, item);
+    }
+  }
+
+  scopeCreator.constructExpandAndInsertUncheckable<CaseStmtBodyScope>(
+      this, stmt);
+}
 
-  // Add a child for the case body.
+void CaseLabelItemScope::expandAScopeThatDoesNotCreateANewInsertionPoint(
+    ScopeCreator &scopeCreator) {
+  scopeCreator.addToScopeTree(item.getGuardExpr(), this);
+}
+
+void CaseStmtBodyScope::expandAScopeThatDoesNotCreateANewInsertionPoint(
+    ScopeCreator &scopeCreator) {
   scopeCreator.addToScopeTree(stmt->getBody(), this);
 }
 
diff --git a/lib/AST/ASTScopeLookup.cpp b/lib/AST/ASTScopeLookup.cpp
@@ -422,12 +422,18 @@ bool ForEachPatternScope::lookupLocalsOrMembers(ArrayRef<const ASTScopeImpl *>,
       stmt->getPattern(), DeclVisibilityKind::LocalVariable, consumer);
 }
 
-bool CaseStmtScope::lookupLocalsOrMembers(ArrayRef<const ASTScopeImpl *>,
-                                          DeclConsumer consumer) const {
-  for (auto &item : stmt->getMutableCaseLabelItems())
-    if (lookupLocalBindingsInPattern(
-            item.getPattern(), DeclVisibilityKind::LocalVariable, consumer))
-      return true;
+bool CaseLabelItemScope::lookupLocalsOrMembers(ArrayRef<const ASTScopeImpl *>,
+                                               DeclConsumer consumer) const {
+  return lookupLocalBindingsInPattern(
+      item.getPattern(), DeclVisibilityKind::LocalVariable, consumer);
+}
+
+bool CaseStmtBodyScope::lookupLocalsOrMembers(ArrayRef<const ASTScopeImpl *>,
+                                              DeclConsumer consumer) const {
+  for (auto *var : stmt->getCaseBodyVariablesOrEmptyArray())
+    if (consumer.consume({var}, DeclVisibilityKind::LocalVariable))
+        return true;
+
   return false;
 }
 
@@ -554,7 +560,7 @@ bool ConditionalClausePatternUseScope::lookupLocalsOrMembers(
       pattern, DeclVisibilityKind::LocalVariable, consumer);
 }
 
-bool ASTScopeImpl::lookupLocalBindingsInPattern(Pattern *p,
+bool ASTScopeImpl::lookupLocalBindingsInPattern(const Pattern *p,
                                                 DeclVisibilityKind vis,
                                                 DeclConsumer consumer) {
   if (!p)
@@ -893,6 +899,10 @@ bool ForEachPatternScope::isLabeledStmtLookupTerminator() const {
   return false;
 }
 
+bool CaseStmtBodyScope::isLabeledStmtLookupTerminator() const {
+  return false;
+}
+
 llvm::SmallVector<LabeledStmt *, 4>
 ASTScopeImpl::lookupLabeledStmts(SourceFile *sourceFile, SourceLoc loc) {
   // Find the innermost scope from which to start our search.
diff --git a/lib/AST/ASTScopeSourceRange.cpp b/lib/AST/ASTScopeSourceRange.cpp
@@ -408,20 +408,13 @@ SourceRange ForEachPatternScope::getSourceRangeOfThisASTNode(
 }
 
 SourceRange
-CaseStmtScope::getSourceRangeOfThisASTNode(const bool omitAssertions) const {
-  // The scope of the case statement begins at the first guard expression,
-  // if there is one, and extends to the end of the body.
-  // FIXME: Figure out what to do about multiple pattern bindings. We might
-  // want a more restrictive rule in those cases.
-  for (const auto &caseItem : stmt->getCaseLabelItems()) {
-    if (auto guardExpr = caseItem.getGuardExpr())
-      return SourceRange(guardExpr->getStartLoc(),
-                         stmt->getBody()->getEndLoc());
-  }
+CaseLabelItemScope::getSourceRangeOfThisASTNode(const bool omitAssertions) const {
+  return item.getGuardExpr()->getSourceRange();
+}
 
-  // Otherwise, it covers the body.
-  return stmt->getBody()
-      ->getSourceRange(); // The scope of the case statement begins
+SourceRange
+CaseStmtBodyScope::getSourceRangeOfThisASTNode(const bool omitAssertions) const {
+  return stmt->getBody()->getSourceRange();
 }
 
 SourceRange
diff --git a/lib/SIL/Parser/ParseSIL.cpp b/lib/SIL/Parser/ParseSIL.cpp
@@ -168,7 +168,7 @@ namespace {
                                GenericEnvironment *GenericEnv,
                                GenericParamList *GenericParams);
 
-    void convertRequirements(SILFunction *F, ArrayRef<RequirementRepr> From,
+    void convertRequirements(ArrayRef<RequirementRepr> From,
                              SmallVectorImpl<Requirement> &To);
 
     ProtocolConformanceRef parseProtocolConformanceHelper(
@@ -845,49 +845,19 @@ static bool parseSILOptional(bool &Result, SILParser &SP, StringRef Expected) {
   return false;
 }
 
-namespace {
-  // FIXME: Nuke this.
-
-  /// A helper class to perform lookup of IdentTypes in the
-  /// current parser scope.
-  class IdentTypeReprLookup : public ASTWalker {
-    Parser &P;
-  public:
-    IdentTypeReprLookup(Parser &P) : P(P) {}
-
-    bool walkToTypeReprPre(TypeRepr *Ty) override {
-      auto *T = dyn_cast_or_null<IdentTypeRepr>(Ty);
-      auto Comp = T->getComponentRange().front();
-      if (auto Entry = P.lookupInScope(Comp->getNameRef()))
-        if (auto *TD = dyn_cast<TypeDecl>(Entry)) {
-          Comp->setValue(TD, nullptr);
-          return false;
-        }
-      return true;
-    }
-  };
-} // end anonymous namespace
-
 /// Remap RequirementReps to Requirements.
-void SILParser::convertRequirements(SILFunction *F,
-                                    ArrayRef<RequirementRepr> From,
+void SILParser::convertRequirements(ArrayRef<RequirementRepr> From,
                                     SmallVectorImpl<Requirement> &To) {
   if (From.empty()) {
     To.clear();
     return;
   }
 
-  auto *GenericEnv = F->getGenericEnvironment();
-  assert(GenericEnv);
-  (void)GenericEnv;
-
-  IdentTypeReprLookup PerformLookup(P);
   // Use parser lexical scopes to resolve references
   // to the generic parameters.
   auto ResolveToInterfaceType = [&](TypeRepr *TyR) -> Type {
-    TyR->walk(PerformLookup);
     return performTypeResolution(TyR, /*IsSILType=*/false,
-                                 ContextGenericEnv, nullptr)
+                                 ContextGenericEnv, ContextGenericParams)
         ->mapTypeOutOfContext();
   };
 
@@ -5792,8 +5762,7 @@ bool SILParserState::parseDeclSIL(Parser &P) {
         for (auto &Attr : SpecAttrs) {
           SmallVector<Requirement, 2> requirements;
           // Resolve types and convert requirements.
-          FunctionState.convertRequirements(FunctionState.F,
-                                            Attr.requirements, requirements);
+          FunctionState.convertRequirements(Attr.requirements, requirements);
           auto *fenv = FunctionState.F->getGenericEnvironment();
           auto genericSig = evaluateOrDefault(
               P.Context.evaluator,
diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp
@@ -2229,7 +2229,7 @@ FunctionType::ExtInfo ConstraintSystem::closureEffects(ClosureExpr *expr) {
       // Okay, resolve the pattern.
       Pattern *pattern = LabelItem.getPattern();
       if (!LabelItem.isPatternResolved()) {
-        pattern = TypeChecker::resolvePattern(pattern, CS.DC,
+        pattern = TypeChecker::resolvePattern(pattern, DC,
                                        /*isStmtCondition*/false);
         if (!pattern) return false;
 
@@ -2244,7 +2244,7 @@ FunctionType::ExtInfo ConstraintSystem::closureEffects(ClosureExpr *expr) {
         Type castType;
         if (auto castTypeRepr = isp->getCastTypeRepr()) {
           castType = TypeResolution::forContextual(
-                         CS.DC, TypeResolverContext::InExpression,
+                         DC, TypeResolverContext::InExpression,
                          /*unboundTyOpener*/ nullptr)
                          .resolveType(castTypeRepr);
         } else {

Original file line number	Diff line number	Diff line change
`@@ -6035,10 +6035,8 @@ inline ArrayRef<AnyFunctionType::Param> AnyFunctionType::getParams() const {`
`6035`	`6035`	`}`
`6036`	`6036`	`}`
`6037`	`6037`
`6038`		`-/// If this is a method in a type or extension thereof, compute`
`6039`		`-/// and return a parameter to be used for the 'self' argument. The type of`
`6040`		`-/// the parameter is the empty Type() if no 'self' argument should exist. This`
`6041`		`-/// can only be used after types have been resolved.`
	`6038`	`+/// If this is a method in a type or extension thereof, return the`
	`6039`	`+/// 'self' parameter.`
`6042`	`6040`	`///`
`6043`	`6041`	`/// \param isInitializingCtor Specifies whether we're computing the 'self'`
`6044`	`6042`	`/// type of an initializing constructor, which accepts an instance 'self'`