diff --git a/src/libexpr-tests/primops.cc b/src/libexpr-tests/primops.cc index 74d676844b7..2c2adf5d62d 100644 --- a/src/libexpr-tests/primops.cc +++ b/src/libexpr-tests/primops.cc @@ -771,7 +771,7 @@ TEST_F(PrimOpTest, derivation) ASSERT_EQ(v.type(), nFunction); ASSERT_TRUE(v.isLambda()); ASSERT_NE(v.lambda().fun, nullptr); - ASSERT_TRUE(v.lambda().fun->hasFormals()); + ASSERT_TRUE(v.lambda().fun->hasFormals); } TEST_F(PrimOpTest, currentTime) diff --git a/src/libexpr-tests/value/print.cc b/src/libexpr-tests/value/print.cc index 6cadbc70ac1..20a6f86147c 100644 --- a/src/libexpr-tests/value/print.cc +++ b/src/libexpr-tests/value/print.cc @@ -112,7 +112,7 @@ TEST_F(ValuePrintingTests, vLambda) auto body = ExprInt(0); auto formals = Formals{}; - ExprLambda eLambda(posIdx, createSymbol("a"), &formals, &body); + ExprLambda eLambda(state.mem.exprs.alloc, posIdx, createSymbol("a"), formals, &body); Value vLambda; vLambda.mkLambda(&env, &eLambda); @@ -502,7 +502,7 @@ TEST_F(ValuePrintingTests, ansiColorsLambda) auto body = ExprInt(0); auto formals = Formals{}; - ExprLambda eLambda(posIdx, createSymbol("a"), &formals, &body); + ExprLambda eLambda(state.mem.exprs.alloc, posIdx, createSymbol("a"), formals, &body); Value vLambda; vLambda.mkLambda(&env, &eLambda); diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index a6973f59050..57db58542f2 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -1496,13 +1496,13 @@ void EvalState::callFunction(Value & fun, std::span args, Value & vRes, ExprLambda & lambda(*vCur.lambda().fun); - auto size = (!lambda.arg ? 0 : 1) + (lambda.hasFormals() ? lambda.formals->formals.size() : 0); + auto size = (!lambda.arg ? 0 : 1) + (lambda.hasFormals ? lambda.getFormals().size() : 0); Env & env2(mem.allocEnv(size)); env2.up = vCur.lambda().env; Displacement displ = 0; - if (!lambda.hasFormals()) + if (!lambda.hasFormals) env2.values[displ++] = args[0]; else { try { @@ -1520,7 +1520,7 @@ void EvalState::callFunction(Value & fun, std::span args, Value & vRes, there is no matching actual argument but the formal argument has a default, use the default. */ size_t attrsUsed = 0; - for (auto & i : lambda.formals->formals) { + for (auto & i : lambda.getFormals()) { auto j = args[0]->attrs()->get(i.name); if (!j) { if (!i.def) { @@ -1542,13 +1542,13 @@ void EvalState::callFunction(Value & fun, std::span args, Value & vRes, /* Check that each actual argument is listed as a formal argument (unless the attribute match specifies a `...'). */ - if (!lambda.formals->ellipsis && attrsUsed != args[0]->attrs()->size()) { + if (!lambda.ellipsis && attrsUsed != args[0]->attrs()->size()) { /* Nope, so show the first unexpected argument to the user. */ for (auto & i : *args[0]->attrs()) - if (!lambda.formals->has(i.name)) { + if (!lambda.hasFormal(i.name)) { StringSet formalNames; - for (auto & formal : lambda.formals->formals) + for (auto & formal : lambda.getFormals()) formalNames.insert(std::string(symbols[formal.name])); auto suggestions = Suggestions::bestMatches(formalNames, symbols[i.name]); error( @@ -1747,14 +1747,14 @@ void EvalState::autoCallFunction(const Bindings & args, Value & fun, Value & res } } - if (!fun.isLambda() || !fun.lambda().fun->hasFormals()) { + if (!fun.isLambda() || !fun.lambda().fun->hasFormals) { res = fun; return; } - auto attrs = buildBindings(std::max(static_cast(fun.lambda().fun->formals->formals.size()), args.size())); + auto attrs = buildBindings(std::max(static_cast(fun.lambda().fun->nFormals), args.size())); - if (fun.lambda().fun->formals->ellipsis) { + if (fun.lambda().fun->ellipsis) { // If the formals have an ellipsis (eg the function accepts extra args) pass // all available automatic arguments (which includes arguments specified on // the command line via --arg/--argstr) @@ -1762,7 +1762,7 @@ void EvalState::autoCallFunction(const Bindings & args, Value & fun, Value & res attrs.insert(v); } else { // Otherwise, only pass the arguments that the function accepts - for (auto & i : fun.lambda().fun->formals->formals) { + for (auto & i : fun.lambda().fun->getFormals()) { auto j = args.get(i.name); if (j) { attrs.insert(*j); diff --git a/src/libexpr/include/nix/expr/nixexpr.hh b/src/libexpr/include/nix/expr/nixexpr.hh index 86ad0150419..79a36e845b1 100644 --- a/src/libexpr/include/nix/expr/nixexpr.hh +++ b/src/libexpr/include/nix/expr/nixexpr.hh @@ -475,16 +475,6 @@ struct Formals formals.begin(), formals.end(), arg, [](const Formal & f, const Symbol & sym) { return f.name < sym; }); return it != formals.end() && it->name == arg; } - - std::vector lexicographicOrder(const SymbolTable & symbols) const - { - std::vector result(formals.begin(), formals.end()); - std::sort(result.begin(), result.end(), [&](const Formal & a, const Formal & b) { - std::string_view sa = symbols[a.name], sb = symbols[b.name]; - return sa < sb; - }); - return result; - } }; struct ExprLambda : Expr @@ -492,29 +482,57 @@ struct ExprLambda : Expr PosIdx pos; Symbol name; Symbol arg; - Formals * formals; + + bool ellipsis; + bool hasFormals; + uint16_t nFormals; + Formal * formalsStart; + Expr * body; DocComment docComment; - ExprLambda(PosIdx pos, Symbol arg, Formals * formals, Expr * body) + ExprLambda( + std::pmr::polymorphic_allocator & alloc, PosIdx pos, Symbol arg, const Formals & formals, Expr * body) : pos(pos) , arg(arg) - , formals(formals) - , body(body) {}; + , ellipsis(formals.ellipsis) + , hasFormals(true) + , nFormals(formals.formals.size()) + , formalsStart(alloc.allocate_object(nFormals)) + , body(body) + { + std::ranges::copy(formals.formals, formalsStart); + }; - ExprLambda(PosIdx pos, Formals * formals, Expr * body) + ExprLambda(std::pmr::polymorphic_allocator & alloc, PosIdx pos, Symbol arg, Expr * body) : pos(pos) - , formals(formals) - , body(body) + , arg(arg) + , hasFormals(false) + , formalsStart(nullptr) + , body(body) {}; + + ExprLambda(std::pmr::polymorphic_allocator & alloc, PosIdx pos, Formals formals, Expr * body) + : ExprLambda(alloc, pos, Symbol(), formals, body) {}; + + bool hasFormal(Symbol arg) const { + auto formals = getFormals(); + auto it = std::lower_bound( + formals.begin(), formals.end(), arg, [](const Formal & f, const Symbol & sym) { return f.name < sym; }); + return it != formals.end() && it->name == arg; } void setName(Symbol name) override; std::string showNamePos(const EvalState & state) const; - inline bool hasFormals() const + std::vector getFormalsLexicographic(const SymbolTable & symbols) const { - return formals != nullptr; + std::vector result(getFormals().begin(), getFormals().end()); + std::sort(result.begin(), result.end(), [&](const Formal & a, const Formal & b) { + std::string_view sa = symbols[a.name], sb = symbols[b.name]; + return sa < sb; + }); + return result; } PosIdx getPos() const override @@ -522,6 +540,12 @@ struct ExprLambda : Expr return pos; } + std::span getFormals() const + { + assert(hasFormals); + return {formalsStart, nFormals}; + } + virtual void setDocComment(DocComment docComment) override; COMMON_METHODS }; diff --git a/src/libexpr/include/nix/expr/parser-state.hh b/src/libexpr/include/nix/expr/parser-state.hh index 18d2051d002..4d131244387 100644 --- a/src/libexpr/include/nix/expr/parser-state.hh +++ b/src/libexpr/include/nix/expr/parser-state.hh @@ -93,7 +93,7 @@ struct ParserState void addAttr( ExprAttrs * attrs, AttrPath && attrPath, const ParserLocation & loc, Expr * e, const ParserLocation & exprLoc); void addAttr(ExprAttrs * attrs, AttrPath & attrPath, const Symbol & symbol, ExprAttrs::AttrDef && def); - Formals * validateFormals(Formals * formals, PosIdx pos = noPos, Symbol arg = {}); + void validateFormals(Formals & formals, PosIdx pos = noPos, Symbol arg = {}); Expr * stripIndentation(const PosIdx pos, std::vector>> && es); PosIdx at(const ParserLocation & loc); }; @@ -213,17 +213,17 @@ ParserState::addAttr(ExprAttrs * attrs, AttrPath & attrPath, const Symbol & symb } } -inline Formals * ParserState::validateFormals(Formals * formals, PosIdx pos, Symbol arg) +inline void ParserState::validateFormals(Formals & formals, PosIdx pos, Symbol arg) { - std::sort(formals->formals.begin(), formals->formals.end(), [](const auto & a, const auto & b) { + std::sort(formals.formals.begin(), formals.formals.end(), [](const auto & a, const auto & b) { return std::tie(a.name, a.pos) < std::tie(b.name, b.pos); }); std::optional> duplicate; - for (size_t i = 0; i + 1 < formals->formals.size(); i++) { - if (formals->formals[i].name != formals->formals[i + 1].name) + for (size_t i = 0; i + 1 < formals.formals.size(); i++) { + if (formals.formals[i].name != formals.formals[i + 1].name) continue; - std::pair thisDup{formals->formals[i].name, formals->formals[i + 1].pos}; + std::pair thisDup{formals.formals[i].name, formals.formals[i + 1].pos}; duplicate = std::min(thisDup, duplicate.value_or(thisDup)); } if (duplicate) @@ -231,11 +231,9 @@ inline Formals * ParserState::validateFormals(Formals * formals, PosIdx pos, Sym {.msg = HintFmt("duplicate formal function argument '%1%'", symbols[duplicate->first]), .pos = positions[duplicate->second]}); - if (arg && formals->has(arg)) + if (arg && formals.has(arg)) throw ParseError( {.msg = HintFmt("duplicate formal function argument '%1%'", symbols[arg]), .pos = positions[pos]}); - - return formals; } inline Expr * diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index a77e42356ff..ce1ea19e13c 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -154,13 +154,13 @@ void ExprList::show(const SymbolTable & symbols, std::ostream & str) const void ExprLambda::show(const SymbolTable & symbols, std::ostream & str) const { str << "("; - if (hasFormals()) { + if (hasFormals) { str << "{ "; bool first = true; // the natural Symbol ordering is by creation time, which can lead to the // same expression being printed in two different ways depending on its // context. always use lexicographic ordering to avoid this. - for (auto & i : formals->lexicographicOrder(symbols)) { + for (auto & i : getFormalsLexicographic(symbols)) { if (first) first = false; else @@ -171,7 +171,7 @@ void ExprLambda::show(const SymbolTable & symbols, std::ostream & str) const i.def->show(symbols, str); } } - if (formals->ellipsis) { + if (ellipsis) { if (!first) str << ", "; str << "..."; @@ -451,21 +451,20 @@ void ExprLambda::bindVars(EvalState & es, const std::shared_ptr if (es.debugRepl) es.exprEnvs.insert(std::make_pair(this, env)); - auto newEnv = - std::make_shared(nullptr, env, (hasFormals() ? formals->formals.size() : 0) + (!arg ? 0 : 1)); + auto newEnv = std::make_shared(nullptr, env, (hasFormals ? getFormals().size() : 0) + (!arg ? 0 : 1)); Displacement displ = 0; if (arg) newEnv->vars.emplace_back(arg, displ++); - if (hasFormals()) { - for (auto & i : formals->formals) + if (hasFormals) { + for (auto & i : getFormals()) newEnv->vars.emplace_back(i.name, displ++); newEnv->sort(); - for (auto & i : formals->formals) + for (auto & i : getFormals()) if (i.def) i.def->bindVars(es, newEnv); } diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index cf563e86923..2732fe4c8f9 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -131,7 +131,7 @@ static Expr * makeCall(PosIdx pos, Expr * fn, Expr * arg) { %type expr_pipe_from expr_pipe_into %type expr_list %type binds binds1 -%type formals formal_set +%type formals formal_set %type formal %type > attrpath %type >> attrs @@ -179,26 +179,30 @@ expr: expr_function; expr_function : ID ':' expr_function - { auto me = new ExprLambda(CUR_POS, state->symbols.create($1), 0, $3); + { auto me = new ExprLambda(state->alloc, CUR_POS, state->symbols.create($1), $3); $$ = me; SET_DOC_POS(me, @1); } | formal_set ':' expr_function[body] - { auto me = new ExprLambda(CUR_POS, state->validateFormals($formal_set), $body); + { + state->validateFormals($formal_set); + auto me = new ExprLambda(state->alloc, CUR_POS, std::move($formal_set), $body); $$ = me; SET_DOC_POS(me, @1); } | formal_set '@' ID ':' expr_function[body] { auto arg = state->symbols.create($ID); - auto me = new ExprLambda(CUR_POS, arg, state->validateFormals($formal_set, CUR_POS, arg), $body); + state->validateFormals($formal_set, CUR_POS, arg); + auto me = new ExprLambda(state->alloc, CUR_POS, arg, std::move($formal_set), $body); $$ = me; SET_DOC_POS(me, @1); } | ID '@' formal_set ':' expr_function[body] { auto arg = state->symbols.create($ID); - auto me = new ExprLambda(CUR_POS, arg, state->validateFormals($formal_set, CUR_POS, arg), $body); + state->validateFormals($formal_set, CUR_POS, arg); + auto me = new ExprLambda(state->alloc, CUR_POS, arg, std::move($formal_set), $body); $$ = me; SET_DOC_POS(me, @1); } @@ -490,18 +494,18 @@ expr_list ; formal_set - : '{' formals ',' ELLIPSIS '}' { $$ = $formals; $$->ellipsis = true; } - | '{' ELLIPSIS '}' { $$ = new Formals; $$->ellipsis = true; } - | '{' formals ',' '}' { $$ = $formals; $$->ellipsis = false; } - | '{' formals '}' { $$ = $formals; $$->ellipsis = false; } - | '{' '}' { $$ = new Formals; $$->ellipsis = false; } + : '{' formals ',' ELLIPSIS '}' { $$ = std::move($formals); $$.ellipsis = true; } + | '{' ELLIPSIS '}' { $$.ellipsis = true; } + | '{' formals ',' '}' { $$ = std::move($formals); $$.ellipsis = false; } + | '{' formals '}' { $$ = std::move($formals); $$.ellipsis = false; } + | '{' '}' { $$.ellipsis = false; } ; formals : formals[accum] ',' formal - { $$ = $accum; $$->formals.emplace_back(std::move($formal)); } + { $$ = std::move($accum); $$.formals.emplace_back(std::move($formal)); } | formal - { $$ = new Formals; $$->formals.emplace_back(std::move($formal)); } + { $$.formals.emplace_back(std::move($formal)); } ; formal diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index c8e49cd2929..d95c999ff6e 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -3363,12 +3363,12 @@ static void prim_functionArgs(EvalState & state, const PosIdx pos, Value ** args if (!args[0]->isLambda()) state.error("'functionArgs' requires a function").atPos(pos).debugThrow(); - if (!args[0]->lambda().fun->hasFormals()) { + if (!args[0]->lambda().fun->hasFormals) { v.mkAttrs(&Bindings::emptyBindings); return; } - const auto & formals = args[0]->lambda().fun->formals->formals; + const auto & formals = args[0]->lambda().fun->getFormals(); auto attrs = state.buildBindings(formals.size()); for (auto & i : formals) attrs.insert(i.name, state.getBool(i.def), i.pos); diff --git a/src/libexpr/value-to-xml.cc b/src/libexpr/value-to-xml.cc index 31400e439e5..547597ca46d 100644 --- a/src/libexpr/value-to-xml.cc +++ b/src/libexpr/value-to-xml.cc @@ -145,14 +145,14 @@ static void printValueAsXML( posToXML(state, xmlAttrs, state.positions[v.lambda().fun->pos]); XMLOpenElement _(doc, "function", xmlAttrs); - if (v.lambda().fun->hasFormals()) { + if (v.lambda().fun->hasFormals) { XMLAttrs attrs; if (v.lambda().fun->arg) attrs["name"] = state.symbols[v.lambda().fun->arg]; - if (v.lambda().fun->formals->ellipsis) + if (v.lambda().fun->ellipsis) attrs["ellipsis"] = "1"; XMLOpenElement _(doc, "attrspat", attrs); - for (auto & i : v.lambda().fun->formals->lexicographicOrder(state.symbols)) + for (auto & i : v.lambda().fun->getFormalsLexicographic(state.symbols)) doc.writeEmptyElement("attr", singletonAttrs("name", state.symbols[i.name])); } else doc.writeEmptyElement("varpat", singletonAttrs("name", state.symbols[v.lambda().fun->arg])); diff --git a/src/libflake/flake.cc b/src/libflake/flake.cc index 8e7e2be26b1..29158a7a25d 100644 --- a/src/libflake/flake.cc +++ b/src/libflake/flake.cc @@ -281,8 +281,8 @@ static Flake readFlake( if (auto outputs = vInfo.attrs()->get(sOutputs)) { expectType(state, nFunction, *outputs->value, outputs->pos); - if (outputs->value->isLambda() && outputs->value->lambda().fun->hasFormals()) { - for (auto & formal : outputs->value->lambda().fun->formals->formals) { + if (outputs->value->isLambda() && outputs->value->lambda().fun->hasFormals) { + for (auto & formal : outputs->value->lambda().fun->getFormals()) { if (formal.name != state.s.self) flake.inputs.emplace( state.symbols[formal.name], diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 04d4ec8eb1a..60105243498 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -473,7 +473,7 @@ struct CmdFlakeCheck : FlakeCommand if (!v.isLambda()) { throw Error("overlay is not a function, but %s instead", showType(v)); } - if (v.lambda().fun->hasFormals() || !argHasName(v.lambda().fun->arg, "final")) + if (v.lambda().fun->hasFormals || !argHasName(v.lambda().fun->arg, "final")) throw Error("overlay does not take an argument named 'final'"); // FIXME: if we have a 'nixpkgs' input, use it to // evaluate the overlay. diff --git a/src/nix/nix-build/nix-build.cc b/src/nix/nix-build/nix-build.cc index 8aced503bec..f76a4f2415e 100644 --- a/src/nix/nix-build/nix-build.cc +++ b/src/nix/nix-build/nix-build.cc @@ -416,8 +416,8 @@ static void main_nix_build(int argc, char ** argv) return false; } bool add = false; - if (v.type() == nFunction && v.lambda().fun->hasFormals()) { - for (auto & i : v.lambda().fun->formals->formals) { + if (v.type() == nFunction && v.lambda().fun->hasFormals) { + for (auto & i : v.lambda().fun->getFormals()) { if (state->symbols[i.name] == "inNixShell") { add = true; break; diff --git a/tests/functional/lang/eval-fail-empty-formals.err.exp b/tests/functional/lang/eval-fail-empty-formals.err.exp new file mode 100644 index 00000000000..5cd4829f7d0 --- /dev/null +++ b/tests/functional/lang/eval-fail-empty-formals.err.exp @@ -0,0 +1,12 @@ +error: + … from call site + at /pwd/lang/eval-fail-empty-formals.nix:1:1: + 1| (foo@{ }: 1) { a = 3; } + | ^ + 2| + + error: function 'anonymous lambda' called with unexpected argument 'a' + at /pwd/lang/eval-fail-empty-formals.nix:1:2: + 1| (foo@{ }: 1) { a = 3; } + | ^ + 2| diff --git a/tests/functional/lang/eval-fail-empty-formals.nix b/tests/functional/lang/eval-fail-empty-formals.nix new file mode 100644 index 00000000000..597f4049632 --- /dev/null +++ b/tests/functional/lang/eval-fail-empty-formals.nix @@ -0,0 +1 @@ +(foo@{ }: 1) { a = 3; }