From 0ad643ed5c9642738a8e8b37b7e4b1835f160bc8 Mon Sep 17 00:00:00 2001 From: aszlig Date: Wed, 29 Aug 2018 00:23:51 +0200 Subject: [PATCH] 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 Fixes: #2339 --- src/libexpr/lexer.l | 8 ++++++-- src/libexpr/value.hh | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/libexpr/lexer.l b/src/libexpr/lexer.l index 29ca327c1..a052447d3 100644 --- a/src/libexpr/lexer.l +++ b/src/libexpr/lexer.l @@ -12,6 +12,8 @@ %{ +#include + #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(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 809772f7c..e1ec87d3b 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