From cae41eebffce5802918ae3ceb7e7c669ea94243c Mon Sep 17 00:00:00 2001 From: Andreas Rammhold Date: Wed, 6 Oct 2021 17:08:08 +0200 Subject: [PATCH] libexpr: remove matchAttrs boolean from ExprLambda The boolean is only used to determine if the formals are set to a non-null pointer in all our cases. We can get rid of that allocation and instead just compare the pointer value with NULL. Saving up to sizeof(bool) + platform specific alignment per ExprLambda instace. Probably not a lot of memory but perhaps a few kilobyte with nixpkgs? This also gets rid of a potential issue with dereferencing formals based on the value of the boolean that didn't have to be aligned with the formals pointer but was in all our cases. --- src/libexpr/eval.cc | 6 +++--- src/libexpr/flake/flake.cc | 2 +- src/libexpr/nixexpr.cc | 4 ++-- src/libexpr/nixexpr.hh | 6 +++--- src/libexpr/parser.y | 8 ++++---- src/libexpr/primops.cc | 2 +- src/libexpr/value-to-xml.cc | 2 +- src/nix/flake.cc | 6 +++--- 8 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 800839a8d..2a79090cd 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -1316,13 +1316,13 @@ void EvalState::callFunction(Value & fun, Value & arg, Value & v, const Pos & po auto size = (lambda.arg.empty() ? 0 : 1) + - (lambda.matchAttrs ? lambda.formals->formals.size() : 0); + (lambda.hasFormals() ? lambda.formals->formals.size() : 0); Env & env2(allocEnv(size)); env2.up = fun.lambda.env; size_t displ = 0; - if (!lambda.matchAttrs) + if (!lambda.hasFormals()) env2.values[displ++] = &arg; else { @@ -1402,7 +1402,7 @@ void EvalState::autoCallFunction(Bindings & args, Value & fun, Value & res) } } - if (!fun.isLambda() || !fun.lambda.fun->matchAttrs) { + if (!fun.isLambda() || !fun.lambda.fun->hasFormals()) { res = fun; return; } diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc index 1a1fa6938..6a2902dff 100644 --- a/src/libexpr/flake/flake.cc +++ b/src/libexpr/flake/flake.cc @@ -231,7 +231,7 @@ static Flake getFlake( if (auto outputs = vInfo.attrs->get(sOutputs)) { expectType(state, nFunction, *outputs->value, *outputs->pos); - if (outputs->value->isLambda() && outputs->value->lambda.fun->matchAttrs) { + if (outputs->value->isLambda() && outputs->value->lambda.fun->hasFormals()) { for (auto & formal : outputs->value->lambda.fun->formals->formals) { if (formal.name != state.sSelf) flake.inputs.emplace(formal.name, FlakeInput { diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index 492b819e7..0d0f3e469 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -124,7 +124,7 @@ void ExprList::show(std::ostream & str) const void ExprLambda::show(std::ostream & str) const { str << "("; - if (matchAttrs) { + if (hasFormals()) { str << "{ "; bool first = true; for (auto & i : formals->formals) { @@ -348,7 +348,7 @@ void ExprLambda::bindVars(const StaticEnv & env) if (!arg.empty()) newEnv.vars[arg] = displ++; - if (matchAttrs) { + if (hasFormals()) { for (auto & i : formals->formals) newEnv.vars[i.name] = displ++; diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index 51a14cd59..851e875bd 100644 --- a/src/libexpr/nixexpr.hh +++ b/src/libexpr/nixexpr.hh @@ -233,11 +233,10 @@ struct ExprLambda : Expr Pos pos; Symbol name; Symbol arg; - bool matchAttrs; Formals * formals; Expr * body; - ExprLambda(const Pos & pos, const Symbol & arg, bool matchAttrs, Formals * formals, Expr * body) - : pos(pos), arg(arg), matchAttrs(matchAttrs), formals(formals), body(body) + ExprLambda(const Pos & pos, const Symbol & arg, Formals * formals, Expr * body) + : pos(pos), arg(arg), formals(formals), body(body) { if (!arg.empty() && formals && formals->argNames.find(arg) != formals->argNames.end()) throw ParseError({ @@ -247,6 +246,7 @@ struct ExprLambda : Expr }; void setName(Symbol & name); string showNamePos() const; + inline bool hasFormals() const { return formals != nullptr; } COMMON_METHODS }; diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index e3749783a..8a0a79c96 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -324,13 +324,13 @@ expr: expr_function; expr_function : ID ':' expr_function - { $$ = new ExprLambda(CUR_POS, data->symbols.create($1), false, 0, $3); } + { $$ = new ExprLambda(CUR_POS, data->symbols.create($1), 0, $3); } | '{' formals '}' ':' expr_function - { $$ = new ExprLambda(CUR_POS, data->symbols.create(""), true, $2, $5); } + { $$ = new ExprLambda(CUR_POS, data->symbols.create(""), $2, $5); } | '{' formals '}' '@' ID ':' expr_function - { $$ = new ExprLambda(CUR_POS, data->symbols.create($5), true, $2, $7); } + { $$ = new ExprLambda(CUR_POS, data->symbols.create($5), $2, $7); } | ID '@' '{' formals '}' ':' expr_function - { $$ = new ExprLambda(CUR_POS, data->symbols.create($1), true, $4, $7); } + { $$ = new ExprLambda(CUR_POS, data->symbols.create($1), $4, $7); } | ASSERT expr ';' expr_function { $$ = new ExprAssert(CUR_POS, $2, $4); } | WITH expr ';' expr_function diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 3bf091438..8d4b817c9 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -2365,7 +2365,7 @@ static void prim_functionArgs(EvalState & state, const Pos & pos, Value * * args .errPos = pos }); - if (!args[0]->lambda.fun->matchAttrs) { + if (!args[0]->lambda.fun->hasFormals()) { state.mkAttrs(v, 0); return; } diff --git a/src/libexpr/value-to-xml.cc b/src/libexpr/value-to-xml.cc index 2ddc5f751..b44455f5f 100644 --- a/src/libexpr/value-to-xml.cc +++ b/src/libexpr/value-to-xml.cc @@ -135,7 +135,7 @@ static void printValueAsXML(EvalState & state, bool strict, bool location, if (location) posToXML(xmlAttrs, v.lambda.fun->pos); XMLOpenElement _(doc, "function", xmlAttrs); - if (v.lambda.fun->matchAttrs) { + if (v.lambda.fun->hasFormals()) { XMLAttrs attrs; if (!v.lambda.fun->arg.empty()) attrs["name"] = v.lambda.fun->arg; if (v.lambda.fun->formals->ellipsis) attrs["ellipsis"] = "1"; diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 7d7ada707..7e4d23f6e 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -346,10 +346,10 @@ struct CmdFlakeCheck : FlakeCommand auto checkOverlay = [&](const std::string & attrPath, Value & v, const Pos & pos) { try { state->forceValue(v, pos); - if (!v.isLambda() || v.lambda.fun->matchAttrs || std::string(v.lambda.fun->arg) != "final") + if (!v.isLambda() || v.lambda.fun->hasFormals() || std::string(v.lambda.fun->arg) != "final") throw Error("overlay does not take an argument named 'final'"); auto body = dynamic_cast(v.lambda.fun->body); - if (!body || body->matchAttrs || std::string(body->arg) != "prev") + if (!body || body->hasFormals() || std::string(body->arg) != "prev") throw Error("overlay does not take an argument named 'prev'"); // FIXME: if we have a 'nixpkgs' input, use it to // evaluate the overlay. @@ -363,7 +363,7 @@ struct CmdFlakeCheck : FlakeCommand try { state->forceValue(v, pos); if (v.isLambda()) { - if (!v.lambda.fun->matchAttrs || !v.lambda.fun->formals->ellipsis) + if (!v.lambda.fun->hasFormals() || !v.lambda.fun->formals->ellipsis) throw Error("module must match an open attribute set ('{ config, ... }')"); } else if (v.type() == nAttrs) { for (auto & attr : *v.attrs)