[Scope map] Miscellaneous improvements (and a hack) for unqualified name lookup

include/swift/AST/Decl.h

@@ -1709,7 +1709,10 @@ class PatternBindingEntry {
   void setInitContext(DeclContext *dc) { InitContext = dc; }
 
   /// Retrieve the source range covered by this pattern binding.
-  SourceRange getSourceRange() const;
+  ///
+  /// \param omitAccessors Whether the computation should omit the accessors
+  /// from the source range.
+  SourceRange getSourceRange(bool omitAccessors = false) const;
 };
 
 /// \brief This decl contains a pattern and optional initializer for a set

lib/AST/ASTScope.cpp

@@ -277,6 +277,18 @@ void ASTScope::expand() const {
     return stoleContinuation;
   };
 
+  // Local function to add the accessors of the variables in the given pattern
+  // as children.
+  auto addAccessors = [&](Pattern *pattern) {
+    // Create children for the accessors of any variables in the pattern that
+    // have them.
+    pattern->forEachVariable([&](VarDecl *var) {
+      if (hasAccessors(var)) {
+        addChild(new (ctx) ASTScope(this, var));
+      }
+    });
+  };
+
   if (!parentAndExpanded.getInt()) {
   // Expand the children in the current scope.
   switch (getKind()) {
@@ -365,39 +377,20 @@ void ASTScope::expand() const {
       patternBinding.decl->getPatternList()[patternBinding.entry];
 
     // Create a child for the initializer, if present.
-    ASTScope *initChild = nullptr;
     if (patternEntry.getInitAsWritten() &&
         patternEntry.getInitAsWritten()->getSourceRange().isValid()) {
-      initChild = new (ctx) ASTScope(ASTScopeKind::PatternInitializer, this,
-                                     patternBinding.decl, patternBinding.entry);
+      addChild(new (ctx) ASTScope(ASTScopeKind::PatternInitializer, this,
+                                  patternBinding.decl, patternBinding.entry));
     }
 
-    // Create children for the accessors of any variables in the pattern that
-    // have them.
-    patternEntry.getPattern()->forEachVariable([&](VarDecl *var) {
-      if (hasAccessors(var)) {
-        // If there is an initializer child that precedes this node (the
-        // normal case), add teh initializer child first.
-        if (initChild &&
-            ctx.SourceMgr.isBeforeInBuffer(
-                                 patternEntry.getInitAsWritten()->getEndLoc(),
-                                 var->getBracesRange().Start)) {
-          addChild(initChild);
-          initChild = nullptr;
-        }
-
-        addChild(new (ctx) ASTScope(this, var));
-      }
-    });
-
-    // If an initializer child remains, add it now.
-    if (initChild)
-      addChild(initChild);
-
     // If there is an active continuation, nest the remaining pattern bindings.
     if (getActiveContinuation()) {
+      // Note: the accessors will follow the pattern binding.
       addChild(new (ctx) ASTScope(ASTScopeKind::AfterPatternBinding, this,
                                   patternBinding.decl, patternBinding.entry));
+    } else {
+      // Otherwise, add the accessors immediately.
+      addAccessors(patternEntry.getPattern());
     }
     break;
   }
@@ -413,6 +406,12 @@ void ASTScope::expand() const {
   }
 
   case ASTScopeKind::AfterPatternBinding: {
+    const auto &patternEntry =
+      patternBinding.decl->getPatternList()[patternBinding.entry];
+
+    // Add accessors for the variables in this pattern.
+    addAccessors(patternEntry.getPattern());
+
     // Create a child for the next pattern binding.
     if (auto child = createIfNeeded(this, patternBinding.decl))
       addChild(child);
@@ -1504,10 +1503,6 @@ SourceRange ASTScope::getSourceRangeImpl() const {
     const auto &patternEntry =
       patternBinding.decl->getPatternList()[patternBinding.entry];
 
-    // We expect a continuation here.
-    assert(!patternBinding.decl->getDeclContext()->isLocalContext() ||
-           getHistoricalContinuation());
-
     return patternEntry.getSourceRange();
   }
 
@@ -1522,11 +1517,8 @@ SourceRange ASTScope::getSourceRangeImpl() const {
     const auto &patternEntry =
       patternBinding.decl->getPatternList()[patternBinding.entry];
 
-    // We expect a continuation here.
-    assert(getHistoricalContinuation());
-
-    // The scope of the binding begins at the end of the binding.
-    return SourceRange(patternEntry.getSourceRange().End);
+    return SourceRange(patternEntry.getSourceRange(/*omitAccessors*/true).End,
+                       patternEntry.getSourceRange().End);
   }
 
   case ASTScopeKind::BraceStmt:
@@ -1546,8 +1538,6 @@ SourceRange ASTScope::getSourceRangeImpl() const {
     // For a guard continuation, the scope extends from the end of the 'else'
     // to the end of the continuation.
     if (conditionalClause.isGuardContinuation) {
-      // We expect a continuation here.
-      assert(getHistoricalContinuation());
       const ASTScope *guard = this;
       do {
         guard = guard->getParent();

lib/AST/Decl.cpp

@@ -918,26 +918,20 @@ VarDecl *PatternBindingEntry::getAnchoringVarDecl() const {
   return variables[0];
 }
 
-SourceRange PatternBindingEntry::getSourceRange() const {
-  ASTContext *ctx = nullptr;
-
-  SourceLoc endLoc;
-  getPattern()->forEachVariable([&](VarDecl *var) {
-    auto accessorsEndLoc = var->getBracesRange().End;
-    if (accessorsEndLoc.isValid()) {
-      endLoc = accessorsEndLoc;
-      if (!ctx) ctx = &var->getASTContext();
-    }
-  });
-
-  // Check the initializer.
-  SourceLoc initEndLoc = getOrigInitRange().End;
-  if (initEndLoc.isValid() &&
-      (endLoc.isInvalid() ||
-       (ctx && ctx->SourceMgr.isBeforeInBuffer(endLoc, initEndLoc))))
-    endLoc = initEndLoc;
+SourceRange PatternBindingEntry::getSourceRange(bool omitAccessors) const {
+  // Patterns end at the initializer, if present.
+  SourceLoc endLoc = getOrigInitRange().End;
+
+  // If we're not banned from handling accessors, they follow the initializer.
+  if (!omitAccessors) {
+    getPattern()->forEachVariable([&](VarDecl *var) {
+      auto accessorsEndLoc = var->getBracesRange().End;
+      if (accessorsEndLoc.isValid())
+        endLoc = accessorsEndLoc;
+    });
+  }
 
-  // Check the pattern.
+  // If we didn't find an end yet, check the pattern.
   if (endLoc.isInvalid())
     endLoc = getPattern()->getEndLoc();
 

lib/AST/NameLookup.cpp

@@ -457,7 +457,9 @@ UnqualifiedLookup::UnqualifiedLookup(DeclName Name, DeclContext *DC,
 
   SmallVector<UnqualifiedLookupResult, 4> UnavailableInnerResults;
 
-  if (Loc.isValid() && Ctx.LangOpts.EnableASTScopeLookup) {
+  if (Loc.isValid() &&
+      DC->getParentSourceFile()->Kind != SourceFileKind::REPL &&
+      Ctx.LangOpts.EnableASTScopeLookup) {
     // Find the source file in which we are performing the lookup.
     SourceFile &sourceFile = *DC->getParentSourceFile();
 
@@ -599,6 +601,13 @@ UnqualifiedLookup::UnqualifiedLookup(DeclName Name, DeclContext *DC,
         // Dig out the type we're looking into.
         // FIXME: We shouldn't need to compute a type to perform this lookup.
         Type lookupType = dc->getSelfTypeInContext();
+
+        // FIXME: Hack to deal with missing 'Self' archetypes.
+        if (!lookupType) {
+          if (auto proto = dc->getAsProtocolOrProtocolExtensionContext())
+            lookupType = proto->getDeclaredType();
+        }
+
         if (!lookupType || lookupType->is<ErrorType>()) continue;
 
         // If we're performing a static lookup, use the metatype.

lib/Parse/ParsePattern.cpp

@@ -187,8 +187,7 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
       if (!hasSpecifier) {
         if (Tok.is(tok::kw_let)) {
           diagnose(Tok, diag::parameter_let_as_attr)
-          .fixItRemove(Tok.getLoc());
-          param.isInvalid = true;
+            .fixItRemove(Tok.getLoc());
         } else {
           // We handle the var error in sema for a better fixit and inout is
           // handled later in this function for better fixits.
@@ -201,9 +200,8 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
         // Redundant specifiers are fairly common, recognize, reject, and recover
         // from this gracefully.
         diagnose(Tok, diag::parameter_inout_var_let_repeated)
-        .fixItRemove(Tok.getLoc());
+          .fixItRemove(Tok.getLoc());
         consumeToken();
-        param.isInvalid = true;
       }
     }
 
@@ -252,9 +250,8 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
           hasValidInOut = true;
           if (hasSpecifier) {
             diagnose(Tok.getLoc(), diag::parameter_inout_var_let_repeated)
-            .fixItRemove(param.LetVarInOutLoc);
+              .fixItRemove(param.LetVarInOutLoc);
             consumeToken(tok::kw_inout);
-            param.isInvalid = true;
           } else {
             hasSpecifier = true;
             param.LetVarInOutLoc = consumeToken(tok::kw_inout);
@@ -263,9 +260,8 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
         }
         if (!hasValidInOut && hasDeprecatedInOut) {
           diagnose(Tok.getLoc(), diag::inout_as_attr_disallowed)
-          .fixItRemove(param.LetVarInOutLoc)
-          .fixItInsert(postColonLoc, "inout ");
-          param.isInvalid = true;
+            .fixItRemove(param.LetVarInOutLoc)
+            .fixItInsert(postColonLoc, "inout ");
         }
 
         auto type = parseType(diag::expected_parameter_type);
@@ -322,14 +318,13 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
         if (param.Type) {
           diagnose(typeStartLoc, diag::parameter_unnamed)
             .fixItInsert(typeStartLoc, "_: ");
-        } else {
-          param.isInvalid = true;
         }
       } else {
         // Otherwise, we're not sure what is going on, but this doesn't smell
         // like a parameter.
         diagnose(Tok, diag::expected_parameter_name);
         param.isInvalid = true;
+        param.FirstNameLoc = Tok.getLoc();
       }
     }
 

lib/Sema/CodeSynthesis.cpp

@@ -317,7 +317,7 @@ void swift::convertStoredVarInProtocolToComputed(VarDecl *VD, TypeChecker &TC) {
   auto *Get = createGetterPrototype(VD, TC);
 
   // Okay, we have both the getter and setter.  Set them in VD.
-  VD->makeComputed(VD->getLoc(), Get, nullptr, nullptr, VD->getLoc());
+  VD->makeComputed(SourceLoc(), Get, nullptr, nullptr, SourceLoc());
 
   // We've added some members to our containing class, add them to the members
   // list.
@@ -983,7 +983,7 @@ static void convertNSManagedStoredVarToComputed(VarDecl *VD, TypeChecker &TC) {
   if (VD->hasAccessorFunctions()) return;
 
   // Okay, we have both the getter and setter.  Set them in VD.
-  VD->makeComputed(VD->getLoc(), Get, Set, nullptr, VD->getLoc());
+  VD->makeComputed(SourceLoc(), Get, Set, nullptr, SourceLoc());
 
   TC.validateDecl(Get);
   TC.validateDecl(Set);
@@ -1736,7 +1736,7 @@ void swift::maybeAddAccessorsToVariable(VarDecl *var, TypeChecker &TC) {
         setter->setAccessibility(var->getFormalAccess());
       }
 
-      var->makeComputed(var->getLoc(), getter, setter, nullptr, var->getLoc());
+      var->makeComputed(SourceLoc(), getter, setter, nullptr, SourceLoc());
 
       // Save the conformance and 'value' decl for later type checking.
       behavior->Conformance = conformance;
@@ -1841,8 +1841,7 @@ void swift::maybeAddAccessorsToVariable(VarDecl *var, TypeChecker &TC) {
 
     ParamDecl *newValueParam = nullptr;
     auto *setter = createSetterPrototype(var, newValueParam, TC);
-    var->makeComputed(var->getLoc(), getter, setter, nullptr,
-                      var->getLoc());
+    var->makeComputed(SourceLoc(), getter, setter, nullptr, SourceLoc());
     var->setIsBeingTypeChecked(false);
 
     TC.validateDecl(getter);

lib/Sema/TypeCheckConstraints.cpp

@@ -472,8 +472,14 @@ resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE, DeclContext *DC) {
     }
 
     ValueDecl *D = Result.Decl;
-    if (!D->hasType()) {
-      assert(D->getDeclContext()->isLocalContext());
+    if (!D->hasType()) validateDecl(D);
+
+    // FIXME: The source-location checks won't make sense once
+    // EnableASTScopeLookup is the default.
+    if (Loc.isValid() && D->getLoc().isValid() &&
+        D->getDeclContext()->isLocalContext() &&
+        D->getDeclContext() == DC &&
+        Context.SourceMgr.isBeforeInBuffer(Loc, D->getLoc())) {
       if (!D->isInvalid()) {
         diagnose(Loc, diag::use_local_before_declaration, Name);
         diagnose(D, diag::decl_declared_here, Name);

test/NameBinding/scope_map.swift

@@ -366,13 +366,14 @@ class LazyProperties {
 
 // CHECK-EXPANDED: {{^}}  `-AbstractFunctionParams {{.*}} funcWithComputedProperties(i:) param 0:0 [142:36 - 155:1] expanded
 // CHECK-EXPANDED: {{^}}  `-BraceStmt {{.*}} [142:41 - 155:1] expanded
-// CHECK-EXPANDED: {{^}}      |-Accessors {{.*}} scope_map.(file).func decl.computed@{{.*}}scope_map.swift:143:7 [143:21 - 149:3] expanded
+// CHECK-EXPANDED-NEXT: {{^}} `-PatternBinding {{.*}} entry 0 [143:7 - 155:1] expanded
+// CHECK-EXPANDED-NEXT: {{^}}    `-AfterPatternBinding {{.*}} entry 0 [143:17 - 155:1] expanded
+// CHECK-EXPANDED-NEXT: {{^}}      |-Accessors {{.*}} scope_map.(file).func decl.computed@{{.*}}scope_map.swift:143:7 [143:21 - 149:3] expanded
 // CHECK-EXPANDED-NEXT: {{^}}       |-AbstractFunctionDecl {{.*}} _ [144:5 - 145:5] expanded
 // CHECK-EXPANDED-NEXT: {{^}}        `-AbstractFunctionParams {{.*}} _ param 0:0 [144:5 - 145:5] expanded
 // CHECK-EXPANDED: {{^}}          `-BraceStmt {{.*}} [144:9 - 145:5] expanded
 // CHECK-EXPANDED-NEXT: {{^}}    `-AbstractFunctionDecl {{.*}} _ [146:5 - 148:5] expanded
 // CHECK-EXPANDED: {{^}}        `-BraceStmt {{.*}} [146:9 - 148:5] expanded
-// CHECK-EXPANDED: {{^}}    `-AfterPatternBinding {{.*}} entry 0 [149:3 - 155:1] expanded
 // CHECK-EXPANDED: {{^}}      `-AfterPatternBinding {{.*}} entry 1 [149:36 - 155:1] expanded
 // CHECK-EXPANDED: {{^}}        `-AfterPatternBinding {{.*}} entry 2 [150:21 - 155:1] expanded
 // CHECK-EXPANDED-NEXT: {{^}}           `-AbstractFunctionDecl {{.*}} _ [150:25 - 155:1] expanded
@@ -493,6 +494,5 @@ class LazyProperties {
 // CHECK-SEARCHES: -TypeOrExtensionBody {{.*}} 'LazyProperties' [190:22 - 194:1] expanded
 // CHECK-SEARCHES-NEXT:   |-PatternBinding {{.*}} entry 0 [191:7 - 191:20] unexpanded
 // CHECK-SEARCHES-NEXT:   `-PatternBinding {{.*}} entry 0 [193:12 - 193:29] expanded
-// CHECK-SEARCHES-NEXT:     |-Accessors {{.*}} scope_map.(file).LazyProperties.prop@{{.*}}scope_map.swift:193:12 [193:12 - 193:12] unexpanded
 // CHECK-SEARCHES-NEXT:     `-PatternInitializer {{.*}} entry 0 [193:24 - 193:29] expanded
 // CHECK-SEARCHES-NOT: {{ expanded}}

test/NameBinding/scope_map_lookup.swift

@@ -22,6 +22,11 @@ protocol P1 {
   associatedtype A = Self
 }
 
+// Protocols involving associated types.
+protocol AProtocol {
+  associatedtype e : e  // expected-error {{inheritance from non-protocol, non-class type 'Self.e'}}
+}
+
 // Extensions.
 protocol P2 {
 }
@@ -78,6 +83,19 @@ extension PConstrained4 where Self : Superclass {
   }
 }
 
+// Local computed properties.
+func localComputedProperties() {
+  var localProperty: Int {
+    get {
+      return localProperty // expected-warning{{attempting to access 'localProperty' within its own getter}}
+    }
+    set {
+      print(localProperty)
+    }
+  }
+  { print(localProperty) }()
+}
+
 // Top-level code.
 func topLevel() { }
 
@@ -91,3 +109,7 @@ protocol Fooable {
 
   var foo: Foo { get }
 }
+
+let a = b ; let b = a // expected-error{{could not infer type for 'a'}} 
+// expected-error@-1 {{'a' used within its own type}}
+// FIXME: That second error is bogus.

test/expr/capture/order.swift

@@ -78,11 +78,14 @@ func outOfOrderEnum() {
 }
 
 func captureInClosure() {
-  var x = { (i: Int) in 
-    currentTotal += i // expected-error{{use of local variable 'currentTotal' before its declaration}}
+  let x = { (i: Int) in
+    currentTotal += i // expected-error{{cannot capture 'currentTotal' before it is declared}}
   }
 
   var currentTotal = 0 // expected-note{{'currentTotal' declared here}}
+
+  _ = x
+  currentTotal += 1
 }
 
 class X {