Address review comments

Author: swiftix

include/swift/AST/Substitution.h

@@ -48,6 +48,9 @@ class Substitution {
 
   Substitution(Type Replacement, ArrayRef<ProtocolConformanceRef> Conformance);
 
+  /// Checks whether the current substituion is canonical.
+  bool isCanonical() const;
+
   /// Get the canonicalized substitution. If wasCanonical is not nullptr,
   /// store there whether the current substitution was canonical already.
   Substitution getCanonicalSubstitution(bool *wasCanonical = nullptr) const;

lib/AST/ProtocolConformance.cpp

@@ -1073,11 +1073,8 @@ bool ProtocolConformance::isCanonical() const {
 
   switch (getKind()) {
   case ProtocolConformanceKind::Normal: {
-    auto normalConformance = cast<NormalProtocolConformance>(this);
-    auto sigConformances = normalConformance->getSignatureConformances();
-    for (auto conf : sigConformances)
-      if (!conf.isCanonical())
-        return false;
+    // Normal conformances are considered canonical by
+    // construction.
     return true;
   }
   case ProtocolConformanceKind::Inherited: {
@@ -1093,13 +1090,9 @@ bool ProtocolConformance::isCanonical() const {
     if (!genericConformance->isCanonical())
       return false;
     auto specSubs = spec->getGenericSubstitutions();
-    for (auto sub : specSubs) {
-      if (!sub.getReplacement()->isCanonical())
+    for (const auto &sub : specSubs) {
+      if (!sub.isCanonical())
         return false;
-      for (auto conf : sub.getConformances()) {
-        if (!conf.isCanonical())
-          return false;
-      }
     }
     return true;
   }

lib/AST/Substitution.cpp

@@ -42,3 +42,13 @@ Substitution::Substitution(Type Replacement,
   assert(Replacement->isMaterializable()
          && "cannot substitute with a non-materializable type");
 }
+
+bool Substitution::isCanonical() const {
+  if (!getReplacement()->isCanonical())
+    return false;
+  for (auto conf : getConformances()) {
+    if (!conf.isCanonical())
+      return false;
+  }
+  return true;
+}