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
This commit is contained in:
jade 2024-07-12 14:56:30 +02:00
parent 4b109ec1a8
commit dde51af97d
4 changed files with 18 additions and 17 deletions

View file

@ -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 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"); 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({ static RegisterPrimOp primop_compareVersions({

View file

@ -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 p1 = v1.begin();
auto p2 = v2.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()) { while (p1 != v1.end() || p2 != v2.end()) {
auto c1 = nextComponent(p1, v1.end()); auto c1 = nextComponent(p1, v1.end());
auto c2 = nextComponent(p2, v2.end()); auto c2 = nextComponent(p2, v2.end());
if (componentsLT(c1, c2)) return -1; if (componentsLT(c1, c2)) return std::strong_ordering::less;
else if (componentsLT(c2, c1)) return 1; else if (componentsLT(c2, c1)) return std::strong_ordering::greater;
} }
return 0; return std::strong_ordering::equal;
} }

View file

@ -30,7 +30,7 @@ typedef std::list<DrvName> DrvNames;
std::string_view nextComponent(std::string_view::const_iterator & p, std::string_view nextComponent(std::string_view::const_iterator & p,
const std::string_view::const_iterator end); 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); DrvNames drvNamesFromArgs(const Strings & opArgs);
} }

View file

@ -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; auto & oneDrv = match.drvInfo;
const auto drvName = DrvName { oneDrv.queryName() }; const auto drvName = DrvName { oneDrv.queryName() };
long comparison = 1; std::strong_ordering comparison = std::strong_ordering::greater;
const auto itOther = newest.find(drvName.name); 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; auto & newestDrv = itOther->second.drvInfo;
comparison = comparison =
oneDrv.querySystem() == newestDrv.querySystem() ? 0 : oneDrv.querySystem() == newestDrv.querySystem() ? std::strong_ordering::equal :
oneDrv.querySystem() == settings.thisSystem ? 1 : oneDrv.querySystem() == settings.thisSystem ? std::strong_ordering::greater :
newestDrv.querySystem() == settings.thisSystem ? -1 : 0; newestDrv.querySystem() == settings.thisSystem ? std::strong_ordering::less : std::strong_ordering::equal;
if (comparison == 0) if (comparison == 0)
comparison = comparePriorities(state, oneDrv, newestDrv); comparison = comparePriorities(state, oneDrv, newestDrv);
if (comparison == 0) if (comparison == 0)
@ -627,13 +627,13 @@ static void upgradeDerivations(Globals & globals,
continue; continue;
DrvName newName(j->queryName()); DrvName newName(j->queryName());
if (newName.name == drvName.name) { 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) || if ((upgradeType == utLt && d < 0) ||
(upgradeType == utLeq && d <= 0) || (upgradeType == utLeq && d <= 0) ||
(upgradeType == utEq && d == 0) || (upgradeType == utEq && d == 0) ||
upgradeType == utAlways) upgradeType == utAlways)
{ {
long d2 = -1; std::strong_ordering d2 = std::strong_ordering::less;
if (bestElem != availElems.end()) { if (bestElem != availElems.end()) {
d2 = comparePriorities(*globals.state, *bestElem, *j); d2 = comparePriorities(*globals.state, *bestElem, *j);
if (d2 == 0) d2 = compareVersions(bestVersion, newName.version); if (d2 == 0) d2 = compareVersions(bestVersion, newName.version);
@ -904,7 +904,7 @@ static VersionDiff compareVersionAgainstSet(
for (auto & i : elems) { for (auto & i : elems) {
DrvName name2(i.queryName()); DrvName name2(i.queryName());
if (name.name == name2.name) { if (name.name == name2.name) {
int d = compareVersions(name.version, name2.version); std::strong_ordering d = compareVersions(name.version, name2.version);
if (d < 0) { if (d < 0) {
diff = cvGreater; diff = cvGreater;
version = name2.version; version = name2.version;