Merge pull request #2992 from slavapestov/protocol-typealias-fixes

Author: swift-ci

include/swift/AST/LookupKinds.h

@@ -62,6 +62,9 @@ enum NLOptions : unsigned {
   /// \see NL_KnownDependencyMask
   NL_KnownCascadingDependency = 0x80,
 
+  /// This lookup should only return type declarations.
+  NL_OnlyTypes = 0x100,
+
   /// This lookup is known to not add any additional dependencies to the
   /// primary source file.
   ///

include/swift/AST/NameLookup.h

@@ -194,11 +194,18 @@ class NamedDeclConsumer : public VisibleDeclConsumer {
 public:
   DeclName name;
   SmallVectorImpl<UnqualifiedLookupResult> &results;
+  bool isTypeLookup;
+
   NamedDeclConsumer(DeclName name,
-                    SmallVectorImpl<UnqualifiedLookupResult> &results)
-    : name(name), results(results) {}
+                    SmallVectorImpl<UnqualifiedLookupResult> &results,
+                    bool isTypeLookup)
+    : name(name), results(results), isTypeLookup(isTypeLookup) {}
 
   virtual void foundDecl(ValueDecl *VD, DeclVisibilityKind Reason) override {
+    // Give clients an opportunity to filter out non-type declarations early,
+    // to avoid circular validation.
+    if (isTypeLookup && !isa<TypeDecl>(VD))
+      return;
     if (VD->getFullName().matchesRef(name))
       results.push_back(UnqualifiedLookupResult(VD));
   }

lib/AST/Decl.cpp

@@ -286,8 +286,8 @@ llvm::raw_ostream &swift::operator<<(llvm::raw_ostream &OS,
 DeclContext *Decl::getInnermostDeclContext() const {
   if (auto func = dyn_cast<AbstractFunctionDecl>(this))
     return const_cast<AbstractFunctionDecl*>(func);
-  if (auto nominal = dyn_cast<NominalTypeDecl>(this))
-    return const_cast<NominalTypeDecl*>(nominal);
+  if (auto nominal = dyn_cast<GenericTypeDecl>(this))
+    return const_cast<GenericTypeDecl*>(nominal);
   if (auto ext = dyn_cast<ExtensionDecl>(this))
     return const_cast<ExtensionDecl*>(ext);
   if (auto topLevel = dyn_cast<TopLevelCodeDecl>(this))

lib/AST/NameLookup.cpp

@@ -385,7 +385,7 @@ UnqualifiedLookup::UnqualifiedLookup(DeclName Name, DeclContext *DC,
   const SourceManager &SM = Ctx.SourceMgr;
   DebuggerClient *DebugClient = M.getDebugClient();
 
-  NamedDeclConsumer Consumer(Name, Results);
+  NamedDeclConsumer Consumer(Name, Results, IsTypeLookup);
 
   Optional<bool> isCascadingUse;
   if (IsKnownNonCascading)
@@ -530,6 +530,8 @@ UnqualifiedLookup::UnqualifiedLookup(DeclName Name, DeclContext *DC,
 
         if (AllowProtocolMembers)
           options |= NL_ProtocolMembers;
+        if (IsTypeLookup)
+          options |= NL_OnlyTypes;
 
         if (!ExtendedType)
           ExtendedType = ErrorType::get(Ctx);
@@ -1262,6 +1264,11 @@ bool DeclContext::lookupQualified(Type type,
     // Look for results within the current nominal type and its extensions.
     bool currentIsProtocol = isa<ProtocolDecl>(current);
     for (auto decl : current->lookupDirect(member)) {
+      // If we're performing a type lookup, don't even attempt to validate
+      // the decl if its not a type.
+      if ((options & NL_OnlyTypes) && !isa<TypeDecl>(decl))
+        continue;
+
       // Resolve the declaration signature when we find the
       // declaration.
       if (typeResolver && !decl->isBeingTypeChecked()) {

lib/AST/Type.cpp

@@ -2917,15 +2917,8 @@ TypeSubstitutionMap TypeBase::getMemberSubstitutions(const DeclContext *dc) {
   // If the member is part of a protocol or extension thereof, we need
   // to substitute in the type of Self.
   if (dc->getAsProtocolOrProtocolExtensionContext()) {
-    // We only substitute into archetypes for now for protocols.
-    // FIXME: This seems like an odd restriction. Whatever is depending on
-    // this, shouldn't.
-    if (!baseTy->is<ArchetypeType>() && isa<ProtocolDecl>(dc))
-      return substitutions;
-
     // FIXME: This feels painfully inefficient. We're creating a dense map
     // for a single substitution.
-    substitutions[dc->getProtocolSelf()->getArchetype()] = baseTy;
     substitutions[dc->getProtocolSelf()->getDeclaredType()
                     ->getCanonicalType()->castTo<GenericTypeParamType>()]
       = baseTy;

lib/Sema/TypeCheckConstraints.cpp

@@ -1846,11 +1846,11 @@ bool TypeChecker::typeCheckForEachBinding(DeclContext *dc, ForEachStmt *stmt) {
       cs.addConstraint(ConstraintKind::ConformsTo, SequenceType,
                        sequenceProto->getDeclaredType(), Locator);
 
-      auto generatorLocator =
+      auto iteratorLocator =
         cs.getConstraintLocator(Locator,
                                 ConstraintLocator::SequenceIteratorProtocol);
       auto elementLocator =
-        cs.getConstraintLocator(generatorLocator,
+        cs.getConstraintLocator(iteratorLocator,
                                 ConstraintLocator::GeneratorElementType);
 
       // Collect constraints from the element pattern.
@@ -1859,48 +1859,58 @@ bool TypeChecker::typeCheckForEachBinding(DeclContext *dc, ForEachStmt *stmt) {
       if (!InitType)
         return true;
 
-      // Manually search for the generator witness. If no generator/element pair
+      // Manually search for the iterator witness. If no iterator/element pair
       // exists, solve for them.
-      // FIXME: rename generatorType to iteratorType due to the protocol
-      // renaming
-      Type generatorType;
+      Type iteratorType;
       Type elementType;
 
       NameLookupOptions lookupOptions = defaultMemberTypeLookupOptions;
       if (isa<AbstractFunctionDecl>(cs.DC))
         lookupOptions |= NameLookupFlags::KnownPrivate;
-      auto member = cs.TC.lookupMemberType(cs.DC,
-                                           expr->getType()->getRValueType(),
-                                           tc.Context.Id_Iterator,
-                                           lookupOptions);
+
+      auto sequenceType = expr->getType()->getRValueType();
+
+      // If the sequence type is an existential, we should not attempt to
+      // look up the member type at all, since we cannot represent associated
+      // types of existentials.
+      //
+      // We will diagnose it later.
+      if (!sequenceType->isExistentialType()) {
+        auto member = cs.TC.lookupMemberType(cs.DC,
+                                             sequenceType,
+                                             tc.Context.Id_Iterator,
+                                             lookupOptions);
 
-      if (member) {
-        generatorType = member.front().second;
-        
-        member = cs.TC.lookupMemberType(cs.DC,
-                                        generatorType,
-                                        tc.Context.Id_Element,
-                                        lookupOptions);
-        
-        if (member)
-          elementType = member.front().second;
+        if (member) {
+          iteratorType = member.front().second;
+
+          member = cs.TC.lookupMemberType(cs.DC,
+                                          iteratorType,
+                                          tc.Context.Id_Element,
+                                          lookupOptions);
+
+          if (member)
+            elementType = member.front().second;
+        }
       }
-      
+
+      // If the type lookup failed, just add some constraints we can
+      // try to solve later.
       if (elementType.isNull()) {
 
-        // Determine the generator type of the sequence.
-        generatorType = cs.createTypeVariable(Locator, /*options=*/0);
+        // Determine the iterator type of the sequence.
+        iteratorType = cs.createTypeVariable(Locator, /*options=*/0);
         cs.addConstraint(Constraint::create(
                            cs, ConstraintKind::TypeMember,
-                           SequenceType, generatorType,
-                           tc.Context.Id_Iterator, generatorLocator));
+                           SequenceType, iteratorType,
+                           tc.Context.Id_Iterator, iteratorLocator));
 
-        // Determine the element type of the generator.
+        // Determine the element type of the iterator.
         // FIXME: Should look up the type witness.
         elementType = cs.createTypeVariable(Locator, /*options=*/0);
         cs.addConstraint(Constraint::create(
                            cs, ConstraintKind::TypeMember,
-                           generatorType, elementType,
+                           iteratorType, elementType,
                            tc.Context.Id_Element, elementLocator));
       }
 

lib/Sema/TypeCheckNameLookup.cpp

@@ -104,9 +104,9 @@ namespace {
     /// \param foundInType The type through which we found the
     /// declaration.
     void add(ValueDecl *found, ValueDecl *base, Type foundInType) {
-      // If we only want types and we didn't get one, bail out.
-      if (Options.contains(NameLookupFlags::OnlyTypes) && !isa<TypeDecl>(found))
-        return;
+      // If we only want types, AST name lookup should not yield anything else.
+      assert(!Options.contains(NameLookupFlags::OnlyTypes) ||
+             isa<TypeDecl>(found));
 
       ConformanceCheckOptions conformanceOptions;
       if (Options.contains(NameLookupFlags::KnownPrivate))
@@ -245,6 +245,8 @@ LookupResult TypeChecker::lookupMember(DeclContext *dc,
     subOptions |= NL_DynamicLookup;
   if (options.contains(NameLookupFlags::IgnoreAccessibility))
     subOptions |= NL_IgnoreAccessibility;
+  if (options.contains(NameLookupFlags::OnlyTypes))
+    subOptions |= NL_OnlyTypes;
 
   // Dig out the type that we'll actually be looking into, and determine
   // whether it is a nominal type.
@@ -351,37 +353,54 @@ LookupTypeResult TypeChecker::lookupMemberType(DeclContext *dc,
     validateDecl(typeDecl);
     if (!typeDecl->hasType()) // FIXME: recursion-breaking hack
       continue;
-    
-    // If we found a member of a protocol type when looking into a non-protocol,
-    // non-archetype type, only include this member in the result set if
-    // this member was used as the default definition or otherwise inferred.
-    if (auto assocType = dyn_cast<AssociatedTypeDecl>(typeDecl)) {
-      if (!type->is<ArchetypeType>() && !type->isExistentialType()) {
-        inferredAssociatedTypes.push_back(assocType);
-        continue;
+
+    // If we're looking up a member of a protocol, we must take special care.
+    if (typeDecl->getDeclContext()->getAsProtocolOrProtocolExtensionContext()) {
+      // We don't allow lookups of an associated type or typealias of an
+      // existential type, because we have no way to represent such types.
+      //
+      // This is diagnosed further on down in resolveNestedIdentTypeComponent().
+      if (type->isExistentialType()) {
+        auto memberType = typeDecl->getInterfaceType()->getRValueInstanceType();
+
+        if (memberType->hasTypeParameter()) {
+          // If we haven't seen this type result yet, add it to the result set.
+          if (types.insert(memberType->getCanonicalType()).second)
+            result.Results.push_back({typeDecl, memberType});
+
+          continue;
+        }
       }
-    }
 
-    // Substitute the base into the member's type.
-    if (Type memberType = substMemberTypeWithBase(dc->getParentModule(),
-                                                  typeDecl, type,
-                                                  /*isTypeReference=*/true)) {
-
-      // Similar to the associated type case, ignore typealiases containing
-      // associated types when looking into a non-protocol.
-      if (auto alias = dyn_cast<TypeAliasDecl>(typeDecl)) {
-        auto parentProtocol = alias->getDeclContext()->
-          getAsProtocolOrProtocolExtensionContext();
-        if (parentProtocol && memberType->hasTypeParameter() &&
-            !type->is<ArchetypeType>() && !type->isExistentialType()) {
+      // If we're looking up an associated type of a concrete type,
+      // record it later for conformance checking; we might find a more
+      // direct typealias with the same name later.
+      if (auto assocType = dyn_cast<AssociatedTypeDecl>(typeDecl)) {
+        if (!type->is<ArchetypeType>()) {
+          inferredAssociatedTypes.push_back(assocType);
           continue;
         }
       }
 
-      // If we haven't seen this type result yet, add it to the result set.
-      if (types.insert(memberType->getCanonicalType()).second)
-        result.Results.push_back({typeDecl, memberType});
+      // We are looking up an associated type of an archetype, or a
+      // protocol typealias or an archetype or concrete type.
+      //
+      // Proceed with the usual path below.
     }
+
+    // Substitute the base into the member's type.
+    auto memberType = substMemberTypeWithBase(dc->getParentModule(),
+                                              typeDecl, type,
+                                              /*isTypeReference=*/true);
+
+    // FIXME: It is not clear why this substitution can fail, but the
+    // standard library won't build without this check.
+    if (!memberType)
+      continue;
+
+    // If we haven't seen this type result yet, add it to the result set.
+    if (types.insert(memberType->getCanonicalType()).second)
+      result.Results.push_back({typeDecl, memberType});
   }
 
   if (result.Results.empty()) {