diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 94ffab175..73ed3a8a1 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -1368,7 +1368,7 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value & /* Nope, so show the first unexpected argument to the user. */ for (auto & i : *args[0]->attrs) - if (!lambda.formals->argNames.count(i.name)) + if (!lambda.formals->has(i.name)) throwTypeError(pos, "%1% called with unexpected argument '%2%'", lambda, i.name); abort(); // can't happen } diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index a2070ff69..4e923ac89 100644 --- a/src/libexpr/nixexpr.hh +++ b/src/libexpr/nixexpr.hh @@ -216,10 +216,25 @@ struct Formal struct Formals { - typedef std::list Formals_; + typedef std::vector Formals_; Formals_ formals; - std::set argNames; // used during parsing bool 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 + { + std::vector result(formals.begin(), formals.end()); + std::sort(result.begin(), result.end(), + [] (const Formal & a, const Formal & b) { + return std::string_view(a.name) < std::string_view(b.name); + }); + return result; + } }; struct ExprLambda : Expr @@ -232,11 +247,6 @@ struct ExprLambda : Expr 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({ - .msg = hintfmt("duplicate formal function argument '%1%'", arg), - .errPos = pos - }); }; void setName(Symbol & name); string showNamePos() const; diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 88bdebcd6..1b0177e4a 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -41,6 +41,11 @@ namespace nix { { }; }; + struct ParserFormals { + std::vector formals; + bool ellipsis = false; + }; + } // using C a struct allows us to avoid having to define the special @@ -151,15 +156,39 @@ static void addAttr(ExprAttrs * attrs, AttrPath & attrPath, } -static void addFormal(const Pos & pos, Formals * formals, const Formal & formal) +static Formals * toFormals(ParseData & data, ParserFormals * formals, + Pos pos = noPos, Symbol arg = {}) { - if (!formals->argNames.insert(formal.name).second) + 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) + continue; + std::pair thisDup{formals->formals[i].name, formals->formals[i + 1].pos}; + duplicate = std::min(thisDup, duplicate.value_or(thisDup)); + } + if (duplicate) throw ParseError({ - .msg = hintfmt("duplicate formal function argument '%1%'", - formal.name), + .msg = hintfmt("duplicate formal function argument '%1%'", duplicate->first), + .errPos = duplicate->second + }); + + Formals result; + result.ellipsis = formals->ellipsis; + result.formals = std::move(formals->formals); + + if (arg.set() && result.has(arg)) + throw ParseError({ + .msg = hintfmt("duplicate formal function argument '%1%'", arg), .errPos = pos }); - formals->formals.push_front(formal); + + delete formals; + return new Formals(std::move(result)); } @@ -282,7 +311,7 @@ void yyerror(YYLTYPE * loc, yyscan_t scanner, ParseData * data, const char * err nix::Expr * e; nix::ExprList * list; nix::ExprAttrs * attrs; - nix::Formals * formals; + nix::ParserFormals * formals; nix::Formal * formal; nix::NixInt n; nix::NixFloat nf; @@ -340,11 +369,17 @@ expr_function : ID ':' expr_function { $$ = new ExprLambda(CUR_POS, data->symbols.create($1), 0, $3); } | '{' formals '}' ':' expr_function - { $$ = new ExprLambda(CUR_POS, data->symbols.create(""), $2, $5); } + { $$ = new ExprLambda(CUR_POS, data->symbols.create(""), toFormals(*data, $2), $5); } | '{' formals '}' '@' ID ':' expr_function - { $$ = new ExprLambda(CUR_POS, data->symbols.create($5), $2, $7); } + { + Symbol arg = data->symbols.create($5); + $$ = new ExprLambda(CUR_POS, arg, toFormals(*data, $2, CUR_POS, arg), $7); + } | ID '@' '{' formals '}' ':' expr_function - { $$ = new ExprLambda(CUR_POS, data->symbols.create($1), $4, $7); } + { + Symbol arg = data->symbols.create($1); + $$ = new ExprLambda(CUR_POS, arg, toFormals(*data, $4, CUR_POS, arg), $7); + } | ASSERT expr ';' expr_function { $$ = new ExprAssert(CUR_POS, $2, $4); } | WITH expr ';' expr_function @@ -575,13 +610,13 @@ expr_list formals : formal ',' formals - { $$ = $3; addFormal(CUR_POS, $$, *$1); } + { $$ = $3; $$->formals.push_back(*$1); } | formal - { $$ = new Formals; addFormal(CUR_POS, $$, *$1); $$->ellipsis = false; } + { $$ = new ParserFormals; $$->formals.push_back(*$1); $$->ellipsis = false; } | - { $$ = new Formals; $$->ellipsis = false; } + { $$ = new ParserFormals; $$->ellipsis = false; } | ELLIPSIS - { $$ = new Formals; $$->ellipsis = true; } + { $$ = new ParserFormals; $$->ellipsis = true; } ; formal diff --git a/src/libexpr/value-to-xml.cc b/src/libexpr/value-to-xml.cc index a875f82d7..a9fb60b0e 100644 --- a/src/libexpr/value-to-xml.cc +++ b/src/libexpr/value-to-xml.cc @@ -142,7 +142,7 @@ static void printValueAsXML(EvalState & state, bool strict, bool location, if (!v.lambda.fun->arg.empty()) attrs["name"] = v.lambda.fun->arg; if (v.lambda.fun->formals->ellipsis) attrs["ellipsis"] = "1"; XMLOpenElement _(doc, "attrspat", attrs); - for (auto & i : v.lambda.fun->formals->formals) + for (auto & i : v.lambda.fun->formals->lexicographicOrder()) doc.writeEmptyElement("attr", singletonAttrs("name", i.name)); } else doc.writeEmptyElement("varpat", singletonAttrs("name", v.lambda.fun->arg)); diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 97f4d911c..101d38c6c 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -476,8 +476,8 @@ struct CmdFlakeCheck : FlakeCommand if (!v.isLambda()) throw Error("bundler must be a function"); if (!v.lambda.fun->formals || - !v.lambda.fun->formals->argNames.count(state->symbols.create("program")) || - !v.lambda.fun->formals->argNames.count(state->symbols.create("system"))) + !v.lambda.fun->formals->has(state->symbols.create("program")) || + !v.lambda.fun->formals->has(state->symbols.create("system"))) throw Error("bundler must take formal arguments 'program' and 'system'"); } catch (Error & e) { e.addTrace(pos, hintfmt("while checking the template '%s'", attrPath)); diff --git a/tests/check.nix b/tests/check.nix index ec455ae2d..ed91ff845 100644 --- a/tests/check.nix +++ b/tests/check.nix @@ -50,6 +50,6 @@ with import ./config.nix; fetchurl = import { url = "file://" + toString ./lang/eval-okay-xml.exp.xml; - sha256 = "0kg4sla7ihm8ijr8cb3117fhl99zrc2bwy1jrngsfmkh8bav4m0v"; + sha256 = "sha256-behBlX+DQK/Pjvkuc8Tx68Jwi4E5v86wDq+ZLaHyhQE="; }; } diff --git a/tests/lang/eval-okay-xml.exp.xml b/tests/lang/eval-okay-xml.exp.xml index 92b75e0b8..20099326c 100644 --- a/tests/lang/eval-okay-xml.exp.xml +++ b/tests/lang/eval-okay-xml.exp.xml @@ -31,9 +31,9 @@ - +