fix some reviewed issues

tayloraswift · tayloraswift · commit 270101243496 · 2020-01-22T21:56:47.000-06:00
diff --git a/include/swift/AST/ASTContext.h b/include/swift/AST/ASTContext.h
@@ -437,9 +437,6 @@ class ASTContext final {
   void setStatsReporter(UnifiedStatsReporter *stats);
 
 private:
-  // get `<` or `==`
-  FuncDecl *getBinaryComparisonOperatorIntDecl(StringRef op, FuncDecl **cached) const;
-  
   friend class TypeChecker;
 
   void installGlobalTypeChecker(TypeChecker *TC);
diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp
@@ -1086,35 +1086,36 @@ ASTContext::getBuiltinInitDecl(NominalTypeDecl *decl,
   return witness;
 }
 
-FuncDecl *ASTContext::getBinaryComparisonOperatorIntDecl(StringRef op, FuncDecl **cached) const {
-  if (*cached)
-    return *cached;
+static 
+FuncDecl *getBinaryComparisonOperatorIntDecl(const ASTContext &C, StringRef op, FuncDecl *&cached) {
+  if (cached)
+    return cached;
 
-  if (!getIntDecl() || !getBoolDecl())
+  if (!C.getIntDecl() || !C.getBoolDecl())
     return nullptr;
 
-  auto intType = getIntDecl()->getDeclaredType();
+  auto intType = C.getIntDecl()->getDeclaredType();
   auto isIntParam = [&](AnyFunctionType::Param param) {
     return (!param.isVariadic() && !param.isInOut() &&
             param.getPlainType()->isEqual(intType));
   };
-  auto boolType = getBoolDecl()->getDeclaredType();
-  auto decl = lookupOperatorFunc(*this, op,
-                                 intType, [=](FunctionType *type) {
+  auto boolType = C.getBoolDecl()->getDeclaredType();
+  auto decl = lookupOperatorFunc(C, op, intType, 
+      [=](FunctionType *type) {
     // Check for the signature: (Int, Int) -> Bool
     if (type->getParams().size() != 2) return false;
     if (!isIntParam(type->getParams()[0]) ||
         !isIntParam(type->getParams()[1])) return false;
     return type->getResult()->isEqual(boolType);
   });
-  *cached = decl;
+  cached = decl;
   return decl;
 }
 FuncDecl *ASTContext::getLessThanIntDecl() const {
-  return getBinaryComparisonOperatorIntDecl("<", &getImpl().LessThanIntDecl);
+  return getBinaryComparisonOperatorIntDecl(*this, "<", getImpl().LessThanIntDecl);
 }
 FuncDecl *ASTContext::getEqualIntDecl() const {
-  return getBinaryComparisonOperatorIntDecl("==", &getImpl().EqualIntDecl);
+  return getBinaryComparisonOperatorIntDecl(*this, "==", getImpl().EqualIntDecl);
 }
 
 FuncDecl *ASTContext::getHashValueForDecl() const {
diff --git a/lib/Sema/DerivedConformanceComparable.cpp b/lib/Sema/DerivedConformanceComparable.cpp
@@ -31,52 +31,6 @@
 
 using namespace swift;
 
-/// Returns a generated guard statement that checks whether the given lhs and
-/// rhs expressions are equal. If not equal, the else block for the guard
-/// returns lhs < rhs.
-/// \p C The AST context.
-/// \p lhsExpr The first expression to compare for equality.
-/// \p rhsExpr The second expression to compare for equality.
-static GuardStmt *returnComparisonIfNotEqualGuard(ASTContext &C,
-                                        Expr *lhsExpr,
-                                        Expr *rhsExpr) {
-  SmallVector<StmtConditionElement, 1> conditions;
-  SmallVector<ASTNode, 1> statements;
-
-  // First, generate the statement for the body of the guard.
-  // return lhs < rhs
-  auto ltFuncExpr = new (C) UnresolvedDeclRefExpr(
-    DeclNameRef(C.Id_LessThanOperator), DeclRefKind::BinaryOperator,
-    DeclNameLoc());
-  auto ltArgsTuple = TupleExpr::create(C, SourceLoc(),
-                                        { lhsExpr, rhsExpr },
-                                        { }, { }, SourceLoc(),
-                                        /*HasTrailingClosure*/false,
-                                        /*Implicit*/true);
-  auto ltExpr = new (C) BinaryExpr(ltFuncExpr, ltArgsTuple, /*Implicit*/true);
-  auto returnStmt = new (C) ReturnStmt(SourceLoc(), ltExpr);
-  statements.emplace_back(ASTNode(returnStmt));
-
-  // Next, generate the condition being checked.
-  // lhs == rhs
-  auto cmpFuncExpr = new (C) UnresolvedDeclRefExpr(
-    DeclNameRef(C.Id_EqualsOperator), DeclRefKind::BinaryOperator,
-    DeclNameLoc());
-  auto cmpArgsTuple = TupleExpr::create(C, SourceLoc(),
-                                        { lhsExpr, rhsExpr },
-                                        { }, { }, SourceLoc(),
-                                        /*HasTrailingClosure*/false,
-                                        /*Implicit*/true);
-  auto cmpExpr = new (C) BinaryExpr(cmpFuncExpr, cmpArgsTuple,
-                                    /*Implicit*/true);
-  conditions.emplace_back(cmpExpr);
-
-  // Build and return the complete guard statement.
-  // guard lhs == rhs else { return lhs < rhs }
-  auto body = BraceStmt::create(C, SourceLoc(), statements, SourceLoc());
-  return new (C) GuardStmt(SourceLoc(), C.AllocateCopy(conditions), body);
-}
-
 // how does this code ever even get invoked? you can’t compare uninhabited enums...
 static std::pair<BraceStmt *, bool>
 deriveBodyComparable_enum_uninhabited_lt(AbstractFunctionDecl *ltDecl, void *) {
@@ -129,9 +83,9 @@ deriveBodyComparable_enum_noAssociatedValues_lt(AbstractFunctionDecl *ltDecl,
 
   // Generate the conversion from the enums to integer indices.
   SmallVector<ASTNode, 6> statements;
-  DeclRefExpr *aIndex = convertEnumToIndex(statements, parentDC, enumDecl,
+  DeclRefExpr *aIndex = DerivedConformance::convertEnumToIndex(statements, parentDC, enumDecl,
                                            aParam, ltDecl, "index_a");
-  DeclRefExpr *bIndex = convertEnumToIndex(statements, parentDC, enumDecl,
+  DeclRefExpr *bIndex = DerivedConformance::convertEnumToIndex(statements, parentDC, enumDecl,
                                            bParam, ltDecl, "index_b");
 
   // Generate the compare of the indices.
@@ -199,7 +153,7 @@ deriveBodyComparable_enum_hasAssociatedValues_lt(AbstractFunctionDecl *ltDecl, v
 
     // .<elt>(let l0, let l1, ...)
     SmallVector<VarDecl*, 3> lhsPayloadVars;
-    auto lhsSubpattern = enumElementPayloadSubpattern(elt, 'l', ltDecl,
+    auto lhsSubpattern = DerivedConformance::enumElementPayloadSubpattern(elt, 'l', ltDecl,
                                                       lhsPayloadVars);
     auto lhsElemPat = new (C) EnumElementPattern(TypeLoc::withoutLoc(enumType),
                                                  SourceLoc(), DeclNameLoc(),
@@ -209,7 +163,7 @@ deriveBodyComparable_enum_hasAssociatedValues_lt(AbstractFunctionDecl *ltDecl, v
 
     // .<elt>(let r0, let r1, ...)
     SmallVector<VarDecl*, 3> rhsPayloadVars;
-    auto rhsSubpattern = enumElementPayloadSubpattern(elt, 'r', ltDecl,
+    auto rhsSubpattern = DerivedConformance::enumElementPayloadSubpattern(elt, 'r', ltDecl,
                                                       rhsPayloadVars);
     auto rhsElemPat = new (C) EnumElementPattern(TypeLoc::withoutLoc(enumType),
                                                  SourceLoc(), DeclNameLoc(),
@@ -254,7 +208,8 @@ deriveBodyComparable_enum_hasAssociatedValues_lt(AbstractFunctionDecl *ltDecl, v
       auto rhsVar = rhsPayloadVars[varIdx];
       auto rhsExpr = new (C) DeclRefExpr(rhsVar, DeclNameLoc(),
                                          /*Implicit*/true);
-      auto guardStmt = returnComparisonIfNotEqualGuard(C, lhsExpr, rhsExpr);
+      auto guardStmt = DerivedConformance::returnComparisonIfNotEqualGuard(C, 
+          lhsExpr, rhsExpr);
       statementsInCase.emplace_back(guardStmt);
     }
 
@@ -380,14 +335,17 @@ deriveComparable_lt(
 
 bool 
 DerivedConformance::canDeriveComparable(DeclContext *context, NominalTypeDecl *declaration) {
+  auto enumeration = dyn_cast<EnumDecl>(declaration);
   // The type must be an enum.
-  if (EnumDecl *const enumeration = dyn_cast<EnumDecl>(declaration)) {
-    // The cases must not have non-comparable associated values or raw backing
-    auto comparable = context->getASTContext().getProtocol(KnownProtocolKind::Comparable);
-    return allAssociatedValuesConformToProtocol(context, enumeration, comparable) && !enumeration->hasRawType();
-  } else {
-    return false;
+  if (!enumeration) {
+      return false;
   }
+  auto comparable = context->getASTContext().getProtocol(KnownProtocolKind::Comparable);
+  if (!comparable) {
+      return false; // not sure what should be done here instead
+  }
+  // The cases must not have non-comparable associated values or raw backing
+  return allAssociatedValuesConformToProtocol(context, enumeration, comparable) && !enumeration->hasRawType();
 }
 
 ValueDecl *DerivedConformance::deriveComparable(ValueDecl *requirement) {
@@ -396,12 +354,17 @@ ValueDecl *DerivedConformance::deriveComparable(ValueDecl *requirement) {
   // Build the necessary decl.
   if (requirement->getBaseName() == "<") {
     if (EnumDecl const *const enumeration = dyn_cast<EnumDecl>(this->Nominal)) {
-      auto bodySynthesizer = !enumeration->hasCases() 
-        ? &deriveBodyComparable_enum_uninhabited_lt 
-        : enumeration->hasOnlyCasesWithoutAssociatedValues()
-            ? &deriveBodyComparable_enum_noAssociatedValues_lt
-            : &deriveBodyComparable_enum_hasAssociatedValues_lt; 
-      return deriveComparable_lt(*this, bodySynthesizer);
+      std::pair<BraceStmt *, bool> (*synthesizer)(AbstractFunctionDecl *, void *);
+      if (enumeration->hasCases()) {
+        if (enumeration->hasOnlyCasesWithoutAssociatedValues()) {
+            synthesizer = &deriveBodyComparable_enum_noAssociatedValues_lt;
+        } else {
+            synthesizer = &deriveBodyComparable_enum_hasAssociatedValues_lt;
+        }
+      } else {
+        synthesizer = &deriveBodyComparable_enum_uninhabited_lt;
+      }
+      return deriveComparable_lt(*this, synthesizer);
     } else {
       llvm_unreachable("todo");
     }
diff --git a/lib/Sema/DerivedConformanceEquatableHashable.cpp b/lib/Sema/DerivedConformanceEquatableHashable.cpp
@@ -82,7 +82,7 @@ static bool canDeriveConformance(DeclContext *DC,
   if (auto enumDecl = dyn_cast<EnumDecl>(target)) {
     // The cases must not have associated values, or all associated values must
     // conform to the protocol.
-    return allAssociatedValuesConformToProtocol(DC, enumDecl, protocol);
+    return DerivedConformance::allAssociatedValuesConformToProtocol(DC, enumDecl, protocol);
   }
 
   if (auto structDecl = dyn_cast<StructDecl>(target)) {
@@ -101,7 +101,7 @@ void diagnoseFailedDerivation(DeclContext *DC, NominalTypeDecl *nominal,
 
   if (auto *enumDecl = dyn_cast<EnumDecl>(nominal)) {
     auto nonconformingAssociatedTypes =
-        associatedValuesNotConformingToProtocol(DC, enumDecl, protocol);
+        DerivedConformance::associatedValuesNotConformingToProtocol(DC, enumDecl, protocol);
     for (auto *typeToDiagnose : nonconformingAssociatedTypes) {
       SourceLoc reprLoc;
       if (auto *repr = typeToDiagnose->getTypeRepr())
@@ -135,45 +135,6 @@ void diagnoseFailedDerivation(DeclContext *DC, NominalTypeDecl *nominal,
   }
 }
 
-/// Returns a generated guard statement that checks whether the given lhs and
-/// rhs expressions are equal. If not equal, the else block for the guard
-/// returns false.
-/// \p C The AST context.
-/// \p lhsExpr The first expression to compare for equality.
-/// \p rhsExpr The second expression to compare for equality.
-static GuardStmt *returnIfNotEqualGuard(ASTContext &C,
-                                        Expr *lhsExpr,
-                                        Expr *rhsExpr) {
-  SmallVector<StmtConditionElement, 1> conditions;
-  SmallVector<ASTNode, 1> statements;
-
-  // First, generate the statement for the body of the guard.
-  // return false
-  auto falseExpr = new (C) BooleanLiteralExpr(false, SourceLoc(),
-                                              /*Implicit*/true);
-  auto returnStmt = new (C) ReturnStmt(SourceLoc(), falseExpr);
-  statements.emplace_back(ASTNode(returnStmt));
-
-  // Next, generate the condition being checked.
-  // lhs == rhs
-  auto cmpFuncExpr = new (C) UnresolvedDeclRefExpr(
-    DeclNameRef(C.Id_EqualsOperator), DeclRefKind::BinaryOperator,
-    DeclNameLoc());
-  auto cmpArgsTuple = TupleExpr::create(C, SourceLoc(),
-                                        { lhsExpr, rhsExpr },
-                                        { }, { }, SourceLoc(),
-                                        /*HasTrailingClosure*/false,
-                                        /*Implicit*/true);
-  auto cmpExpr = new (C) BinaryExpr(cmpFuncExpr, cmpArgsTuple,
-                                    /*Implicit*/true);
-  conditions.emplace_back(cmpExpr);
-
-  // Build and return the complete guard statement.
-  // guard lhs == rhs else { return false }
-  auto body = BraceStmt::create(C, SourceLoc(), statements, SourceLoc());
-  return new (C) GuardStmt(SourceLoc(), C.AllocateCopy(conditions), body);
-}
-
 static std::pair<BraceStmt *, bool>
 deriveBodyEquatable_enum_uninhabited_eq(AbstractFunctionDecl *eqDecl, void *) {
   auto parentDC = eqDecl->getDeclContext();
@@ -225,9 +186,9 @@ deriveBodyEquatable_enum_noAssociatedValues_eq(AbstractFunctionDecl *eqDecl,
 
   // Generate the conversion from the enums to integer indices.
   SmallVector<ASTNode, 6> statements;
-  DeclRefExpr *aIndex = convertEnumToIndex(statements, parentDC, enumDecl,
+  DeclRefExpr *aIndex = DerivedConformance::convertEnumToIndex(statements, parentDC, enumDecl,
                                            aParam, eqDecl, "index_a");
-  DeclRefExpr *bIndex = convertEnumToIndex(statements, parentDC, enumDecl,
+  DeclRefExpr *bIndex = DerivedConformance::convertEnumToIndex(statements, parentDC, enumDecl,
                                            bParam, eqDecl, "index_b");
 
   // Generate the compare of the indices.
@@ -296,7 +257,7 @@ deriveBodyEquatable_enum_hasAssociatedValues_eq(AbstractFunctionDecl *eqDecl,
 
     // .<elt>(let l0, let l1, ...)
     SmallVector<VarDecl*, 3> lhsPayloadVars;
-    auto lhsSubpattern = enumElementPayloadSubpattern(elt, 'l', eqDecl,
+    auto lhsSubpattern = DerivedConformance::enumElementPayloadSubpattern(elt, 'l', eqDecl,
                                                       lhsPayloadVars);
     auto lhsElemPat = new (C) EnumElementPattern(TypeLoc::withoutLoc(enumType),
                                                  SourceLoc(), DeclNameLoc(),
@@ -306,7 +267,7 @@ deriveBodyEquatable_enum_hasAssociatedValues_eq(AbstractFunctionDecl *eqDecl,
 
     // .<elt>(let r0, let r1, ...)
     SmallVector<VarDecl*, 3> rhsPayloadVars;
-    auto rhsSubpattern = enumElementPayloadSubpattern(elt, 'r', eqDecl,
+    auto rhsSubpattern = DerivedConformance::enumElementPayloadSubpattern(elt, 'r', eqDecl,
                                                       rhsPayloadVars);
     auto rhsElemPat = new (C) EnumElementPattern(TypeLoc::withoutLoc(enumType),
                                                  SourceLoc(), DeclNameLoc(),
@@ -352,7 +313,8 @@ deriveBodyEquatable_enum_hasAssociatedValues_eq(AbstractFunctionDecl *eqDecl,
       auto rhsVar = rhsPayloadVars[varIdx];
       auto rhsExpr = new (C) DeclRefExpr(rhsVar, DeclNameLoc(),
                                          /*Implicit*/true);
-      auto guardStmt = returnIfNotEqualGuard(C, lhsExpr, rhsExpr);
+      auto guardStmt = DerivedConformance::returnFalseIfNotEqualGuard(C, 
+          lhsExpr, rhsExpr);
       statementsInCase.emplace_back(guardStmt);
     }
 
@@ -438,7 +400,8 @@ deriveBodyEquatable_struct_eq(AbstractFunctionDecl *eqDecl, void *) {
     auto bPropertyExpr = new (C) DotSyntaxCallExpr(bPropertyRef, SourceLoc(),
                                                    bParamRef);
 
-    auto guardStmt = returnIfNotEqualGuard(C, aPropertyExpr, bPropertyExpr);
+    auto guardStmt = DerivedConformance::returnFalseIfNotEqualGuard(C,
+      aPropertyExpr, bPropertyExpr);
     statements.emplace_back(guardStmt);
   }
 
@@ -763,7 +726,7 @@ deriveBodyHashable_enum_noAssociatedValues_hashInto(
 
   // generate: switch self {...}
   SmallVector<ASTNode, 3> stmts;
-  auto discriminatorExpr = convertEnumToIndex(stmts, parentDC, enumDecl,
+  auto discriminatorExpr = DerivedConformance::convertEnumToIndex(stmts, parentDC, enumDecl,
                                               selfDecl, hashIntoDecl,
                                               "discriminator");
   // generate: hasher.combine(discriminator)
@@ -818,7 +781,7 @@ deriveBodyHashable_enum_hasAssociatedValues_hashInto(
     SmallVector<VarDecl*, 3> payloadVars;
     SmallVector<ASTNode, 3> statements;
 
-    auto payloadPattern = enumElementPayloadSubpattern(elt, 'a', hashIntoDecl,
+    auto payloadPattern = DerivedConformance::enumElementPayloadSubpattern(elt, 'a', hashIntoDecl,
                                                        payloadVars);
     auto pat = new (C) EnumElementPattern(TypeLoc::withoutLoc(enumType),
                                           SourceLoc(), DeclNameLoc(),
diff --git a/lib/Sema/DerivedConformances.cpp b/lib/Sema/DerivedConformances.cpp
diff --git a/lib/Sema/DerivedConformances.h b/lib/Sema/DerivedConformances.h
