[PATCH] D55468: Use zip_longest for list comparisons.

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur created this revision.
Meinersbur added reviewers: dblaikie, hfinkel.

Use zip_longest in two locations that compare iterator ranges. zip_longest allows the iteration using a range-based for-loop and to be symmetric over both ranges instead of prioritizing one over the other. In that latter case code have to handle the case that the first is longer than the second, the second is longer than the first, and both are of the same length, which must partially be checked after the loop.

With zip_longest, this becomes an element comparison within the loop like the comparison of the elements themselves. The symmetry makes it clearer that neither the first and second iterators are handled differently. The iterators are not event used directly anymore, just the ranges.


Repository:
  rC Clang

https://reviews.llvm.org/D55468

Files:
  lib/Sema/SemaOverload.cpp
  lib/Serialization/ASTReaderDecl.cpp


Index: lib/Serialization/ASTReaderDecl.cpp
===================================================================
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -2913,25 +2913,27 @@
   // Note that pass_object_size attributes are represented in the function's
   // ExtParameterInfo, so we don't need to check them here.
 
-  // Return false if any of the enable_if expressions of A and B are different.
   llvm::FoldingSetNodeID Cand1ID, Cand2ID;
   auto AEnableIfAttrs = A->specific_attrs<EnableIfAttr>();
   auto BEnableIfAttrs = B->specific_attrs<EnableIfAttr>();
-  auto AEnableIf = AEnableIfAttrs.begin();
-  auto BEnableIf = BEnableIfAttrs.begin();
-  for (; AEnableIf != AEnableIfAttrs.end() && BEnableIf != BEnableIfAttrs.end();
-       ++BEnableIf, ++AEnableIf) {
+
+  for (auto Pair : zip_longest(AEnableIfAttrs, BEnableIfAttrs)) {
+    // Return false if the number of enable_if attributes is different.
+    if (std::get<0>(Pair).hasValue() != std::get<1>(Pair).hasValue())
+      return false;
+
     Cand1ID.clear();
     Cand2ID.clear();
 
-    AEnableIf->getCond()->Profile(Cand1ID, A->getASTContext(), true);
-    BEnableIf->getCond()->Profile(Cand2ID, B->getASTContext(), true);
+    (*std::get<0>(Pair))->getCond()->Profile(Cand1ID, A->getASTContext(), true);
+    (*std::get<1>(Pair))->getCond()->Profile(Cand2ID, B->getASTContext(), true);
+
+    // Return false if any of the enable_if expressions of A and B are
+    // different.
     if (Cand1ID != Cand2ID)
       return false;
   }
-
-  // Return false if the number of enable_if attributes was different.
-  return AEnableIf == AEnableIfAttrs.end() && BEnableIf == BEnableIfAttrs.end();
+  return true;
 }
 
 /// Determine whether the two declarations refer to the same entity.
Index: lib/Sema/SemaOverload.cpp
===================================================================
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -8972,25 +8972,25 @@
   auto Cand1Attrs = Cand1->specific_attrs<EnableIfAttr>();
   auto Cand2Attrs = Cand2->specific_attrs<EnableIfAttr>();
 
-  auto Cand1I = Cand1Attrs.begin();
   llvm::FoldingSetNodeID Cand1ID, Cand2ID;
-  for (EnableIfAttr *Cand2A : Cand2Attrs) {
-    Cand1ID.clear();
-    Cand2ID.clear();
-
+  for (auto Pair : zip_longest(Cand1Attrs, Cand2Attrs)) {
     // It's impossible for Cand1 to be better than (or equal to) Cand2 if Cand1
-    // has fewer enable_if attributes than Cand2.
-    auto Cand1A = Cand1I++;
-    if (Cand1A == Cand1Attrs.end())
+    // has fewer enable_if attributes than Cand2, and vice versa.
+    if (std::get<0>(Pair) == None)
       return Comparison::Worse;
+    if (std::get<1>(Pair) == None)
+      return Comparison::Better;
+
+    Cand1ID.clear();
+    Cand2ID.clear();
 
-    Cand1A->getCond()->Profile(Cand1ID, S.getASTContext(), true);
-    Cand2A->getCond()->Profile(Cand2ID, S.getASTContext(), true);
+    (*std::get<0>(Pair))->getCond()->Profile(Cand1ID, S.getASTContext(), true);
+    (*std::get<1>(Pair))->getCond()->Profile(Cand2ID, S.getASTContext(), true);
     if (Cand1ID != Cand2ID)
       return Comparison::Worse;
   }
 
-  return Cand1I == Cand1Attrs.end() ? Comparison::Equal : Comparison::Better;
+  return Comparison::Equal;
 }
 
 static bool isBetterMultiversionCandidate(const OverloadCandidate &Cand1,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D55468.177369.patch
Type: text/x-patch
Size: 3333 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20181208/544b01c2/attachment-0001.bin>