about summary refs log tree commit diff
diff options
context:
space:
mode:
authoraszlig <aszlig@nix.build>2018-08-28T22·23+0200
committeraszlig <aszlig@nix.build>2018-08-28T23·05+0200
commit0ad643ed5c9642738a8e8b37b7e4b1835f160bc8 (patch)
tree9e543e44240dd1859d8e791ec8b99b187502562c
parentc651b7bdc996a18688c5e5dd5dd84eeeb8d3376d (diff)
libexpr: Use int64_t for NixInt
Using a 64bit integer on 32bit systems will come with a bit of a
performance overhead, but given that Nix doesn't use a lot of integers
compared to other types, I think the overhead is negligible also
considering that 32bit systems are in decline.

The biggest advantage however is that when we use a consistent integer
size across all platforms it's less likely that we miss things that we
break due to that. One example would be:

https://github.com/NixOS/nixpkgs/pull/44233

On Hydra it will evaluate, because the evaluator runs on a 64bit
machine, but when evaluating the same on a 32bit machine it will fail,
so using 64bit integers should make that consistent.

While the change of the type in value.hh is rather easy to do, we have a
few more options available for doing the conversion in the lexer:

  * Via an #ifdef on the architecture and using strtol() or strtoll()
    accordingly depending on which architecture we are. For the #ifdef
    we would need another AX_COMPILE_CHECK_SIZEOF in configure.ac.
  * Using istringstream, which would involve copying the value.
  * As we're already using boost, lexical_cast might be a good idea.

Spoiler: I went for the latter, first of all because lexical_cast does
have an overload for const char* and second of all, because it doesn't
involve copying around the input string. Also, because istringstream
seems to come with a bigger overhead than boost::lexical_cast:

https://www.boost.org/doc/libs/release/doc/html/boost_lexical_cast/performance.html

The first method (still using strtol/strtoll) also wasn't something I
pursued further, because it is also locale-aware which I doubt is what
we want, given that the regex for int is [0-9]+.

Signed-off-by: aszlig <aszlig@nix.build>
Fixes: #2339
-rw-r--r--src/libexpr/lexer.l8
-rw-r--r--src/libexpr/value.hh2
2 files changed, 7 insertions, 3 deletions
diff --git a/src/libexpr/lexer.l b/src/libexpr/lexer.l
index 29ca327c1e4e..a052447d3dce 100644
--- a/src/libexpr/lexer.l
+++ b/src/libexpr/lexer.l
@@ -12,6 +12,8 @@
 
 
 %{
+#include <boost/lexical_cast.hpp>
+
 #include "nixexpr.hh"
 #include "parser-tab.hh"
 
@@ -124,9 +126,11 @@ or          { return OR_KW; }
 
 {ID}        { yylval->id = strdup(yytext); return ID; }
 {INT}       { errno = 0;
-              yylval->n = strtol(yytext, 0, 10);
-              if (errno != 0)
+              try {
+                  yylval->n = boost::lexical_cast<int64_t>(yytext);
+              } catch (const boost::bad_lexical_cast &) {
                   throw ParseError(format("invalid integer '%1%'") % yytext);
+              }
               return INT;
             }
 {FLOAT}     { errno = 0;
diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh
index 809772f7c084..e1ec87d3b84c 100644
--- a/src/libexpr/value.hh
+++ b/src/libexpr/value.hh
@@ -43,7 +43,7 @@ class XMLWriter;
 class JSONPlaceholder;
 
 
-typedef long NixInt;
+typedef int64_t NixInt;
 typedef double NixFloat;
 
 /* External values must descend from ExternalValueBase, so that