AST: Remove TypeBase::hasDependentProtocolConformances()

slavapestov · slavapestov · commit 3eabc2139ba6 · 2016-12-04T21:15:03.000-08:00
Another oddly-named utility function with poorly-defined behavior.

It returned true for archetypes, generic parameters, existential
types, and metatypes of existential types.

However, it would return false for dependent member types, or
metatypes of archetypes, and so on.

All the callers were doing something bad to begin with, so
changing them over to more precise predicates improved the code.

In particular, this simplifies substitution construction in
the SIL parser, and makes it stricter, which turned up a couple
of mistakes in the SIL tests where we were doing stuff with
non-conforming types.
diff --git a/include/swift/AST/DiagnosticsParse.def b/include/swift/AST/DiagnosticsParse.def
@@ -508,7 +508,8 @@ ERROR(sil_member_lookup_bad_type,none,
 ERROR(sil_member_decl_type_mismatch,none,
       "member defined with mismatching type %0 (expected %1)", (Type, Type))
 ERROR(sil_substitution_mismatch,none,
-      "substitution conformances don't match archetype", ())
+      "substitution replacement type %0 does not conform to protocol %1",
+      (Type, DeclName))
 ERROR(sil_missing_substitutions,none,
       "missing substitutions", ())
 ERROR(sil_too_many_substitutions,none,
diff --git a/include/swift/AST/Types.h b/include/swift/AST/Types.h
@@ -938,13 +938,6 @@ class alignas(1 << TypeAlignInBits) TypeBase {
   /// Return whether this type is or can be substituted for a bridgeable
   /// object type.
   TypeTraitResult canBeClass();
-  
-  /// Returns true if the type conforms to protocols using witnesses from the
-  /// environment or from within the value. Generic parameters and existentials
-  /// meet this criteria. In these cases we represent the
-  /// conformance as a null ProtocolConformance* pointer, because there is no
-  /// static conformance associated with the conforming type.
-  bool hasDependentProtocolConformances();
 
 private:
   // Make vanilla new/delete illegal for Types.
@@ -4615,11 +4608,6 @@ inline CanType Type::getCanonicalTypeOrNull() const {
   return isNull() ? CanType() : getPointer()->getCanonicalType();
 }
 
-inline bool TypeBase::hasDependentProtocolConformances() {
-  return is<SubstitutableType>() || is<GenericTypeParamType>()
-      || isAnyExistentialType();
-}
-
 #define TYPE(id, parent)
 #define SUGARED_TYPE(id, parent) \
 template <> \
diff --git a/lib/AST/Substitution.cpp b/lib/AST/Substitution.cpp
@@ -92,15 +92,10 @@ Substitution Substitution::subst(Module *module,
       conformance = ProtocolConformanceRef(lookupResults.front());
     }
 
-    if (conformance) {
-      if (conformance->isConcrete())
-        conformancesChanged = true;
-      substConformances.push_back(*conformance);
-    } else {
-      assert(substReplacement->hasDependentProtocolConformances() &&
-             "couldn't find concrete conformance for concrete type?");
-      substConformances.push_back(ProtocolConformanceRef(proto));
-    }
+    assert(conformance);
+    if (conformance->isConcrete())
+      conformancesChanged = true;
+    substConformances.push_back(*conformance);
   }
   assert(substConformances.size() == Conformance.size());
 
diff --git a/lib/Parse/ParseSIL.cpp b/lib/Parse/ParseSIL.cpp
@@ -217,6 +217,10 @@ namespace {
 
     /// @{ Type parsing.
     bool parseASTType(CanType &result);
+    bool parseASTType(CanType &result, SourceLoc &TypeLoc) {
+      TypeLoc = P.Tok.getLoc();
+      return parseASTType(result);
+    }
     bool parseSILType(SILType &Result,
                       GenericEnvironment *&genericEnv,
                       bool IsFuncDecl = false);
@@ -1277,71 +1281,24 @@ bool SILParser::parseSubstitutions(SmallVectorImpl<ParsedSubstitution> &parsed,
   return false;
 }
 
-/// Get ProtocolConformance for a replacement type.
-static ProtocolConformance*
-getConformanceOfReplacement(Parser &P, Type subReplacement,
-                            ProtocolDecl *proto) {
-  auto conformance = P.SF.getParentModule()->lookupConformance(
-                       subReplacement, proto, nullptr);
-  if (conformance && conformance->isConcrete())
-    return conformance->getConcrete();
-  return nullptr;
-}
-
-static bool isImpliedBy(ProtocolDecl *proto, ArrayRef<ProtocolDecl*> derived) {
-  for (auto derivedProto : derived) {
-    if (derivedProto == proto || derivedProto->inheritsFrom(proto))
-      return true;
-  }
-  return false;
-}
-
-static bool allowAbstractConformance(Parser &P, Type subReplacement,
-                                     ProtocolDecl *proto) {
-  if (!subReplacement->hasDependentProtocolConformances())
-    return false;
-
-  // AnyObject is implicitly conformed to by anything with a class bound.
-  if (proto->isSpecificProtocol(KnownProtocolKind::AnyObject) &&
-      subReplacement->isAnyClassReferenceType()) {
-    return true;
-  }
-
-  if (auto archetype = subReplacement->getAs<ArchetypeType>()) {
-    return isImpliedBy(proto, archetype->getConformsTo());
-  }
-
-  SmallVector<ProtocolDecl *, 4> existentialProtos;
-  if (subReplacement->isExistentialType(existentialProtos)) {
-    return isImpliedBy(proto, existentialProtos);
-  }
-
-  return false;
-}
-
 /// Collect conformances by looking up the conformance from replacement
 /// type and protocol decl.
 static bool getConformancesForSubstitution(Parser &P,
               ArrayRef<ProtocolDecl *> protocols,
               Type subReplacement,
               SourceLoc loc,
               SmallVectorImpl<ProtocolConformanceRef> &conformances) {
-  for (auto proto : protocols) {
-    // Try looking up a concrete conformance.
-    if (auto conformance =
-          getConformanceOfReplacement(P, subReplacement, proto)) {
-      conformances.push_back(ProtocolConformanceRef(conformance));
-      continue;
-    }
+  auto M = P.SF.getParentModule();
 
-    // If the replacement type has dependent conformances, we might be
-    // able to use an abstract conformance.
-    if (allowAbstractConformance(P, subReplacement, proto)) {
-      conformances.push_back(ProtocolConformanceRef(proto));
+  for (auto proto : protocols) {
+    auto conformance = M->lookupConformance(subReplacement, proto, nullptr);
+    if (conformance) {
+      conformances.push_back(*conformance);
       continue;
     }
 
-    P.diagnose(loc, diag::sil_substitution_mismatch);
+    P.diagnose(loc, diag::sil_substitution_mismatch, subReplacement,
+               proto->getName());
     return true;
   }
 
@@ -1403,25 +1360,20 @@ bool getApplySubstitutionsFromParsed(
 }
 
 static ArrayRef<ProtocolConformanceRef>
-collectExistentialConformances(Parser &P,
-                               CanType conformingType, CanType protocolType) {
+collectExistentialConformances(Parser &P, CanType conformingType, SourceLoc loc,
+                               CanType protocolType) {
   SmallVector<ProtocolDecl *, 2> protocols;
   bool isExistential = protocolType->isAnyExistentialType(protocols);
   assert(isExistential);
   (void)isExistential;
   if (protocols.empty())
     return {};
-  
-  MutableArrayRef<ProtocolConformanceRef> conformances =
-    P.Context.AllocateUninitialized<ProtocolConformanceRef>(protocols.size());
-  
-  for (unsigned i : indices(protocols)) {
-    auto proto = protocols[i];
-    auto conformance = getConformanceOfReplacement(P, conformingType, proto);
-    conformances[i] = ProtocolConformanceRef(proto, conformance);
-  }
-  
-  return conformances;
+
+  SmallVector<ProtocolConformanceRef, 2> conformances;
+  getConformancesForSubstitution(P, protocols, conformingType,
+                                 loc, conformances);
+
+  return P.Context.AllocateCopy(conformances);
 }
 
 /// sil-loc ::= 'loc' string-literal ':' [0-9]+ ':' [0-9]+
@@ -3568,10 +3520,11 @@ bool SILParser::parseSILInstruction(SILBasicBlock *BB, SILBuilder &B) {
   }
   case ValueKind::InitExistentialAddrInst: {
     CanType Ty;
+    SourceLoc TyLoc;
     if (parseTypedValueRef(Val, B) ||
         P.parseToken(tok::comma, diag::expected_tok_in_sil_instr, ",") ||
         P.parseToken(tok::sil_dollar, diag::expected_tok_in_sil_instr, "$") ||
-        parseASTType(Ty) ||
+        parseASTType(Ty, TyLoc) ||
         parseSILDebugLocation(InstLoc, B))
       return true;
     
@@ -3586,7 +3539,7 @@ bool SILParser::parseSILInstruction(SILBasicBlock *BB, SILBuilder &B) {
     
     // Collect conformances for the type.
     ArrayRef<ProtocolConformanceRef> conformances
-      = collectExistentialConformances(P, Ty,
+      = collectExistentialConformances(P, Ty, TyLoc,
                                        Val->getType().getSwiftRValueType());
     
     ResultVal = B.createInitExistentialAddr(InstLoc, Val, Ty, LoweredTy,
@@ -3596,17 +3549,18 @@ bool SILParser::parseSILInstruction(SILBasicBlock *BB, SILBuilder &B) {
   case ValueKind::AllocExistentialBoxInst: {
     SILType ExistentialTy;
     CanType ConcreteFormalTy;
+    SourceLoc TyLoc;
     
     if (parseSILType(ExistentialTy) ||
         P.parseToken(tok::comma, diag::expected_tok_in_sil_instr, ",") ||
         P.parseToken(tok::sil_dollar, diag::expected_tok_in_sil_instr, "$") ||
-        parseASTType(ConcreteFormalTy) ||
+        parseASTType(ConcreteFormalTy, TyLoc) ||
         parseSILDebugLocation(InstLoc, B))
       return true;
     
     // Collect conformances for the type.
     ArrayRef<ProtocolConformanceRef> conformances
-      = collectExistentialConformances(P, ConcreteFormalTy,
+      = collectExistentialConformances(P, ConcreteFormalTy, TyLoc,
                                        ExistentialTy.getSwiftRValueType());
     
     ResultVal = B.createAllocExistentialBox(InstLoc, ExistentialTy,
@@ -3617,17 +3571,19 @@ bool SILParser::parseSILInstruction(SILBasicBlock *BB, SILBuilder &B) {
   case ValueKind::InitExistentialRefInst: {
     CanType FormalConcreteTy;
     SILType ExistentialTy;
+    SourceLoc TyLoc;
+
     if (parseTypedValueRef(Val, B) ||
         P.parseToken(tok::colon, diag::expected_tok_in_sil_instr, ":") ||
         P.parseToken(tok::sil_dollar, diag::expected_tok_in_sil_instr, "$") ||
-        parseASTType(FormalConcreteTy) ||
+        parseASTType(FormalConcreteTy, TyLoc) ||
         P.parseToken(tok::comma, diag::expected_tok_in_sil_instr, ",") ||
         parseSILType(ExistentialTy) ||
         parseSILDebugLocation(InstLoc, B))
       return true;
     
     ArrayRef<ProtocolConformanceRef> conformances
-      = collectExistentialConformances(P, FormalConcreteTy,
+      = collectExistentialConformances(P, FormalConcreteTy, TyLoc,
                             ExistentialTy.getSwiftRValueType());
 
     // FIXME: Conformances in InitExistentialRefInst is currently not included
@@ -3638,10 +3594,11 @@ bool SILParser::parseSILInstruction(SILBasicBlock *BB, SILBuilder &B) {
     break;
   }
   case ValueKind::InitExistentialMetatypeInst: {
+    SourceLoc TyLoc;
     SILType ExistentialTy;
     if (parseTypedValueRef(Val, B) ||
         P.parseToken(tok::comma, diag::expected_tok_in_sil_instr, ",") ||
-        parseSILType(ExistentialTy) ||
+        parseSILType(ExistentialTy, TyLoc) ||
         parseSILDebugLocation(InstLoc, B))
       return true;
 
@@ -3654,7 +3611,7 @@ bool SILParser::parseSILInstruction(SILBasicBlock *BB, SILBuilder &B) {
     }
 
     ArrayRef<ProtocolConformanceRef> conformances
-      = collectExistentialConformances(P, formalConcreteType,
+      = collectExistentialConformances(P, formalConcreteType, TyLoc,
                             ExistentialTy.getSwiftRValueType());
     
     ResultVal = B.createInitExistentialMetatype(InstLoc, Val, ExistentialTy,
@@ -4388,9 +4345,8 @@ static NormalProtocolConformance *parseNormalProtocolConformance(Parser &P,
   // Calling lookupConformance on a BoundGenericType will return a specialized
   // conformance. We use UnboundGenericType to find the normal conformance.
   Type lookupTy = ConformingTy;
-  if (auto bound = dyn_cast<BoundGenericType>(lookupTy.getPointer()))
-    lookupTy = UnboundGenericType::get(bound->getDecl(), bound->getParent(),
-                                       P.Context);
+  if (auto bound = lookupTy->getAs<BoundGenericType>())
+    lookupTy = bound->getDecl()->getDeclaredType();
   auto lookup = P.SF.getParentModule()->lookupConformance(
                          lookupTy, proto, nullptr);
   if (!lookup) {
diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp
@@ -108,30 +108,18 @@ Type Solution::computeSubstitutions(
   auto lookupConformanceFn =
       [&](CanType original, Type replacement, ProtocolType *protoType)
           -> ProtocolConformanceRef {
+    if (replacement->hasError() ||
+        isOpenedAnyObject(replacement) ||
+        replacement->is<GenericTypeParamType>()) {
+      return ProtocolConformanceRef(protoType->getDecl());
+    }
+
     auto conformance = tc.conformsToProtocol(
                          replacement,
                          protoType->getDecl(),
                          getConstraintSystem().DC,
                          (ConformanceCheckFlags::InExpression|
                           ConformanceCheckFlags::Used));
-    (void)&isOpenedAnyObject;
-    assert((conformance ||
-            replacement->hasError() ||
-            isOpenedAnyObject(replacement) ||
-            replacement->is<GenericTypeParamType>()) &&
-           "Constraint system missed a conformance?");
-
-    // Put an abstract conformance in place if we don't already have one.
-    // FIXME: Feels like a hack
-    if (!conformance &&
-        (replacement->hasDependentProtocolConformances() ||
-         replacement->hasError()))
-      conformance = ProtocolConformanceRef(protoType->getDecl());
-
-    assert(conformance->isConcrete() ||
-           replacement->hasError() ||
-           replacement->hasDependentProtocolConformances());
-
     return *conformance;
   };
 
@@ -190,11 +178,8 @@ static DeclTy *findNamedWitnessImpl(
 
   // For a type with dependent conformance, just return the requirement from
   // the protocol. There are no protocol conformance tables.
-  if (type->hasDependentProtocolConformances()) {
+  if (!conformance->isConcrete())
     return requirement;
-  }
-
-  if (!conformance->isConcrete()) return nullptr;
   auto concrete = conformance->getConcrete();
   // FIXME: Dropping substitutions here.
   return cast_or_null<DeclTy>(concrete->getWitness(requirement, &tc).getDecl());
@@ -859,11 +844,12 @@ namespace {
       }
 
       // Otherwise, we're referring to a member of a type.
-
-      // Is it an archetype member?
-      bool isDependentConformingRef
-            = isa<ProtocolDecl>(member->getDeclContext()) &&
-              baseTy->hasDependentProtocolConformances();
+      Type selfTy;
+      if (isa<ProtocolDecl>(member->getDeclContext()) &&
+          (baseTy->is<ArchetypeType>() || baseTy->isExistentialType()))
+        selfTy = baseTy;
+      else
+        selfTy = containerTy;
 
       // References to properties with accessors and storage usually go
       // through the accessors, but sometimes are direct.
@@ -875,12 +861,7 @@ namespace {
       if (baseIsInstance) {
         // Convert the base to the appropriate container type, turning it
         // into an lvalue if required.
-        Type selfTy;
-        if (isDependentConformingRef)
-          selfTy = baseTy;
-        else
-          selfTy = containerTy;
-        
+
         // If the base is already an lvalue with the right base type, we can
         // pass it as an inout qualified type.
         if (selfTy->isEqual(baseTy))
@@ -892,9 +873,7 @@ namespace {
       } else {
         // Convert the base to an rvalue of the appropriate metatype.
         base = coerceToType(base,
-                            MetatypeType::get(isDependentConformingRef
-                                                ? baseTy
-                                                : containerTy),
+                            MetatypeType::get(selfTy),
                             locator.withPathElement(
                               ConstraintLocator::MemberRefBase));
         if (!base)
@@ -5685,8 +5664,7 @@ static Type adjustSelfTypeForMember(Type baseTy, ValueDecl *member,
       // case, the member will be modeled as an inout but ExistentialMemberRef
       // and ArchetypeMemberRef want to take the base as an rvalue.
       if (auto *fd = dyn_cast<FuncDecl>(func))
-        if (!fd->isMutating() &&
-            baseObjectTy->hasDependentProtocolConformances())
+        if (!fd->isMutating() && baseObjectTy->is<ArchetypeType>())
           return baseObjectTy;
       
       return InOutType::get(baseObjectTy);
@@ -6223,7 +6201,7 @@ Expr *ExprRewriter::finishApply(ApplyExpr *apply, Type openedType,
   // We're constructing a value of nominal type. Look for the constructor or
   // enum element to use.
   assert(ty->getNominalOrBoundGenericNominal() || ty->is<DynamicSelfType>() ||
-         ty->hasDependentProtocolConformances());
+         ty->isExistentialType() || ty->is<ArchetypeType>());
   auto ctorLocator = cs.getConstraintLocator(
                  locator.withPathElement(ConstraintLocator::ApplyFunction)
                         .withPathElement(ConstraintLocator::ConstructorMember));
diff --git a/test/SIL/Parser/undef.sil b/test/SIL/Parser/undef.sil
diff --git a/test/SILOptimizer/arcsequenceopts_rcidentityanalysis.sil b/test/SILOptimizer/arcsequenceopts_rcidentityanalysis.sil
