libexpr/parser: Test experimental features

Currently, the parser relies on the global experimental feature flags.
In order to properly test conditional language features, we instead need
to pass it around in the parser::State.

This means that the parser cannot cache the result of isEnabled anymore,
which wouldn't necessarily hurt performance if the function didn't
perform a linear search on the list of enabled features on every single
call. While we could simply evaluate once at the start of parsing and
cache the result in the parser state, the more sustainable solution
would be to fix `isEnabled` such that all callers may profit from the
performance improvement.

Change-Id: Ic9b9c5d882b6270e1114988b63e6064d36c25cf2
This commit is contained in:
piegames 2024-07-22 21:26:17 +02:00
parent 27a63db710
commit ec7552ff74
6 changed files with 32 additions and 11 deletions

View file

@ -2808,20 +2808,20 @@ Expr & EvalState::parseExprFromFile(const SourcePath & path, std::shared_ptr<Sta
} }
Expr & EvalState::parseExprFromString(std::string s_, const SourcePath & basePath, std::shared_ptr<StaticEnv> & staticEnv) Expr & EvalState::parseExprFromString(std::string s_, const SourcePath & basePath, std::shared_ptr<StaticEnv> & staticEnv, const ExperimentalFeatureSettings & xpSettings)
{ {
// NOTE this method (and parseStdin) must take care to *fully copy* their input // NOTE this method (and parseStdin) must take care to *fully copy* their input
// into their respective Pos::Origin until the parser stops overwriting its input // into their respective Pos::Origin until the parser stops overwriting its input
// data. // data.
auto s = make_ref<std::string>(s_); auto s = make_ref<std::string>(s_);
s_.append("\0\0", 2); s_.append("\0\0", 2);
return *parse(s_.data(), s_.size(), Pos::String{.source = s}, basePath, staticEnv); return *parse(s_.data(), s_.size(), Pos::String{.source = s}, basePath, staticEnv, xpSettings);
} }
Expr & EvalState::parseExprFromString(std::string s, const SourcePath & basePath) Expr & EvalState::parseExprFromString(std::string s, const SourcePath & basePath, const ExperimentalFeatureSettings & xpSettings)
{ {
return parseExprFromString(std::move(s), basePath, staticBaseEnv); return parseExprFromString(std::move(s), basePath, staticBaseEnv, xpSettings);
} }

View file

@ -344,8 +344,8 @@ public:
/** /**
* Parse a Nix expression from the specified string. * Parse a Nix expression from the specified string.
*/ */
Expr & parseExprFromString(std::string s, const SourcePath & basePath, std::shared_ptr<StaticEnv> & staticEnv); Expr & parseExprFromString(std::string s, const SourcePath & basePath, std::shared_ptr<StaticEnv> & staticEnv, const ExperimentalFeatureSettings & xpSettings = experimentalFeatureSettings);
Expr & parseExprFromString(std::string s, const SourcePath & basePath); Expr & parseExprFromString(std::string s, const SourcePath & basePath, const ExperimentalFeatureSettings & xpSettings = experimentalFeatureSettings);
Expr & parseStdin(); Expr & parseStdin();
@ -568,7 +568,8 @@ private:
size_t length, size_t length,
Pos::Origin origin, Pos::Origin origin,
const SourcePath & basePath, const SourcePath & basePath,
std::shared_ptr<StaticEnv> & staticEnv); std::shared_ptr<StaticEnv> & staticEnv,
const ExperimentalFeatureSettings & xpSettings = experimentalFeatureSettings);
/** /**
* Current Nix call stack depth, used with `max-call-depth` setting to throw stack overflow hopefully before we run out of system stack. * Current Nix call stack depth, used with `max-call-depth` setting to throw stack overflow hopefully before we run out of system stack.

View file

@ -631,7 +631,7 @@ template<> struct BuildAST<grammar::expr::path> : p::maybe_nothing {};
template<> struct BuildAST<grammar::expr::uri> { template<> struct BuildAST<grammar::expr::uri> {
static void apply(const auto & in, ExprState & s, State & ps) { static void apply(const auto & in, ExprState & s, State & ps) {
static bool noURLLiterals = experimentalFeatureSettings.isEnabled(Xp::NoUrlLiterals); bool noURLLiterals = ps.xpSettings.isEnabled(Xp::NoUrlLiterals);
if (noURLLiterals) if (noURLLiterals)
throw ParseError({ throw ParseError({
.msg = HintFmt("URL literals are disabled"), .msg = HintFmt("URL literals are disabled"),
@ -832,7 +832,8 @@ Expr * EvalState::parse(
size_t length, size_t length,
Pos::Origin origin, Pos::Origin origin,
const SourcePath & basePath, const SourcePath & basePath,
std::shared_ptr<StaticEnv> & staticEnv) std::shared_ptr<StaticEnv> & staticEnv,
const ExperimentalFeatureSettings & xpSettings)
{ {
parser::State s = { parser::State s = {
symbols, symbols,
@ -840,6 +841,7 @@ Expr * EvalState::parse(
basePath, basePath,
positions.addOrigin(origin, length), positions.addOrigin(origin, length),
exprSymbols, exprSymbols,
xpSettings
}; };
parser::ExprState x; parser::ExprState x;

View file

@ -19,6 +19,7 @@ struct State
SourcePath basePath; SourcePath basePath;
PosTable::Origin origin; PosTable::Origin origin;
const Expr::AstSymbols & s; const Expr::AstSymbols & s;
const ExperimentalFeatureSettings & xpSettings;
void dupAttr(const AttrPath & attrPath, const PosIdx pos, const PosIdx prevPos); void dupAttr(const AttrPath & attrPath, const PosIdx pos, const PosIdx prevPos);
void dupAttr(Symbol attr, const PosIdx pos, const PosIdx prevPos); void dupAttr(Symbol attr, const PosIdx pos, const PosIdx prevPos);

View file

@ -26,9 +26,9 @@ namespace nix {
, state({}, store) , state({}, store)
{ {
} }
Value eval(std::string input, bool forceValue = true) { Value eval(std::string input, bool forceValue = true, const ExperimentalFeatureSettings & xpSettings = experimentalFeatureSettings) {
Value v; Value v;
Expr & e = state.parseExprFromString(input, state.rootPath(CanonPath::root)); Expr & e = state.parseExprFromString(input, state.rootPath(CanonPath::root), xpSettings);
state.eval(e, v); state.eval(e, v);
if (forceValue) if (forceValue)
state.forceValue(v, noPos); state.forceValue(v, noPos);

View file

@ -59,6 +59,11 @@ namespace nix {
ASSERT_THAT(v, IsFloatEq(1.234)); ASSERT_THAT(v, IsFloatEq(1.234));
} }
TEST_F(TrivialExpressionTest, pointfloat) {
auto v = eval(".234");
ASSERT_THAT(v, IsFloatEq(0.234));
}
TEST_F(TrivialExpressionTest, updateAttrs) { TEST_F(TrivialExpressionTest, updateAttrs) {
auto v = eval("{ a = 1; } // { b = 2; a = 3; }"); auto v = eval("{ a = 1; } // { b = 2; a = 3; }");
ASSERT_THAT(v, IsAttrsOfSize(2)); ASSERT_THAT(v, IsAttrsOfSize(2));
@ -81,6 +86,18 @@ namespace nix {
ASSERT_THAT(v, IsTrue()); ASSERT_THAT(v, IsTrue());
} }
TEST_F(TrivialExpressionTest, urlLiteral) {
auto v = eval("https://nixos.org");
ASSERT_THAT(v, IsStringEq("https://nixos.org"));
}
TEST_F(TrivialExpressionTest, noUrlLiteral) {
ExperimentalFeatureSettings mockXpSettings;
mockXpSettings.set("experimental-features", "no-url-literals");
ASSERT_THROW(eval("https://nixos.org", true, mockXpSettings), Error);
}
TEST_F(TrivialExpressionTest, withFound) { TEST_F(TrivialExpressionTest, withFound) {
auto v = eval("with { a = 23; }; a"); auto v = eval("with { a = 23; }; a");
ASSERT_THAT(v, IsIntEq(23)); ASSERT_THAT(v, IsIntEq(23));