Use std::strong_ordering for version comparison

The actual motive here is the avoidance of integer overflow if we were
to make these use checked NixInts and retain the subtraction.

However, the actual *intent* of this code is a three-way comparison,
which can be done with operator<=>, so we should just do *that* instead.

Change-Id: I7f9a7da1f3176424b528af6d1b4f1591e4ab26bf

4 files changed, 18 insertions, 17 deletions

diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index 72ae1aeb9..80c64232f 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -4162,7 +4162,8 @@ static void prim_compareVersions(EvalState & state, const PosIdx pos, Value * *
 {
     auto version1 = state.forceStringNoCtx(*args[0], pos, "while evaluating the first argument passed to builtins.compareVersions");
     auto version2 = state.forceStringNoCtx(*args[1], pos, "while evaluating the second argument passed to builtins.compareVersions");
-    v.mkInt(compareVersions(version1, version2));
+    auto result = compareVersions(version1, version2);
+    v.mkInt(result < 0 ? -1 : result > 0 ? 1 : 0);
 }
 
 static RegisterPrimOp primop_compareVersions({
diff --git a/src/libstore/names.cc b/src/libstore/names.cc
index b903a99f0..d72530d6f 100644
--- a/src/libstore/names.cc
+++ b/src/libstore/names.cc
@@ -94,7 +94,7 @@ static bool componentsLT(const std::string_view c1, const std::string_view c2)
 }
 
 
-int compareVersions(const std::string_view v1, const std::string_view v2)
+std::strong_ordering compareVersions(const std::string_view v1, const std::string_view v2)
 {
     auto p1 = v1.begin();
     auto p2 = v2.begin();
@@ -102,11 +102,11 @@ int compareVersions(const std::string_view v1, const std::string_view v2)
     while (p1 != v1.end() || p2 != v2.end()) {
         auto c1 = nextComponent(p1, v1.end());
         auto c2 = nextComponent(p2, v2.end());
-        if (componentsLT(c1, c2)) return -1;
-        else if (componentsLT(c2, c1)) return 1;
+        if (componentsLT(c1, c2)) return std::strong_ordering::less;
+        else if (componentsLT(c2, c1)) return std::strong_ordering::greater;
     }
 
-    return 0;
+    return std::strong_ordering::equal;
 }
 
 
diff --git a/src/libstore/names.hh b/src/libstore/names.hh
index d82b99bb4..a6909d545 100644
--- a/src/libstore/names.hh
+++ b/src/libstore/names.hh
@@ -30,7 +30,7 @@ typedef std::list<DrvName> DrvNames;
 
 std::string_view nextComponent(std::string_view::const_iterator & p,
     const std::string_view::const_iterator end);
-int compareVersions(const std::string_view v1, const std::string_view v2);
+std::strong_ordering compareVersions(const std::string_view v1, const std::string_view v2);
 DrvNames drvNamesFromArgs(const Strings & opArgs);
 
 }
diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc
index fdac7e69a..6dda34a32 100644
--- a/src/nix-env/nix-env.cc
+++ b/src/nix-env/nix-env.cc
@@ -203,15 +203,15 @@ static void loadDerivations(EvalState & state, const SourcePath & nixExprPath,
 }
 
 
-static long getPriority(EvalState & state, DrvInfo & drv)
+static NixInt getPriority(EvalState & state, DrvInfo & drv)
 {
-    return drv.queryMetaInt("priority", 0);
+    return drv.queryMetaInt("priority", NixInt(0));
 }
 
 
-static long comparePriorities(EvalState & state, DrvInfo & drv1, DrvInfo & drv2)
+static std::strong_ordering comparePriorities(EvalState & state, DrvInfo & drv1, DrvInfo & drv2)
 {
-    return getPriority(state, drv2) - getPriority(state, drv1);
+    return getPriority(state, drv2) <=> getPriority(state, drv1);
 }
 
 
@@ -279,7 +279,7 @@ std::vector<Match> pickNewestOnly(EvalState & state, std::vector<Match> matches)
         auto & oneDrv = match.drvInfo;
 
         const auto drvName = DrvName { oneDrv.queryName() };
-        long comparison = 1;
+        std::strong_ordering comparison = std::strong_ordering::greater;
 
         const auto itOther = newest.find(drvName.name);
 
@@ -287,9 +287,9 @@ std::vector<Match> pickNewestOnly(EvalState & state, std::vector<Match> matches)
             auto & newestDrv = itOther->second.drvInfo;
 
             comparison =
-                oneDrv.querySystem() == newestDrv.querySystem() ? 0 :
-                oneDrv.querySystem() == settings.thisSystem ? 1 :
-                newestDrv.querySystem() == settings.thisSystem ? -1 : 0;
+                oneDrv.querySystem() == newestDrv.querySystem() ? std::strong_ordering::equal :
+                oneDrv.querySystem() == settings.thisSystem ? std::strong_ordering::greater :
+                newestDrv.querySystem() == settings.thisSystem ? std::strong_ordering::less : std::strong_ordering::equal;
             if (comparison == 0)
                 comparison = comparePriorities(state, oneDrv, newestDrv);
             if (comparison == 0)
@@ -627,13 +627,13 @@ static void upgradeDerivations(Globals & globals,
                         continue;
                     DrvName newName(j->queryName());
                     if (newName.name == drvName.name) {
-                        int d = compareVersions(drvName.version, newName.version);
+                        std::strong_ordering d = compareVersions(drvName.version, newName.version);
                         if ((upgradeType == utLt && d < 0) ||
                             (upgradeType == utLeq && d <= 0) ||
                             (upgradeType == utEq && d == 0) ||
                             upgradeType == utAlways)
                         {
-                            long d2 = -1;
+                            std::strong_ordering d2 = std::strong_ordering::less;
                             if (bestElem != availElems.end()) {
                                 d2 = comparePriorities(*globals.state, *bestElem, *j);
                                 if (d2 == 0) d2 = compareVersions(bestVersion, newName.version);
@@ -904,7 +904,7 @@ static VersionDiff compareVersionAgainstSet(
     for (auto & i : elems) {
         DrvName name2(i.queryName());
         if (name.name == name2.name) {
-            int d = compareVersions(name.version, name2.version);
+            std::strong_ordering d = compareVersions(name.version, name2.version);
             if (d < 0) {
                 diff = cvGreater;
                 version = name2.version;