Mess with the way we do canonicalization for ProtocolCompositionTypes in a way

that doesn't violate canonical type invariants.  It's still a little messy,
but it's consistent.  Fixes <rdar://problem/11886926>.



Swift SVN r2368

include/swift/AST/Types.h

@@ -991,23 +991,18 @@ private:
 /// protocol, then the canonical type is that protocol type. Otherwise, it is
 /// a composition of the protocols in that list.
 class ProtocolCompositionType : public TypeBase, public llvm::FoldingSetNode {
-  ASTContext &Ctx; // Note: Needed for canonicalization of the empty case.
   SourceLoc FirstLoc; // FIXME: Egregious hack due to lack of TypeLocs
   ArrayRef<Type> Protocols;
   
 public:
   /// \brief Retrieve an instance of a protocol composition type with the
   /// given set of protocols.
-  static ProtocolCompositionType *
+  static Type get(ASTContext &C, SourceLoc FirstLoc, ArrayRef<Type> Protocols);
-  get(ASTContext &C, SourceLoc FirstLoc, ArrayRef<Type> Protocols);
   
   /// \brief Retrieve the source location where this type was first uttered.
   /// FIXME: This is a hackaround for the lack of TypeLocs.
   SourceLoc getFirstLoc() const { return FirstLoc; }
   
-  /// \brief Retrieve the AST context of this type.
-  ASTContext &getASTContext() const { return Ctx; }
-  
   /// \brief Retrieve the set of protocols composed to create this type.
   ArrayRef<Type> getProtocols() const { return Protocols; }
   
@@ -1025,11 +1020,14 @@ public:
   }
   
 private:
-  ProtocolCompositionType(ASTContext &Ctx, SourceLoc FirstLoc,
+  static ProtocolCompositionType *build(ASTContext &C, SourceLoc FirstLoc,
+                                        ArrayRef<Type> Protocols);
+
+  ProtocolCompositionType(ASTContext *Ctx, SourceLoc FirstLoc,
                           ArrayRef<Type> Protocols)
-    : TypeBase(TypeKind::ProtocolComposition, /*Context=*/nullptr,
+    : TypeBase(TypeKind::ProtocolComposition, /*Context=*/Ctx,
                /*Unresolved=*/false),
-      Ctx(Ctx), FirstLoc(FirstLoc), Protocols(Protocols) { }
+      FirstLoc(FirstLoc), Protocols(Protocols) { }
 };
 
 /// LValueType - An l-value is a handle to a physical object.  The

lib/AST/ASTContext.cpp

@@ -240,7 +240,7 @@ IdentifierType *IdentifierType::getNew(ASTContext &C,
 }
 
 ProtocolCompositionType *
-ProtocolCompositionType::get(ASTContext &C, SourceLoc FirstLoc,
+ProtocolCompositionType::build(ASTContext &C, SourceLoc FirstLoc,
                                ArrayRef<Type> Protocols) {
   // Check to see if we've already seen this protocol composition before.
   void *InsertPos = 0;
@@ -250,9 +250,16 @@ ProtocolCompositionType::get(ASTContext &C, SourceLoc FirstLoc,
         = C.Impl.ProtocolCompositionTypes.FindNodeOrInsertPos(ID, InsertPos))
     return Result;
 
+  bool isCanonical = true;
+  for (Type t : Protocols) {
+    if (!t->isCanonical())
+      isCanonical = false;
+  }
+
   // Create a new protocol composition type.
-  ProtocolCompositionType *New = new (C) ProtocolCompositionType(C, FirstLoc,
+  ProtocolCompositionType *New =
-                                                                 Protocols);
+      new (C) ProtocolCompositionType(isCanonical ? &C : nullptr, FirstLoc,
+                                      C.AllocateCopy(Protocols));
   C.Impl.ProtocolCompositionTypes.InsertNode(New, InsertPos);
   return New;
 }

lib/AST/Type.cpp

@@ -435,44 +435,13 @@ CanType TypeBase::getCanonicalType() {
     break;
   }
   case TypeKind::ProtocolComposition: {
-    // Collect all of the protocols composed together.
+    SmallVector<Type, 4> CanProtos;
-    auto PC = cast<ProtocolCompositionType>(this);
+    for (Type t : cast<ProtocolCompositionType>(this)->getProtocols())
-    SmallVector<ProtocolDecl *, 4> Protocols;
+      CanProtos.push_back(t->getCanonicalType());
-    addProtocols(this, Protocols);
+    assert(!CanProtos.empty() && "Non-canonical empty composition?");
-    
+    ASTContext &C = CanProtos[0]->getASTContext();
-    // Minimize the set of protocols composed together.
+    Type Composition = ProtocolCompositionType::get(C, SourceLoc(), CanProtos);
-    minimizeProtocols(Protocols);
+    Result = Composition.getPointer();
-    
-    // If one protocol remains, its nominal type is the canonical type.
-    if (Protocols.size() == 1)
-      Result = Protocols.front()->getDeclaredType();
-    else {
-      // Sort the set of protocols by module + name, to give a stable
-      // ordering.
-      // FIXME: Consider namespaces here as well.
-      llvm::array_pod_sort(Protocols.begin(),Protocols.end(), compareProtocols);
-      
-      // Form the set of canonical protocol types from the protocol
-      // declarations, and use that to buid the canonical composition type.
-      SmallVector<Type, 4> ProtocolTypes;
-      std::transform(Protocols.begin(), Protocols.end(),
-                     std::back_inserter(ProtocolTypes),
-                     [](ProtocolDecl *Proto) {
-                       return Proto->getDeclaredType();
-                     });
-      
-      Result = ProtocolCompositionType::get(
-                 PC->getASTContext(),
-                 PC->getFirstLoc(),
-                 PC->getASTContext().AllocateCopy(ProtocolTypes));
-      
-      // We now know that the result type we computed is canonical; make it so.
-      Result->CanonicalType = &PC->getASTContext();
-    }
-    
-    // If the result type we found was ourselves, we're done.
-    if (this == Result)
-      return CanType(Result);
     break;
   }
   case TypeKind::MetaType: {
@@ -715,6 +684,42 @@ BoundGenericType::setConformances(ArrayRef<unsigned> Offsets,
   AllConformances->Conformances = Ctx.AllocateCopy(Conformances);
 }
 
+Type
+ProtocolCompositionType::get(ASTContext &C, SourceLoc FirstLoc,
+                             ArrayRef<Type> ProtocolTypes) {
+  for (Type t : ProtocolTypes) {
+    if (!t->isCanonical())
+      return build(C, FirstLoc, ProtocolTypes);
+  }
+    
+  SmallVector<ProtocolDecl *, 4> Protocols;
+  for (Type t : ProtocolTypes)
+    addProtocols(t, Protocols);
+  
+  // Minimize the set of protocols composed together.
+  minimizeProtocols(Protocols);
+
+  // If one protocol remains, its nominal type is the canonical type.
+  if (Protocols.size() == 1)
+    return Protocols.front()->getDeclaredType();
+
+  // Sort the set of protocols by module + name, to give a stable
+  // ordering.
+  // FIXME: Consider namespaces here as well.
+  llvm::array_pod_sort(Protocols.begin(), Protocols.end(), compareProtocols);
+
+  // Form the set of canonical protocol types from the protocol
+  // declarations, and use that to buid the canonical composition type.
+  SmallVector<Type, 4> CanProtocolTypes;
+  std::transform(Protocols.begin(), Protocols.end(),
+                 std::back_inserter(CanProtocolTypes),
+                 [](ProtocolDecl *Proto) {
+                   return Proto->getDeclaredType();
+                 });
+
+  return build(C, SourceLoc(), CanProtocolTypes);
+}
+
 //===----------------------------------------------------------------------===//
 //  Type Printing
 //===----------------------------------------------------------------------===//

lib/Parse/ParseType.cpp

@@ -302,8 +302,7 @@ bool Parser::parseTypeComposition(Type &Result, TypeLoc *&ResultLoc) {
     EndLoc = consumeStartingGreater();
   }
   
-  Result = ProtocolCompositionType::get(Context, ProtocolLoc,
+  Result = ProtocolCompositionType::get(Context, ProtocolLoc, Protocols);
-                                        Context.AllocateCopy(Protocols));
   ResultLoc = TypeLoc::get(Context, SourceRange(ProtocolLoc, EndLoc));
   return false;
 }

lib/Sema/TypeCheckExpr.cpp

@@ -247,8 +247,7 @@ collectArchetypeToExistentialSubstitutions(ASTContext &Context,
                        std::back_inserter(ConformsTo),
                        [](ProtocolDecl *P) { return P->getDeclaredType(); });
         Substitutions[Archetype]
-          = ProtocolCompositionType::get(Context, SourceLoc(),
+          = ProtocolCompositionType::get(Context, SourceLoc(), ConformsTo);
-                                         Context.AllocateCopy(ConformsTo));
       }
     }
   }

lib/Sema/TypeCheckType.cpp

@@ -693,8 +693,7 @@ Type TypeChecker::substType(Type T, TypeSubstitutionMap &Substitutions) {
     if (!AnyChanged)
       return T;
     
-    return ProtocolCompositionType::get(Context, PC->getFirstLoc(),
+    return ProtocolCompositionType::get(Context, PC->getFirstLoc(), Protocols);
-                                        Context.AllocateCopy(Protocols));
   }
   }