ASTScope: Rework ConditionalClauseScopes

The top-level scope for a conditional clause with a pattern is now
ConditionalClausePatternUseScope, which introduces the pattern's
bindings.

Since the patterns are not visible in their own initializer, a new
ConditionalClauseInitializerScope is used for the initializer.
While it is nested inside of the ConditionalClausePatternUseScope,
it's lookup parent skips one level, giving us the desired behavior.

Author: slavapestov

include/swift/AST/ASTScope.h

@@ -1082,63 +1082,49 @@ class PatternEntryInitializerScope final : public AbstractPatternEntryScope {
   bool lookupLocalsOrMembers(DeclConsumer) const override;
 };
 
-/// The scope introduced by a conditional clause in an if/guard/while
-/// statement.
-/// Since there may be more than one "let foo = ..." in (e.g.) an "if",
-/// we allocate a matrushka of these.
-class ConditionalClauseScope final : public ASTScopeImpl {
+/// The scope introduced by a conditional clause initializer in an
+/// if/while/guard statement.
+class ConditionalClauseInitializerScope final : public ASTScopeImpl {
 public:
-  LabeledConditionalStmt *const stmt;
-  const unsigned index;
-  const SourceLoc endLoc; // cannot get it from the stmt
-
-  ConditionalClauseScope(LabeledConditionalStmt *stmt, unsigned index,
-                         SourceLoc endLoc)
-      : stmt(stmt), index(index), endLoc(endLoc) {}
-
-  virtual ~ConditionalClauseScope() {}
-
-protected:
-  ASTScopeImpl *expandSpecifically(ScopeCreator &scopeCreator) override;
+  Expr *const initializer;
+  const SourceRange bodyRange;
 
-private:
-  AnnotatedInsertionPoint
-  expandAScopeThatCreatesANewInsertionPoint(ScopeCreator &);
-
-public:
-  std::string getClassName() const override;
-
-protected:
-  void printSpecifics(llvm::raw_ostream &out) const override;
+  ConditionalClauseInitializerScope(Expr *initializer)
+      : initializer(initializer) {}
 
-public:
+  virtual ~ConditionalClauseInitializerScope() {}
   SourceRange
   getSourceRangeOfThisASTNode(bool omitAssertions = false) const override;
+  std::string getClassName() const override;
 
 private:
-  ArrayRef<StmtConditionElement> getCond() const;
-  const StmtConditionElement &getStmtConditionElement() const;
+  void expandAScopeThatDoesNotCreateANewInsertionPoint(ScopeCreator &);
 
 protected:
-  bool isLabeledStmtLookupTerminator() const override;
+  ASTScopeImpl *expandSpecifically(ScopeCreator &scopeCreator) override;
+  NullablePtr<const ASTScopeImpl> getLookupParent() const override;
 };
 
 /// If, while, & guard statements all start with a conditional clause, then some
 /// later part of the statement, (then, body, or after the guard) circumvents
 /// the normal lookup rule to pass the lookup scope into the deepest conditional
 /// clause.
 class ConditionalClausePatternUseScope final : public ASTScopeImpl {
-  Pattern *const pattern;
-  const SourceLoc startLoc;
+  StmtConditionElement sec;
+  SourceLoc endLoc;
 
 public:
-  ConditionalClausePatternUseScope(Pattern *pattern, SourceLoc startLoc)
-      : pattern(pattern), startLoc(startLoc) {}
+  ConditionalClausePatternUseScope(StmtConditionElement sec, SourceLoc endLoc)
+      : sec(sec), endLoc(endLoc) {}
 
   SourceRange
   getSourceRangeOfThisASTNode(bool omitAssertions = false) const override;
   std::string getClassName() const override;
 
+private:
+  AnnotatedInsertionPoint
+  expandAScopeThatCreatesANewInsertionPoint(ScopeCreator &);
+
 protected:
   ASTScopeImpl *expandSpecifically(ScopeCreator &) override;
   bool lookupLocalsOrMembers(DeclConsumer) const override;
@@ -1350,7 +1336,7 @@ class LabeledConditionalStmtScope : public AbstractStmtScope {
 protected:
   /// Return the lookupParent required to search these.
   ASTScopeImpl *createNestedConditionalClauseScopes(ScopeCreator &,
-                                                    const Stmt *afterConds);
+                                                    SourceLoc);
 };
 
 class IfStmtScope final : public LabeledConditionalStmtScope {
@@ -1408,28 +1394,24 @@ class GuardStmtScope final : public LabeledConditionalStmtScope {
   getSourceRangeOfThisASTNode(bool omitAssertions = false) const override;
 };
 
-/// A scope after a guard statement that follows lookups into the conditions
-/// Also for:
-///  The insertion point of the last statement of an active clause in an #if
-///  must be the lookup parent
-/// of any following scopes. But the active clause may not be the last clause.
-/// In short, this is another case where the lookup parent cannot follow the same
-/// nesting as the source order. IfConfigUseScope implements this twist. It
-/// follows the IfConfig, wraps all subsequent scopes, and redirects the lookup.
-class LookupParentDiversionScope final : public ASTScopeImpl {
+/// A scope for the body of a guard statement. Lookups from the body must
+/// skip the parent scopes for introducing pattern bindings, since they're
+/// not visible in the guard body, only after the body ends.
+class GuardStmtBodyScope final : public ASTScopeImpl {
 public:
   ASTScopeImpl *const lookupParent;
-  const SourceLoc startLoc;
-  const SourceLoc endLoc;
+  BraceStmt *const body;
 
-  LookupParentDiversionScope(ASTScopeImpl *lookupParent,
-                             SourceLoc startLoc, SourceLoc endLoc)
-      : lookupParent(lookupParent), startLoc(startLoc), endLoc(endLoc) {}
+  GuardStmtBodyScope(ASTScopeImpl *lookupParent, BraceStmt *body)
+      : lookupParent(lookupParent), body(body) {}
 
   SourceRange
   getSourceRangeOfThisASTNode(bool omitAssertions = false) const override;
   std::string getClassName() const override;
 
+private:
+  void expandAScopeThatDoesNotCreateANewInsertionPoint(ScopeCreator &);
+
 protected:
   ASTScopeImpl *expandSpecifically(ScopeCreator &) override;
   NullablePtr<const ASTScopeImpl> getLookupParent() const override {

lib/AST/ASTScope.cpp

@@ -137,8 +137,8 @@ DEFINE_GET_CLASS_NAME(DefaultArgumentInitializerScope)
 DEFINE_GET_CLASS_NAME(AttachedPropertyWrapperScope)
 DEFINE_GET_CLASS_NAME(PatternEntryDeclScope)
 DEFINE_GET_CLASS_NAME(PatternEntryInitializerScope)
-DEFINE_GET_CLASS_NAME(ConditionalClauseScope)
 DEFINE_GET_CLASS_NAME(ConditionalClausePatternUseScope)
+DEFINE_GET_CLASS_NAME(ConditionalClauseInitializerScope)
 DEFINE_GET_CLASS_NAME(CaptureListScope)
 DEFINE_GET_CLASS_NAME(ClosureParametersScope)
 DEFINE_GET_CLASS_NAME(TopLevelCodeScope)
@@ -149,7 +149,7 @@ DEFINE_GET_CLASS_NAME(EnumElementScope)
 DEFINE_GET_CLASS_NAME(IfStmtScope)
 DEFINE_GET_CLASS_NAME(WhileStmtScope)
 DEFINE_GET_CLASS_NAME(GuardStmtScope)
-DEFINE_GET_CLASS_NAME(LookupParentDiversionScope)
+DEFINE_GET_CLASS_NAME(GuardStmtBodyScope)
 DEFINE_GET_CLASS_NAME(RepeatWhileScope)
 DEFINE_GET_CLASS_NAME(DoStmtScope)
 DEFINE_GET_CLASS_NAME(DoCatchStmtScope)
@@ -199,15 +199,6 @@ void ASTScopeImpl::postOrderDo(function_ref<void(ASTScopeImpl *)> fn) {
   fn(this);
 }
 
-ArrayRef<StmtConditionElement> ConditionalClauseScope::getCond() const {
-  return stmt->getCond();
-}
-
-const StmtConditionElement &
-ConditionalClauseScope::getStmtConditionElement() const {
-  return getCond()[index];
-}
-
 unsigned ASTScopeImpl::countDescendants() const {
   unsigned count = 0;
   const_cast<ASTScopeImpl *>(this)->preOrderDo(

lib/AST/ASTScopeCreation.cpp

@@ -687,6 +687,9 @@ class NodeAdder
 
   NullablePtr<ASTScopeImpl> visitBraceStmt(BraceStmt *bs, ASTScopeImpl *p,
                                            ScopeCreator &scopeCreator) {
+    if (bs->empty())
+      return p;
+
     SmallVector<ValueDecl *, 2> localFuncsAndTypes;
     SmallVector<VarDecl *, 2> localVars;
 
@@ -941,24 +944,26 @@ ASTScopeImpl *ASTScopeImpl::expandAndBeCurrent(ScopeCreator &scopeCreator) {
   ASTScopeImpl *Scope::expandSpecifically(ScopeCreator &) { return this; }
 
 CREATES_NEW_INSERTION_POINT(ASTSourceFileScope)
-CREATES_NEW_INSERTION_POINT(ConditionalClauseScope)
 CREATES_NEW_INSERTION_POINT(GuardStmtScope)
 CREATES_NEW_INSERTION_POINT(PatternEntryDeclScope)
 CREATES_NEW_INSERTION_POINT(GenericTypeOrExtensionScope)
 CREATES_NEW_INSERTION_POINT(BraceStmtScope)
 CREATES_NEW_INSERTION_POINT(TopLevelCodeScope)
+CREATES_NEW_INSERTION_POINT(ConditionalClausePatternUseScope)
 
 NO_NEW_INSERTION_POINT(FunctionBodyScope)
 NO_NEW_INSERTION_POINT(AbstractFunctionDeclScope)
 NO_NEW_INSERTION_POINT(AttachedPropertyWrapperScope)
 NO_NEW_INSERTION_POINT(EnumElementScope)
+NO_NEW_INSERTION_POINT(GuardStmtBodyScope)
 NO_NEW_INSERTION_POINT(ParameterListScope)
 NO_NEW_INSERTION_POINT(PatternEntryInitializerScope)
 
 NO_NEW_INSERTION_POINT(CaptureListScope)
 NO_NEW_INSERTION_POINT(CaseStmtScope)
 NO_NEW_INSERTION_POINT(CaseLabelItemScope)
 NO_NEW_INSERTION_POINT(CaseStmtBodyScope)
+NO_NEW_INSERTION_POINT(ConditionalClauseInitializerScope)
 NO_NEW_INSERTION_POINT(ClosureParametersScope)
 NO_NEW_INSERTION_POINT(DefaultArgumentInitializerScope)
 NO_NEW_INSERTION_POINT(DoStmtScope)
@@ -974,8 +979,6 @@ NO_NEW_INSERTION_POINT(WhileStmtScope)
 NO_EXPANSION(GenericParamScope)
 NO_EXPANSION(SpecializeAttributeScope)
 NO_EXPANSION(DifferentiableAttributeScope)
-NO_EXPANSION(ConditionalClausePatternUseScope)
-NO_EXPANSION(LookupParentDiversionScope)
 
 #undef CREATES_NEW_INSERTION_POINT
 #undef NO_NEW_INSERTION_POINT
@@ -1057,42 +1060,49 @@ PatternEntryInitializerScope::expandAScopeThatDoesNotCreateANewInsertionPoint(
                               this);
 }
 
+
 AnnotatedInsertionPoint
-ConditionalClauseScope::expandAScopeThatCreatesANewInsertionPoint(
+ConditionalClausePatternUseScope::expandAScopeThatCreatesANewInsertionPoint(
     ScopeCreator &scopeCreator) {
-  const StmtConditionElement &sec = getStmtConditionElement();
-  switch (sec.getKind()) {
-  case StmtConditionElement::CK_Availability:
-    return {this, "No introduced variables"};
-  case StmtConditionElement::CK_Boolean:
-    scopeCreator.addToScopeTree(sec.getBoolean(), this);
-    return {this, "No introduced variables"};
-  case StmtConditionElement::CK_PatternBinding:
-    scopeCreator.addToScopeTree(sec.getInitializer(), this);
-    auto *const ccPatternUseScope =
-        scopeCreator.constructExpandAndInsertUncheckable<
-            ConditionalClausePatternUseScope>(this, sec.getPattern(), endLoc);
-    return {ccPatternUseScope,
-            "Succeeding code must be in scope of conditional variables"};
-  }
-  ASTScope_unreachable("Unhandled StmtConditionKind in switch");
+  auto *initializer = sec.getInitializer();
+  if (!isa<ErrorExpr>(initializer)) {
+    scopeCreator
+      .constructExpandAndInsertUncheckable<ConditionalClauseInitializerScope>(
+        this, initializer);
+    }
+
+  return {this,
+          "Succeeding code must be in scope of conditional clause pattern bindings"};
+}
+
+void
+ConditionalClauseInitializerScope::expandAScopeThatDoesNotCreateANewInsertionPoint(
+    ScopeCreator &scopeCreator) {
+  scopeCreator.addToScopeTree(ASTNode(initializer), this);
+}
+
+void
+GuardStmtBodyScope::expandAScopeThatDoesNotCreateANewInsertionPoint(ScopeCreator &
+                                                                    scopeCreator) {
+  scopeCreator.addToScopeTree(ASTNode(body), this);
 }
 
 AnnotatedInsertionPoint
 GuardStmtScope::expandAScopeThatCreatesANewInsertionPoint(ScopeCreator &
                                                           scopeCreator) {
-
   ASTScopeImpl *conditionLookupParent =
-      createNestedConditionalClauseScopes(scopeCreator, stmt->getBody());
+      createNestedConditionalClauseScopes(scopeCreator, endLoc);
+
   // Add a child for the 'guard' body, which always exits.
-  // Parent is whole guard stmt scope, NOT the cond scopes
-  scopeCreator.addToScopeTree(stmt->getBody(), this);
+  // The lookup parent is whole guard stmt scope, NOT the cond scopes
+  auto *body = stmt->getBody();
+  if (!body->empty()) {
+    scopeCreator
+        .constructExpandAndInsertUncheckable<GuardStmtBodyScope>(
+            conditionLookupParent, this, stmt->getBody());
+  }
 
-  auto *const lookupParentDiversionScope =
-      scopeCreator
-          .constructExpandAndInsertUncheckable<LookupParentDiversionScope>(
-              this, conditionLookupParent, stmt->getEndLoc(), endLoc);
-  return {lookupParentDiversionScope,
+  return {conditionLookupParent,
           "Succeeding code must be in scope of guard variables"};
 }
 
@@ -1188,20 +1198,34 @@ void FunctionBodyScope::expandAScopeThatDoesNotCreateANewInsertionPoint(
 
 void IfStmtScope::expandAScopeThatDoesNotCreateANewInsertionPoint(
     ScopeCreator &scopeCreator) {
+  auto *thenStmt = stmt->getThenStmt();
+  auto *elseStmt = stmt->getElseStmt();
+
+  SourceLoc endLoc = thenStmt->getEndLoc();
   ASTScopeImpl *insertionPoint =
-      createNestedConditionalClauseScopes(scopeCreator, stmt->getThenStmt());
+      createNestedConditionalClauseScopes(scopeCreator, endLoc);
 
   // The 'then' branch
-  scopeCreator.addToScopeTree(stmt->getThenStmt(), insertionPoint);
+  scopeCreator.addToScopeTree(thenStmt, insertionPoint);
+
+  // Result builders can add an 'else' block consisting entirely of
+  // implicit expressions. In this case, the end location of the
+  // 'then' block is equal to the start location of the 'else'
+  // block, and the 'else' block source range is empty.
+  if (elseStmt &&
+      thenStmt->getEndLoc() == elseStmt->getStartLoc() &&
+      elseStmt->getStartLoc() == elseStmt->getEndLoc())
+    return;
 
   // Add the 'else' branch, if needed.
-  scopeCreator.addToScopeTree(stmt->getElseStmt(), this);
+  scopeCreator.addToScopeTree(elseStmt, this);
 }
 
 void WhileStmtScope::expandAScopeThatDoesNotCreateANewInsertionPoint(
     ScopeCreator &scopeCreator) {
+  SourceLoc endLoc = stmt->getBody()->getEndLoc();
   ASTScopeImpl *insertionPoint =
-      createNestedConditionalClauseScopes(scopeCreator, stmt->getBody());
+      createNestedConditionalClauseScopes(scopeCreator, endLoc);
   scopeCreator.addToScopeTree(stmt->getBody(), insertionPoint);
 }
 
@@ -1266,8 +1290,10 @@ void CaseStmtScope::expandAScopeThatDoesNotCreateANewInsertionPoint(
     }
   }
 
-  scopeCreator.constructExpandAndInsertUncheckable<CaseStmtBodyScope>(
-      this, stmt);
+  if (!stmt->getBody()->empty()) {
+    scopeCreator.constructExpandAndInsertUncheckable<CaseStmtBodyScope>(
+        this, stmt);
+  }
 }
 
 void CaseLabelItemScope::expandAScopeThatDoesNotCreateANewInsertionPoint(
@@ -1409,14 +1435,23 @@ TypeAliasScope::createTrailingWhereClauseScope(ASTScopeImpl *parent,
 #pragma mark misc
 
 ASTScopeImpl *LabeledConditionalStmtScope::createNestedConditionalClauseScopes(
-    ScopeCreator &scopeCreator, const Stmt *const afterConds) {
+    ScopeCreator &scopeCreator, SourceLoc endLoc) {
   auto *stmt = getLabeledConditionalStmt();
   ASTScopeImpl *insertionPoint = this;
-  for (unsigned i = 0; i < stmt->getCond().size(); ++i) {
-    insertionPoint =
-        scopeCreator
-            .constructExpandAndInsertUncheckable<ConditionalClauseScope>(
-                insertionPoint, stmt, i, afterConds->getStartLoc());
+  for (auto &sec : stmt->getCond()) {
+    switch (sec.getKind()) {
+    case StmtConditionElement::CK_Availability:
+      break;
+    case StmtConditionElement::CK_Boolean:
+      scopeCreator.addToScopeTree(sec.getBoolean(), insertionPoint);
+      break;
+    case StmtConditionElement::CK_PatternBinding:
+      insertionPoint =
+          scopeCreator.constructExpandAndInsertUncheckable<
+              ConditionalClausePatternUseScope>(
+                  insertionPoint, sec, endLoc);
+      break;
+    }
   }
   return insertionPoint;
 }