diff --git a/src/libexpr-tests/primops.cc b/src/libexpr-tests/primops.cc index 74d676844..f00b4f475 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->getFormals()); } TEST_F(PrimOpTest, currentTime) diff --git a/src/libexpr-tests/value/print.cc b/src/libexpr-tests/value/print.cc index 6cadbc70a..0006da2ff 100644 --- a/src/libexpr-tests/value/print.cc +++ b/src/libexpr-tests/value/print.cc @@ -110,9 +110,8 @@ TEST_F(ValuePrintingTests, vLambda) PosTable::Origin origin = state.positions.addOrigin(std::monostate(), 1); auto posIdx = state.positions.add(origin, 0); auto body = ExprInt(0); - auto formals = Formals{}; - ExprLambda eLambda(posIdx, createSymbol("a"), &formals, &body); + ExprLambda eLambda(posIdx, createSymbol("a"), &body); Value vLambda; vLambda.mkLambda(&env, &eLambda); @@ -500,9 +499,8 @@ TEST_F(ValuePrintingTests, ansiColorsLambda) PosTable::Origin origin = state.positions.addOrigin(std::monostate(), 1); auto posIdx = state.positions.add(origin, 0); auto body = ExprInt(0); - auto formals = Formals{}; - ExprLambda eLambda(posIdx, createSymbol("a"), &formals, &body); + ExprLambda eLambda(posIdx, createSymbol("a"), &body); Value vLambda; vLambda.mkLambda(&env, &eLambda); diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 8a03084ee..35a01101b 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -1496,15 +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.getFormals() ? lambda.getFormals()->formals.size() : 0); Env & env2(mem.allocEnv(size)); env2.up = vCur.lambda().env; Displacement displ = 0; - if (!lambda.hasFormals()) - env2.values[displ++] = args[0]; - else { + if (auto formals = lambda.getFormals()) { try { forceAttrs(*args[0], lambda.pos, "while evaluating the value passed for the lambda argument"); } catch (Error & e) { @@ -1520,7 +1518,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 : formals->formals) { auto j = args[0]->attrs()->get(i.name); if (!j) { if (!i.def) { @@ -1542,13 +1540,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 (!formals->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 (!formals->has(i.name)) { StringSet formalNames; - for (auto & formal : lambda.formals->formals) + for (auto & formal : formals->formals) formalNames.insert(std::string(symbols[formal.name])); auto suggestions = Suggestions::bestMatches(formalNames, symbols[i.name]); error( @@ -1563,6 +1561,8 @@ void EvalState::callFunction(Value & fun, std::span args, Value & vRes, } unreachable(); } + } else { + env2.values[displ++] = args[0]; } nrFunctionCalls++; @@ -1747,14 +1747,15 @@ void EvalState::autoCallFunction(const Bindings & args, Value & fun, Value & res } } - if (!fun.isLambda() || !fun.lambda().fun->hasFormals()) { + if (!fun.isLambda() || !fun.lambda().fun->getFormals()) { res = fun; return; } + auto formals = fun.lambda().fun->getFormals(); - auto attrs = buildBindings(std::max(static_cast(fun.lambda().fun->formals->formals.size()), args.size())); + auto attrs = buildBindings(std::max(static_cast(formals->formals.size()), args.size())); - if (fun.lambda().fun->formals->ellipsis) { + if (formals->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 +1763,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 : formals->formals) { 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 26d5addd5..4962c7857 100644 --- a/src/libexpr/include/nix/expr/nixexpr.hh +++ b/src/libexpr/include/nix/expr/nixexpr.hh @@ -466,7 +466,7 @@ struct Formal Expr * def; }; -struct Formals +struct FormalsBuilder { typedef std::vector Formals_; /** @@ -481,6 +481,23 @@ struct Formals formals.begin(), formals.end(), arg, [](const Formal & f, const Symbol & sym) { return f.name < sym; }); return it != formals.end() && it->name == arg; } +}; + +struct Formals +{ + std::span formals; + bool ellipsis; + + Formals(std::span formals, bool ellipsis) + : formals(formals) + , ellipsis(ellipsis) {}; + + bool has(Symbol arg) const + { + 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; + } std::vector lexicographicOrder(const SymbolTable & symbols) const { @@ -498,31 +515,57 @@ struct ExprLambda : Expr PosIdx pos; Symbol name; Symbol arg; - Formals * formals; + +private: + bool hasFormals; + bool ellipsis; + uint16_t nFormals; + Formal * formalsStart; +public: + + std::optional getFormals() const + { + if (hasFormals) + return Formals{{formalsStart, nFormals}, ellipsis}; + else + return std::nullopt; + } + Expr * body; DocComment docComment; - ExprLambda(PosIdx pos, Symbol arg, Formals * formals, Expr * body) + ExprLambda( + std::pmr::polymorphic_allocator & alloc, + PosIdx pos, + Symbol arg, + const FormalsBuilder & formals, + Expr * body) : pos(pos) , arg(arg) - , formals(formals) - , body(body) {}; - - ExprLambda(PosIdx pos, Formals * formals, Expr * body) - : pos(pos) - , formals(formals) + , hasFormals(true) + , ellipsis(formals.ellipsis) + , nFormals(formals.formals.size()) + , formalsStart(alloc.allocate_object(nFormals)) , body(body) { - } + std::ranges::copy(formals.formals, formalsStart); + }; + + ExprLambda(PosIdx pos, Symbol arg, Expr * body) + : pos(pos) + , arg(arg) + , hasFormals(false) + , ellipsis(false) + , nFormals(0) + , formalsStart(nullptr) + , body(body) {}; + + ExprLambda(std::pmr::polymorphic_allocator & alloc, PosIdx pos, FormalsBuilder formals, Expr * body) + : ExprLambda(alloc, pos, Symbol(), formals, body) {}; void setName(Symbol name) override; std::string showNamePos(const EvalState & state) const; - inline bool hasFormals() const - { - return formals != nullptr; - } - PosIdx getPos() const override { return pos; diff --git a/src/libexpr/include/nix/expr/parser-state.hh b/src/libexpr/include/nix/expr/parser-state.hh index 18d2051d0..4cffaa497 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(FormalsBuilder & 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(FormalsBuilder & 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 a77e42356..b183f1bbf 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -154,7 +154,7 @@ void ExprList::show(const SymbolTable & symbols, std::ostream & str) const void ExprLambda::show(const SymbolTable & symbols, std::ostream & str) const { str << "("; - if (hasFormals()) { + if (auto formals = getFormals()) { str << "{ "; bool first = true; // the natural Symbol ordering is by creation time, which can lead to the @@ -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 << "..."; @@ -452,14 +452,14 @@ void ExprLambda::bindVars(EvalState & es, const std::shared_ptr es.exprEnvs.insert(std::make_pair(this, env)); auto newEnv = - std::make_shared(nullptr, env, (hasFormals() ? formals->formals.size() : 0) + (!arg ? 0 : 1)); + std::make_shared(nullptr, env, (getFormals() ? getFormals()->formals.size() : 0) + (!arg ? 0 : 1)); Displacement displ = 0; if (arg) newEnv->vars.emplace_back(arg, displ++); - if (hasFormals()) { + if (auto formals = getFormals()) { for (auto & i : formals->formals) newEnv->vars.emplace_back(i.name, displ++); diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 51c82efe5..02fc4c3b6 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 > 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(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 @@ 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 c8e49cd29..04196bc1f 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -3363,21 +3363,20 @@ 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 (const auto & formals = args[0]->lambda().fun->getFormals()) { + auto attrs = state.buildBindings(formals->formals.size()); + for (auto & i : formals->formals) + attrs.insert(i.name, state.getBool(i.def), i.pos); + /* Optimization: avoid sorting bindings. `formals` must already be sorted according to + (std::tie(a.name, a.pos) < std::tie(b.name, b.pos)) predicate, so the following assertion + always holds: + assert(std::is_sorted(attrs.alreadySorted()->begin(), attrs.alreadySorted()->end())); + .*/ + v.mkAttrs(attrs.alreadySorted()); + } else { v.mkAttrs(&Bindings::emptyBindings); return; } - - const auto & formals = args[0]->lambda().fun->formals->formals; - auto attrs = state.buildBindings(formals.size()); - for (auto & i : formals) - attrs.insert(i.name, state.getBool(i.def), i.pos); - /* Optimization: avoid sorting bindings. `formals` must already be sorted according to - (std::tie(a.name, a.pos) < std::tie(b.name, b.pos)) predicate, so the following assertion - always holds: - assert(std::is_sorted(attrs.alreadySorted()->begin(), attrs.alreadySorted()->end())); - .*/ - v.mkAttrs(attrs.alreadySorted()); } static RegisterPrimOp primop_functionArgs({ diff --git a/src/libexpr/value-to-xml.cc b/src/libexpr/value-to-xml.cc index 31400e439..d5959e894 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 (auto formals = v.lambda().fun->getFormals()) { XMLAttrs attrs; if (v.lambda().fun->arg) attrs["name"] = state.symbols[v.lambda().fun->arg]; - if (v.lambda().fun->formals->ellipsis) + if (formals->ellipsis) attrs["ellipsis"] = "1"; XMLOpenElement _(doc, "attrspat", attrs); - for (auto & i : v.lambda().fun->formals->lexicographicOrder(state.symbols)) + for (auto & i : formals->lexicographicOrder(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 2aec0fac4..42385712c 100644 --- a/src/libflake/flake.cc +++ b/src/libflake/flake.cc @@ -281,12 +281,15 @@ 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 (formal.name != state.s.self) - flake.inputs.emplace( - state.symbols[formal.name], - FlakeInput{.ref = parseFlakeRef(state.fetchSettings, std::string(state.symbols[formal.name]))}); + if (outputs->value->isLambda()) { + if (auto formals = outputs->value->lambda().fun->getFormals()) { + for (auto & formal : formals->formals) { + if (formal.name != state.s.self) + flake.inputs.emplace( + state.symbols[formal.name], + FlakeInput{ + .ref = parseFlakeRef(state.fetchSettings, std::string(state.symbols[formal.name]))}); + } } } diff --git a/src/nix/flake.cc b/src/nix/flake.cc index a7e7d0039..b826e943c 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -468,7 +468,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->getFormals() || !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 ab65fba7e..4d876c9eb 100644 --- a/src/nix/nix-build/nix-build.cc +++ b/src/nix/nix-build/nix-build.cc @@ -415,11 +415,13 @@ 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 (state->symbols[i.name] == "inNixShell") { - add = true; - break; + if (v.type() == nFunction) { + if (auto formals = v.lambda().fun->getFormals()) { + for (auto & i : formals->formals) { + 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 000000000..5cd4829f7 --- /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 000000000..597f40496 --- /dev/null +++ b/tests/functional/lang/eval-fail-empty-formals.nix @@ -0,0 +1 @@ +(foo@{ }: 1) { a = 3; }