Sema: Tighten up name lookup routines to not look through metatypes/lvalues/etc

Previously all of the following would strip off varying amounts of
MetatypeType, LValueType, InOutType, DynamicSelfType, etc:

- ConstraintSystem::performMemberLookup()
- ConstraintSystem::lookupMember()
- TypeChecker::lookupMember()
- DeclContext::lookupQualified()
- Type::getContextSubstitutions()

The problem is that the higher level methods that took a lookup type
would call the lower level methods, and post-process the result using
the given lookup type. Since different levels of sugar were stripped,
it made the code hard to reason about and opened up edge cases, eg
if a DynamicSelfType or InOutType appears where we didn't expect it.

Since filtering out static/instance and mutating/nonmutating members
is done at higher levels, there's no reason for these name lookup
operations to accept anything other than nominal types, existentials
and archetypes.

Make this so with assertions, and deal with the fallout.

Author: slavapestov

include/swift/AST/Types.h

@@ -648,9 +648,17 @@ class alignas(1 << TypeAlignInBits) TypeBase {
   /// with a class type.
   bool mayHaveSuperclass();
 
-  /// \brief Determine whether this type may have nested types, which holds for
-  /// nominal types, existentials and archetypes.
-  bool mayHaveMemberTypes() {
+  /// \brief Determine whether this type can be used as a base type for AST
+  /// name lookup, which is the case for nominal types, protocol compositions
+  /// and archetypes.
+  ///
+  /// Generally, the static vs instanec and mutating vs nonmutating distinction
+  /// is handled elsewhere, so metatypes, lvalue types and inout types are not
+  /// allowed here.
+  ///
+  /// Similarly, tuples formally have members, but this does not go through
+  /// name lookup.
+  bool mayHaveMembers() {
     return (is<ArchetypeType>() ||
             is<ModuleType>() ||
             isExistentialType() ||

lib/AST/ASTPrinter.cpp

@@ -224,7 +224,7 @@ struct SynthesizedExtensionAnalyzer::Implementation {
     // the extension to the interface types of the base type's
     // declaration.
     TypeSubstitutionMap subMap;
-    if (!BaseType->isAnyExistentialType())
+    if (!BaseType->isExistentialType())
       subMap = BaseType->getContextSubstitutions(Ext);
     auto *M = DC->getParentModule();
 
@@ -453,7 +453,9 @@ void PrintOptions::clearSynthesizedExtension() {
 }
 
 TypeTransformContext::TypeTransformContext(Type T)
-    : BaseType(T.getPointer()) {}
+    : BaseType(T.getPointer()) {
+  assert(T->mayHaveMembers());
+}
 
 TypeTransformContext::TypeTransformContext(NominalTypeDecl *NTD)
     : BaseType(NTD->getDeclaredTypeInContext().getPointer()), Nominal(NTD) {}
@@ -984,8 +986,9 @@ class PrintAST : public ASTVisitor<PrintAST> {
     Type OldType = CurrentType;
     if (CurrentType && (Old != nullptr || Options.PrintAsMember)) {
       if (auto *NTD = dyn_cast<NominalTypeDecl>(D)) {
-        CurrentType = CurrentType->getTypeOfMember(
-            Options.CurrentModule, NTD, nullptr);
+        CurrentType = NTD->getDeclaredInterfaceType().subst(
+            Options.CurrentModule,
+            CurrentType->getContextSubstitutions(NTD->getDeclContext()));
       }
     }
 
@@ -1259,7 +1262,7 @@ void PrintAST::printSingleDepthOfGenericSignature(
   ModuleDecl *M = nullptr;
 
   if (CurrentType) {
-    if (!CurrentType->isAnyExistentialType()) {
+    if (!CurrentType->isExistentialType()) {
       auto *DC = Current->getInnermostDeclContext()->getInnermostTypeContext();
       subMap = CurrentType->getContextSubstitutions(DC);
       M = DC->getParentModule();

lib/AST/NameLookup.cpp

@@ -1316,17 +1316,6 @@ bool DeclContext::lookupQualified(Type type,
     }
   };
 
-  // Look through lvalue and inout types.
-  type = type->getLValueOrInOutObjectType();
-
-  // Look through metatypes.
-  if (auto metaTy = type->getAs<AnyMetatypeType>())
-    type = metaTy->getInstanceType();
-
-  // Look through DynamicSelf.
-  if (auto dynamicSelf = type->getAs<DynamicSelfType>())
-    type = dynamicSelf->getSelfType();
-
   // Look for module references.
   if (auto moduleTy = type->getAs<ModuleType>()) {
     Module *module = moduleTy->getModule();
@@ -1438,6 +1427,8 @@ bool DeclContext::lookupQualified(Type type,
         }
       }
     }
+  } else {
+    llvm_unreachable("Bad type for qualified lookup");
   }
 
   // Allow filtering of the visible declarations based on various

lib/AST/Type.cpp

@@ -3099,23 +3099,15 @@ Type TypeBase::getSuperclassForDecl(const ClassDecl *baseClass,
 }
 
 TypeSubstitutionMap TypeBase::getContextSubstitutions(const DeclContext *dc) {
+  assert(dc->isTypeContext());
+  Type baseTy(this);
 
-  // Ignore lvalues in the base type.
-  Type baseTy(getRValueType());
-
-  // Look through the metatype; it has no bearing on the result.
-  if (auto metaBase = baseTy->getAs<AnyMetatypeType>()) {
-    baseTy = metaBase->getInstanceType()->getRValueType();
-  }
+  assert(!baseTy->isLValueType() && !baseTy->is<AnyMetatypeType>());
 
   // The resulting set of substitutions. Always use this to ensure we
   // don't miss out on NRVO anywhere.
   TypeSubstitutionMap substitutions;
 
-  // Look through non-type contexts.
-  while (!dc->isTypeContext())
-    dc = dc->getParent();
-
   // If the member is part of a protocol or extension thereof, we need
   // to substitute in the type of Self.
   if (dc->getAsProtocolOrProtocolExtensionContext()) {
@@ -3127,19 +3119,24 @@ TypeSubstitutionMap TypeBase::getContextSubstitutions(const DeclContext *dc) {
     return substitutions;
   }
 
+  // If we found a member of a concrete type from a protocol extension,
+  // get the superclass out of the archetype.
+  if (auto *archetypeTy = baseTy->getAs<ArchetypeType>())
+    baseTy = archetypeTy->getSuperclass();
+
   // Extract the lazy resolver.
   LazyResolver *resolver = dc->getASTContext().getLazyResolver();
 
   // Find the superclass type with the context matching that of the member.
   //
   // FIXME: Do this in the caller?
-  if (baseTy->getAnyNominal()) {
-    auto *ownerNominal = dc->getAsNominalTypeOrNominalTypeExtensionContext();
-    if (auto *ownerClass = dyn_cast<ClassDecl>(ownerNominal))
-      baseTy = baseTy->getSuperclassForDecl(ownerClass, resolver);
+  assert(baseTy->getAnyNominal());
 
-    assert(ownerNominal == baseTy->getAnyNominal());
-  }
+  auto *ownerNominal = dc->getAsNominalTypeOrNominalTypeExtensionContext();
+  if (auto *ownerClass = dyn_cast<ClassDecl>(ownerNominal))
+    baseTy = baseTy->getSuperclassForDecl(ownerClass, resolver);
+
+  assert(ownerNominal == baseTy->getAnyNominal());
 
   // If the base type isn't specialized, there's nothing to substitute.
   if (!baseTy->isSpecialized())
@@ -3177,8 +3174,7 @@ TypeSubstitutionMap TypeBase::getContextSubstitutions(const DeclContext *dc) {
       continue;
     }
 
-    // We're done.
-    break;
+    llvm_unreachable("Bad base type");
   }
 
   return substitutions;

lib/IDE/CodeCompletion.cpp

@@ -1975,8 +1975,13 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
       Type ContextTy = VD->getDeclContext()->getDeclaredInterfaceType();
       if (ContextTy) {
         // Look through lvalue types and metatypes
-        Type MaybeNominalType = (*ExprType)->getRValueType()
-            ->getRValueInstanceType();
+        Type MaybeNominalType = (*ExprType)->getRValueType();
+
+        if (auto Metatype = MaybeNominalType->getAs<MetatypeType>())
+          MaybeNominalType = Metatype->getInstanceType();
+
+        if (auto SelfType = MaybeNominalType->getAs<DynamicSelfType>())
+          MaybeNominalType = SelfType->getSelfType();
 
         // For optional protocol requirements and dynamic dispatch,
         // strip off optionality from the base type, but only if
@@ -1989,6 +1994,12 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
         if (MaybeNominalType->isAnyObject())
           return T;
 
+        // FIXME: Sometimes ExprType is the type of the member here,
+        // and not the type of the base. That is inconsistent and
+        // should be cleaned up.
+        if (!MaybeNominalType->mayHaveMembers())
+          return T;
+
         // For everything else, substitute in the base type.
         auto Subs = MaybeNominalType->getMemberSubstitutions(VD);
 
@@ -2865,23 +2876,24 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
 
       if (auto *NTD = dyn_cast<NominalTypeDecl>(D)) {
         addNominalTypeRef(NTD, Reason);
-        addConstructorCallsForType(NTD->getInterfaceType(), NTD->getName(),
-                                   Reason);
+        addConstructorCallsForType(NTD->getDeclaredInterfaceType(),
+                                   NTD->getName(), Reason);
         return;
       }
 
       if (auto *TAD = dyn_cast<TypeAliasDecl>(D)) {
         addTypeAliasRef(TAD, Reason);
         auto type = TAD->mapTypeIntoContext(TAD->getUnderlyingTypeLoc().getType());
-        addConstructorCallsForType(type, TAD->getName(), Reason);
+        if (type->mayHaveMembers())
+          addConstructorCallsForType(type, TAD->getName(), Reason);
         return;
       }
 
       if (auto *GP = dyn_cast<GenericTypeParamDecl>(D)) {
         addGenericTypeParamRef(GP, Reason);
         for (auto *protocol : GP->getConformingProtocols())
-          addConstructorCallsForType(protocol->getInterfaceType(), GP->getName(),
-                                     Reason);
+          addConstructorCallsForType(protocol->getDeclaredInterfaceType(),
+                                     GP->getName(), Reason);
         return;
       }
 
@@ -2934,23 +2946,24 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
 
       if (auto *NTD = dyn_cast<NominalTypeDecl>(D)) {
         addNominalTypeRef(NTD, Reason);
-        addConstructorCallsForType(NTD->getInterfaceType(), NTD->getName(),
-                                   Reason);
+        addConstructorCallsForType(NTD->getDeclaredInterfaceType(),
+                                   NTD->getName(), Reason);
         return;
       }
 
       if (auto *TAD = dyn_cast<TypeAliasDecl>(D)) {
         addTypeAliasRef(TAD, Reason);
         auto type = TAD->mapTypeIntoContext(TAD->getDeclaredInterfaceType());
-        addConstructorCallsForType(type, TAD->getName(), Reason);
+        if (type->mayHaveMembers())
+          addConstructorCallsForType(type, TAD->getName(), Reason);
         return;
       }
 
       if (auto *GP = dyn_cast<GenericTypeParamDecl>(D)) {
         addGenericTypeParamRef(GP, Reason);
         for (auto *protocol : GP->getConformingProtocols())
-          addConstructorCallsForType(protocol->getInterfaceType(), GP->getName(),
-                                     Reason);
+          addConstructorCallsForType(protocol->getDeclaredInterfaceType(),
+                                     GP->getName(), Reason);
         return;
       }
 

lib/SILGen/SILGenMaterializeForSet.cpp

@@ -385,7 +385,8 @@ struct MaterializeForSetEmitter {
   /// Given part of the witness's interface type, produce its
   /// substitution according to the witness substitutions.
   CanType getSubstWitnessInterfaceType(CanType type) {
-    auto subs = SubstSelfType->getMemberSubstitutions(WitnessStorage);
+    auto subs = SubstSelfType->getRValueInstanceType()
+        ->getMemberSubstitutions(WitnessStorage);
     return type.subst(SGM.SwiftModule, subs)->getCanonicalType();
   }
 

lib/Sema/CSDiag.cpp

@@ -1606,6 +1606,9 @@ void CalleeCandidateInfo::collectCalleeCandidates(Expr *fn,
 
         // Compute a new substituted type if we have a base type to apply.
         if (baseType && C.level == 1 && C.getDecl()) {
+          baseType = baseType
+              ->getLValueOrInOutObjectType()
+              ->getRValueInstanceType();
           C.entityType = baseType->getTypeOfMember(CS->DC->getParentModule(),
                                                    C.getDecl(), nullptr);
           C.substituted = true;
@@ -1793,22 +1796,18 @@ CalleeCandidateInfo::CalleeCandidateInfo(Type baseType,
     //
     if (baseType) {
       auto substType = replaceTypeVariablesWithUnresolved(baseType);
+      if (substType)
+        substType = substType
+            ->getLValueOrInOutObjectType()
+            ->getRValueInstanceType();
 
       // If this is a DeclViaUnwrappingOptional, then we're actually looking
       // through an optional to get the member, and baseType is an Optional or
       // Metatype<Optional>.
       if (cand.getKind() == OverloadChoiceKind::DeclViaUnwrappedOptional) {
-        bool isMeta = false;
-        if (auto MTT = substType->getAs<MetatypeType>()) {
-          isMeta = true;
-          substType = MTT->getInstanceType();
-        }
-
         // Look through optional or IUO to get the underlying type the decl was
         // found in.
         substType = substType->getAnyOptionalObjectType();
-        if (isMeta && substType)
-          substType = MetatypeType::get(substType);
       } else if (cand.getKind() != OverloadChoiceKind::Decl) {
         // Otherwise, if it is a remapping we can't handle, don't try to compute
         // a substitution.
@@ -2330,8 +2329,8 @@ bool FailureDiagnosis::diagnoseGeneralMemberFailure(Constraint *constraint) {
   anchor = typeCheckArbitrarySubExprIndependently(anchor, TCC_AllowLValue);
   if (!anchor) return true;
 
-  auto baseTy = anchor->getType();
-  auto baseObjTy = baseTy->getRValueType();
+  auto baseTy = anchor->getType()->getLValueOrInOutObjectType();
+  auto baseObjTy = baseTy;
 
   // If the base type is an IUO, look through it.  Odds are, the code is not
   // trying to find a member of it.
@@ -3105,7 +3104,7 @@ bool FailureDiagnosis::diagnoseGeneralConversionFailure(Constraint *constraint){
   // contain a member var 'boolValue', now does not convert to Bool. This block
   // tries to add a specific diagnosis/fixit to explicitly invoke 'boolValue'.
   if (toType->isBool() &&
-      fromType->mayHaveMemberTypes()) {
+      fromType->mayHaveMembers()) {
     auto LookupResult = CS->TC.lookupMember(CS->DC, fromType,
       DeclName(CS->TC.Context.getIdentifier("boolValue")));
     if (!LookupResult.empty()) {

lib/Sema/CSSimplify.cpp

@@ -2644,9 +2644,6 @@ performMemberLookup(ConstraintKind constraintKind, DeclName memberName,
     return result;
   }
 
-  if (instanceTy->isTypeParameter())
-    return MemberLookupResult();
-
   // Okay, start building up the result list.
   MemberLookupResult result;
   result.OverallResult = MemberLookupResult::HasResults;
@@ -2676,7 +2673,12 @@ performMemberLookup(ConstraintKind constraintKind, DeclName memberName,
     result.ViableCandidates.push_back(OverloadChoice(baseTy, fieldIdx));
     return result;
   }
-  
+
+  if (auto *selfTy = instanceTy->getAs<DynamicSelfType>())
+    instanceTy = selfTy->getSelfType();
+
+  if (!instanceTy->mayHaveMembers())
+    return result;
 
   // If we have a simple name, determine whether there are argument
   // labels we can use to restrict the set of lookup results.
@@ -2741,7 +2743,7 @@ performMemberLookup(ConstraintKind constraintKind, DeclName memberName,
     // extensions cannot yet define designated initializers.
     lookupOptions -= NameLookupFlags::PerformConformanceCheck;
 
-    LookupResult ctors = TC.lookupConstructors(DC, baseObjTy, lookupOptions);
+    LookupResult ctors = TC.lookupConstructors(DC, instanceTy, lookupOptions);
     if (!ctors)
       return result;    // No result.
 
@@ -2831,7 +2833,7 @@ performMemberLookup(ConstraintKind constraintKind, DeclName memberName,
   }
 
   // Look for members within the base.
-  LookupResult &lookup = lookupMember(baseObjTy, memberName);
+  LookupResult &lookup = lookupMember(instanceTy, memberName);
 
   // The set of directly accessible types, which is only used when
   // we're performing dynamic lookup into an existential type.
@@ -3017,10 +3019,11 @@ performMemberLookup(ConstraintKind constraintKind, DeclName memberName,
   if (result.ViableCandidates.empty() && isMetatype &&
       constraintKind == ConstraintKind::UnresolvedValueMember) {
     if (auto objectType = instanceTy->getAnyOptionalObjectType()) {
-      LookupResult &optionalLookup = lookupMember(MetatypeType::get(objectType),
-                                                  memberName);
-      for (auto result : optionalLookup)
-        addChoice(result, /*bridged*/false, /*isUnwrappedOptional=*/true);
+      if (objectType->mayHaveMembers()) {
+        LookupResult &optionalLookup = lookupMember(objectType, memberName);
+        for (auto result : optionalLookup)
+          addChoice(result, /*bridged*/false, /*isUnwrappedOptional=*/true);
+      }
     }
   }
 
@@ -3045,7 +3048,7 @@ performMemberLookup(ConstraintKind constraintKind, DeclName memberName,
     // This is only used for diagnostics, so always use KnownPrivate.
     lookupOptions |= NameLookupFlags::KnownPrivate;
 
-    auto lookup = TC.lookupMember(DC, baseObjTy->getCanonicalType(),
+    auto lookup = TC.lookupMember(DC, instanceTy,
                                   memberName, lookupOptions);
     for (auto cand : lookup) {
       // If the result is invalid, skip it.