string2Int(): Return std::optional

17 files changed, 82 insertions, 69 deletions

diff --git a/src/libexpr/attr-path.cc b/src/libexpr/attr-path.cc
index 2d37dcb7e..9dd557205 100644
--- a/src/libexpr/attr-path.cc
+++ b/src/libexpr/attr-path.cc
@@ -52,9 +52,7 @@ std::pair<Value *, Pos> findAlongAttrPath(EvalState & state, const string & attr
     for (auto & attr : tokens) {
 
         /* Is i an index (integer) or a normal attribute name? */
-        enum { apAttr, apIndex } apType = apAttr;
-        unsigned int attrIndex;
-        if (string2Int(attr, attrIndex)) apType = apIndex;
+        auto attrIndex = string2Int<unsigned int>(attr);
 
         /* Evaluate the expression. */
         Value * vNew = state.allocValue();
@@ -65,7 +63,7 @@ std::pair<Value *, Pos> findAlongAttrPath(EvalState & state, const string & attr
         /* It should evaluate to either a set or an expression,
            according to what is specified in the attrPath. */
 
-        if (apType == apAttr) {
+        if (!attrIndex) {
 
             if (v->type() != nAttrs)
                 throw TypeError(
@@ -82,17 +80,17 @@ std::pair<Value *, Pos> findAlongAttrPath(EvalState & state, const string & attr
             pos = *a->pos;
         }
 
-        else if (apType == apIndex) {
+        else {
 
             if (!v->isList())
                 throw TypeError(
                     "the expression selected by the selection path '%1%' should be a list but is %2%",
                     attrPath,
                     showType(*v));
-            if (attrIndex >= v->listSize())
-                throw AttrPathNotFound("list index %1% in selection path '%2%' is out of range", attrIndex, attrPath);
+            if (*attrIndex >= v->listSize())
+                throw AttrPathNotFound("list index %1% in selection path '%2%' is out of range", *attrIndex, attrPath);
 
-            v = v->listElems()[attrIndex];
+            v = v->listElems()[*attrIndex];
             pos = noPos;
         }
 
diff --git a/src/libexpr/get-drvs.cc b/src/libexpr/get-drvs.cc
index 32c115c12..1a3990ea1 100644
--- a/src/libexpr/get-drvs.cc
+++ b/src/libexpr/get-drvs.cc
@@ -214,8 +214,8 @@ NixInt DrvInfo::queryMetaInt(const string & name, NixInt def)
     if (v->type() == nString) {
         /* Backwards compatibility with before we had support for
            integer meta fields. */
-        NixInt n;
-        if (string2Int(v->string.s, n)) return n;
+        if (auto n = string2Int<NixInt>(v->string.s))
+            return *n;
     }
     return def;
 }
@@ -228,8 +228,8 @@ NixFloat DrvInfo::queryMetaFloat(const string & name, NixFloat def)
     if (v->type() == nString) {
         /* Backwards compatibility with before we had support for
            float meta fields. */
-        NixFloat n;
-        if (string2Float(v->string.s, n)) return n;
+        if (auto n = string2Float<NixFloat>(v->string.s))
+            return *n;
     }
     return def;
 }
diff --git a/src/libfetchers/path.cc b/src/libfetchers/path.cc
index bcb904c0d..d1003de57 100644
--- a/src/libfetchers/path.cc
+++ b/src/libfetchers/path.cc
@@ -20,10 +20,10 @@ struct PathInputScheme : InputScheme
             if (name == "rev" || name == "narHash")
                 input.attrs.insert_or_assign(name, value);
             else if (name == "revCount" || name == "lastModified") {
-                uint64_t n;
-                if (!string2Int(value, n))
+                if (auto n = string2Int<uint64_t>(value))
+                    input.attrs.insert_or_assign(name, *n);
+                else
                     throw Error("path URL '%s' has invalid parameter '%s'", url.to_string(), name);
-                input.attrs.insert_or_assign(name, n);
             }
             else
                 throw Error("path URL '%s' has unsupported parameter '%s'", url.to_string(), name);
diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc
index 223020378..f1feeddd6 100644
--- a/src/libmain/shared.cc
+++ b/src/libmain/shared.cc
@@ -219,10 +219,10 @@ LegacyArgs::LegacyArgs(const std::string & programName,
             .description = description,
             .labels = {"n"},
             .handler = {[=](std::string s) {
-                unsigned int n;
-                if (!string2Int(s, n))
+                if (auto n = string2Int<unsigned int>(s))
+                    settings.set(dest, std::to_string(*n));
+                else
                     throw UsageError("'%s' is not an integer", s);
-                settings.set(dest, std::to_string(n));
             }}
         });
     };
diff --git a/src/libmain/shared.hh b/src/libmain/shared.hh
index ffae5d796..38f627b44 100644
--- a/src/libmain/shared.hh
+++ b/src/libmain/shared.hh
@@ -70,10 +70,9 @@ template<class N> N getIntArg(const string & opt,
             s.resize(s.size() - 1);
         }
     }
-    N n;
-    if (!string2Int(s, n))
-        throw UsageError("'%1%' requires an integer argument", opt);
-    return n * multiplier;
+    if (auto n = string2Int<N>(s))
+        return *n * multiplier;
+    throw UsageError("'%1%' requires an integer argument", opt);
 }
 
 
diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc
index af3ab87a9..35f365795 100644
--- a/src/libstore/build/derivation-goal.cc
+++ b/src/libstore/build/derivation-goal.cc
@@ -1699,12 +1699,10 @@ void DerivationGoal::startBuilder()
             userNamespaceSync.writeSide = -1;
         });
 
-        pid_t tmp;
         auto ss = tokenizeString<std::vector<std::string>>(readLine(builderOut.readSide.get()));
         assert(ss.size() == 2);
         usingUserNamespace = ss[0] == "1";
-        if (!string2Int<pid_t>(ss[1], tmp)) abort();
-        pid = tmp;
+        pid = string2Int<pid_t>(ss[1]).value();
 
         if (usingUserNamespace) {
             /* Set the UID/GID mapping of the builder's user namespace
diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc
index ad66ef8a8..0531aad9f 100644
--- a/src/libstore/globals.cc
+++ b/src/libstore/globals.cc
@@ -228,8 +228,12 @@ template<> void BaseSetting<SandboxMode>::convertToArg(Args & args, const std::s
 void MaxBuildJobsSetting::set(const std::string & str, bool append)
 {
     if (str == "auto") value = std::max(1U, std::thread::hardware_concurrency());
-    else if (!string2Int(str, value))
-        throw UsageError("configuration setting '%s' should be 'auto' or an integer", name);
+    else {
+        if (auto n = string2Int<decltype(value)>(str))
+            value = *n;
+        else
+            throw UsageError("configuration setting '%s' should be 'auto' or an integer", name);
+    }
 }
 
 
diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
index 702e7b136..c61f34275 100644
--- a/src/libstore/local-store.cc
+++ b/src/libstore/local-store.cc
@@ -66,8 +66,10 @@ int getSchema(Path schemaPath)
     int curSchema = 0;
     if (pathExists(schemaPath)) {
         string s = readFile(schemaPath);
-        if (!string2Int(s, curSchema))
+        auto n = string2Int<int>(s);
+        if (!n)
             throw Error("'%1%' is corrupt", schemaPath);
+        curSchema = *n;
     }
     return curSchema;
 }
diff --git a/src/libstore/names.cc b/src/libstore/names.cc
index 41e28dc99..ce808accc 100644
--- a/src/libstore/names.cc
+++ b/src/libstore/names.cc
@@ -80,16 +80,16 @@ string nextComponent(string::const_iterator & p,
 
 static bool componentsLT(const string & c1, const string & c2)
 {
-    int n1, n2;
-    bool c1Num = string2Int(c1, n1), c2Num = string2Int(c2, n2);
+    auto n1 = string2Int<int>(c1);
+    auto n2 = string2Int<int>(c2);
 
-    if (c1Num && c2Num) return n1 < n2;
-    else if (c1 == "" && c2Num) return true;
+    if (n1 && n2) return *n1 < *n2;
+    else if (c1 == "" && n2) return true;
     else if (c1 == "pre" && c2 != "pre") return true;
     else if (c2 == "pre") return false;
     /* Assume that `2.3a' < `2.3.1'. */
-    else if (c2Num) return true;
-    else if (c1Num) return false;
+    else if (n2) return true;
+    else if (n1) return false;
     else return c1 < c2;
 }
 
diff --git a/src/libstore/nar-info.cc b/src/libstore/nar-info.cc
index 3454f34bb..49079388a 100644
--- a/src/libstore/nar-info.cc
+++ b/src/libstore/nar-info.cc
@@ -46,14 +46,18 @@ NarInfo::NarInfo(const Store & store, const std::string & s, const std::string &
         else if (name == "FileHash")
             fileHash = parseHashField(value);
         else if (name == "FileSize") {
-            if (!string2Int(value, fileSize)) throw corrupt();
+            auto n = string2Int<decltype(fileSize)>(value);
+            if (!n) throw corrupt();
+            fileSize = *n;
         }
         else if (name == "NarHash") {
             narHash = parseHashField(value);
             haveNarHash = true;
         }
         else if (name == "NarSize") {
-            if (!string2Int(value, narSize)) throw corrupt();
+            auto n = string2Int<decltype(narSize)>(value);
+            if (!n) throw corrupt();
+            narSize = *n;
         }
         else if (name == "References") {
             auto refs = tokenizeString<Strings>(value, " ");
diff --git a/src/libstore/profiles.cc b/src/libstore/profiles.cc
index ed10dd519..5d1723886 100644
--- a/src/libstore/profiles.cc
+++ b/src/libstore/profiles.cc
@@ -21,9 +21,8 @@ static std::optional<GenerationNumber> parseName(const string & profileName, con
     string s = string(name, profileName.size() + 1);
     string::size_type p = s.find("-link");
     if (p == string::npos) return {};
-    unsigned int n;
-    if (string2Int(string(s, 0, p), n) && n >= 0)
-        return n;
+    if (auto n = string2Int<unsigned int>(s.substr(0, p)))
+        return *n;
     else
         return {};
 }
@@ -214,12 +213,12 @@ void deleteGenerationsOlderThan(const Path & profile, const string & timeSpec, b
 {
     time_t curTime = time(0);
     string strDays = string(timeSpec, 0, timeSpec.size() - 1);
-    int days;
+    auto days = string2Int<int>(strDays);
 
-    if (!string2Int(strDays, days) || days < 1)
+    if (!days || *days < 1)
         throw Error("invalid number of days specifier '%1%'", timeSpec);
 
-    time_t oldTime = curTime - days * 24 * 3600;
+    time_t oldTime = curTime - *days * 24 * 3600;
 
     deleteGenerationsOlderThan(profile, oldTime, dryRun);
 }
diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc
index 7aca22bde..01e2fcc7b 100644
--- a/src/libstore/store-api.cc
+++ b/src/libstore/store-api.cc
@@ -932,19 +932,20 @@ std::optional<ValidPathInfo> decodeValidPathInfo(const Store & store, std::istre
         getline(str, s);
         auto narHash = Hash::parseAny(s, htSHA256);
         getline(str, s);
-        uint64_t narSize;
-        if (!string2Int(s, narSize)) throw Error("number expected");
-        hashGiven = { narHash, narSize };
+        auto narSize = string2Int<uint64_t>(s);
+        if (!narSize) throw Error("number expected");
+        hashGiven = { narHash, *narSize };
     }
     ValidPathInfo info(store.parseStorePath(path), hashGiven->first);
     info.narSize = hashGiven->second;
     std::string deriver;
     getline(str, deriver);
     if (deriver != "") info.deriver = store.parseStorePath(deriver);
-    string s; int n;
+    string s;
     getline(str, s);
-    if (!string2Int(s, n)) throw Error("number expected");
-    while (n--) {
+    auto n = string2Int<int>(s);
+    if (!n) throw Error("number expected");
+    while ((*n)--) {
         getline(str, s);
         info.references.insert(store.parseStorePath(s));
     }
diff --git a/src/libutil/args.hh b/src/libutil/args.hh
index 62b9516d8..823d843aa 100644
--- a/src/libutil/args.hh
+++ b/src/libutil/args.hh
@@ -87,7 +87,9 @@ protected:
         template<class I>
         Handler(I * dest)
             : fun([=](std::vector<std::string> ss) {
-                if (!string2Int(ss[0], *dest))
+                if (auto n = string2Int<I>(ss[0]))
+                    *dest = *n;
+                else
                     throw UsageError("'%s' is not an integer", ss[0]);
               })
             , arity(1)
diff --git a/src/libutil/config.cc b/src/libutil/config.cc
index 7af3e7883..7467e5ac0 100644
--- a/src/libutil/config.cc
+++ b/src/libutil/config.cc
@@ -230,7 +230,9 @@ template<typename T>
 void BaseSetting<T>::set(const std::string & str, bool append)
 {
     static_assert(std::is_integral<T>::value, "Integer required.");
-    if (!string2Int(str, value))
+    if (auto n = string2Int<T>(str))
+        value = *n;
+    else
         throw UsageError("setting '%s' has invalid value '%s'", name, str);
 }
 
diff --git a/src/libutil/util.hh b/src/libutil/util.hh
index 0f82bed78..7a4d5fe92 100644
--- a/src/libutil/util.hh
+++ b/src/libutil/util.hh
@@ -397,21 +397,27 @@ bool statusOk(int status);
 
 
 /* Parse a string into an integer. */
-template<class N> bool string2Int(const string & s, N & n)
+template<class N>
+std::optional<N> string2Int(const std::string & s)
 {
-    if (string(s, 0, 1) == "-" && !std::numeric_limits<N>::is_signed)
-        return false;
+    if (s.substr(0, 1) == "-" && !std::numeric_limits<N>::is_signed)
+        return {};
     std::istringstream str(s);
+    N n;
     str >> n;
-    return str && str.get() == EOF;
+    if (str && str.get() == EOF) return n;
+    return {};
 }
 
 /* Parse a string into a float. */
-template<class N> bool string2Float(const string & s, N & n)
+template<class N>
+std::optional<N> string2Float(const string & s)
 {
     std::istringstream str(s);
+    N n;
     str >> n;
-    return str && str.get() == EOF;
+    if (str && str.get() == EOF) return n;
+    return {};
 }
 
 
diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc
index 6c2e075ed..9963f05d9 100644
--- a/src/nix-env/nix-env.cc
+++ b/src/nix-env/nix-env.cc
@@ -1250,11 +1250,10 @@ static void opSwitchGeneration(Globals & globals, Strings opFlags, Strings opArg
     if (opArgs.size() != 1)
         throw UsageError("exactly one argument expected");
 
-    GenerationNumber dstGen;
-    if (!string2Int(opArgs.front(), dstGen))
+    if (auto dstGen = string2Int<GenerationNumber>(opArgs.front()))
+        switchGeneration(globals, *dstGen);
+    else
         throw UsageError("expected a generation number");
-
-    switchGeneration(globals, dstGen);
 }
 
 
@@ -1308,17 +1307,17 @@ static void opDeleteGenerations(Globals & globals, Strings opFlags, Strings opAr
         if(opArgs.front().size() < 2)
             throw Error("invalid number of generations ‘%1%’", opArgs.front());
         string str_max = string(opArgs.front(), 1, opArgs.front().size());
-        GenerationNumber max;
-        if (!string2Int(str_max, max) || max == 0)
+        auto max = string2Int<GenerationNumber>(str_max);
+        if (!max || *max == 0)
             throw Error("invalid number of generations to keep ‘%1%’", opArgs.front());
-        deleteGenerationsGreaterThan(globals.profile, max, globals.dryRun);
+        deleteGenerationsGreaterThan(globals.profile, *max, globals.dryRun);
     } else {
         std::set<GenerationNumber> gens;
         for (auto & i : opArgs) {
-            GenerationNumber n;
-            if (!string2Int(i, n))
+            if (auto n = string2Int<GenerationNumber>(i))
+                gens.insert(*n);
+            else
                 throw UsageError("invalid generation number '%1%'", i);
-            gens.insert(n);
         }
         deleteGenerations(globals.profile, gens, globals.dryRun);
     }
diff --git a/src/nix/profile.cc b/src/nix/profile.cc
index d8d2b3a70..8cdd34a20 100644
--- a/src/nix/profile.cc
+++ b/src/nix/profile.cc
@@ -209,9 +209,8 @@ public:
         std::vector<Matcher> res;
 
         for (auto & s : _matchers) {
-            size_t n;
-            if (string2Int(s, n))
-                res.push_back(n);
+            if (auto n = string2Int<size_t>(s))
+                res.push_back(*n);
             else if (store->isStorePath(s))
                 res.push_back(s);
             else