From 136f2f7046dfed18fde0b5f9933ddfafc1518ef5 Mon Sep 17 00:00:00 2001 From: Shea Levy Date: Fri, 13 Sep 2013 16:55:33 -0400 Subject: [PATCH 1/4] Add the ExprBuiltin Expr type to the AST Certain desugaring schemes may require the parser to use some builtin function to do some of the work (e.g. currently `throw` is used to lazily cause an error if a `<>`-style path is not in the search path) Unfortunately, these names are not reserved keywords, so an expression that uses such a syntactic sugar will not see the expected behavior (see tests/lang/eval-okay-redefine-builtin.nix for an example). This adds the ExprBuiltin AST type, which when evaluated uses the value from the rootmost variable scope (which of course is initialized internally and can't shadow any of the builtins). Signed-off-by: Shea Levy --- src/libexpr/eval.cc | 11 +++++++++++ src/libexpr/nixexpr.cc | 9 +++++++++ src/libexpr/nixexpr.hh | 7 +++++++ src/libexpr/parser.y | 18 +++++++++--------- tests/lang/eval-okay-redefine-builtin.exp | 1 + tests/lang/eval-okay-redefine-builtin.nix | 3 +++ 6 files changed, 40 insertions(+), 9 deletions(-) create mode 100644 tests/lang/eval-okay-redefine-builtin.exp create mode 100644 tests/lang/eval-okay-redefine-builtin.nix diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 3db4bb66f4..538325601b 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -908,6 +908,17 @@ void ExprOpNot::eval(EvalState & state, Env & env, Value & v) } +void ExprBuiltin::eval(EvalState & state, Env & env, Value & v) +{ + // Not a hot path at all, but would be nice to access state.baseEnv directly + Env *baseEnv = &env; + while (baseEnv->up) baseEnv = baseEnv->up; + Bindings::iterator binding = baseEnv->values[0]->attrs->find(name); + assert(binding != baseEnv->values[0]->attrs->end()); + v = *binding->value; +} + + void ExprOpEq::eval(EvalState & state, Env & env, Value & v) { Value v1; e1->eval(state, env, v1); diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index f4b4295e29..c7ac967968 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -121,6 +121,11 @@ void ExprOpNot::show(std::ostream & str) str << "! " << *e; } +void ExprBuiltin::show(std::ostream & str) +{ + str << "builtins." << name; +} + void ExprConcatStrings::show(std::ostream & str) { bool first = true; @@ -314,6 +319,10 @@ void ExprOpNot::bindVars(const StaticEnv & env) e->bindVars(env); } +void ExprBuiltin::bindVars(const StaticEnv & env) +{ +} + void ExprConcatStrings::bindVars(const StaticEnv & env) { foreach (vector::iterator, i, *es) diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index 61eb81fab1..2ae2e2e96f 100644 --- a/src/libexpr/nixexpr.hh +++ b/src/libexpr/nixexpr.hh @@ -248,6 +248,13 @@ struct ExprOpNot : Expr COMMON_METHODS }; +struct ExprBuiltin : Expr +{ + Symbol name; + ExprBuiltin(Symbol name) : name(name) { }; + COMMON_METHODS +}; + #define MakeBinOp(name, s) \ struct Expr##name : Expr \ { \ diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 7699cf502b..d30882ac84 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -311,13 +311,13 @@ expr_if expr_op : '!' expr_op %prec NOT { $$ = new ExprOpNot($2); } -| '-' expr_op %prec NEGATE { $$ = new ExprApp(new ExprApp(new ExprVar(noPos, data->symbols.create("__sub")), new ExprInt(0)), $2); } +| '-' expr_op %prec NEGATE { $$ = new ExprApp(new ExprApp(new ExprBuiltin(data->symbols.create("sub")), new ExprInt(0)), $2); } | expr_op EQ expr_op { $$ = new ExprOpEq($1, $3); } | expr_op NEQ expr_op { $$ = new ExprOpNEq($1, $3); } - | expr_op '<' expr_op { $$ = new ExprApp(new ExprApp(new ExprVar(noPos, data->symbols.create("__lessThan")), $1), $3); } - | expr_op LEQ expr_op { $$ = new ExprOpNot(new ExprApp(new ExprApp(new ExprVar(noPos, data->symbols.create("__lessThan")), $3), $1)); } - | expr_op '>' expr_op { $$ = new ExprApp(new ExprApp(new ExprVar(noPos, data->symbols.create("__lessThan")), $3), $1); } - | expr_op GEQ expr_op { $$ = new ExprOpNot(new ExprApp(new ExprApp(new ExprVar(noPos, data->symbols.create("__lessThan")), $1), $3)); } + | expr_op '<' expr_op { $$ = new ExprApp(new ExprApp(new ExprBuiltin(data->symbols.create("lessThan")), $1), $3); } + | expr_op LEQ expr_op { $$ = new ExprOpNot(new ExprApp(new ExprApp(new ExprBuiltin(data->symbols.create("lessThan")), $3), $1)); } + | expr_op '>' expr_op { $$ = new ExprApp(new ExprApp(new ExprBuiltin(data->symbols.create("lessThan")), $3), $1); } + | expr_op GEQ expr_op { $$ = new ExprOpNot(new ExprApp(new ExprApp(new ExprBuiltin(data->symbols.create("lessThan")), $1), $3)); } | expr_op AND expr_op { $$ = new ExprOpAnd($1, $3); } | expr_op OR expr_op { $$ = new ExprOpOr($1, $3); } | expr_op IMPL expr_op { $$ = new ExprOpImpl($1, $3); } @@ -329,9 +329,9 @@ expr_op l->push_back($3); $$ = new ExprConcatStrings(false, l); } - | expr_op '-' expr_op { $$ = new ExprApp(new ExprApp(new ExprVar(noPos, data->symbols.create("__sub")), $1), $3); } - | expr_op '*' expr_op { $$ = new ExprApp(new ExprApp(new ExprVar(noPos, data->symbols.create("__mul")), $1), $3); } - | expr_op '/' expr_op { $$ = new ExprApp(new ExprApp(new ExprVar(noPos, data->symbols.create("__div")), $1), $3); } + | expr_op '-' expr_op { $$ = new ExprApp(new ExprApp(new ExprBuiltin(data->symbols.create("sub")), $1), $3); } + | expr_op '*' expr_op { $$ = new ExprApp(new ExprApp(new ExprBuiltin(data->symbols.create("mul")), $1), $3); } + | expr_op '/' expr_op { $$ = new ExprApp(new ExprApp(new ExprBuiltin(data->symbols.create("div")), $1), $3); } | expr_op CONCAT expr_op { $$ = new ExprOpConcatLists($1, $3); } | expr_app ; @@ -381,7 +381,7 @@ expr_simple ‘throw’. */ $$ = path2 == "" ? (Expr * ) new ExprApp( - new ExprVar(noPos, data->symbols.create("throw")), + new ExprBuiltin(data->symbols.create("throw")), new ExprString(data->symbols.create( (format("file `%1%' was not found in the Nix search path (add it using $NIX_PATH or -I)") % path).str()))) : (Expr * ) new ExprPath(path2); diff --git a/tests/lang/eval-okay-redefine-builtin.exp b/tests/lang/eval-okay-redefine-builtin.exp new file mode 100644 index 0000000000..c508d5366f --- /dev/null +++ b/tests/lang/eval-okay-redefine-builtin.exp @@ -0,0 +1 @@ +false diff --git a/tests/lang/eval-okay-redefine-builtin.nix b/tests/lang/eval-okay-redefine-builtin.nix new file mode 100644 index 0000000000..df9fc3f37d --- /dev/null +++ b/tests/lang/eval-okay-redefine-builtin.nix @@ -0,0 +1,3 @@ +let + throw = abort "Error!"; +in (builtins.tryEval ).success From 18fefacf7df570444b332a8a0c2dc4f9d98d4344 Mon Sep 17 00:00:00 2001 From: Shea Levy Date: Fri, 20 Sep 2013 23:25:30 -0400 Subject: [PATCH 2/4] Dynamic attrs This adds new syntax for attribute names: * attrs."${name}" => getAttr name attrs * attrs ? "${name}" => isAttrs attrs && hasAttr attrs name * attrs."${name}" or def => if attrs ? "${name}" then attrs."${name}" else def * { "${name}" = value; } => listToAttrs [{ inherit name value; }] Of course, it's a bit more complicated than that. The attribute chains can be arbitrarily long and contain combinations of static and dynamic parts (e.g. attrs."${foo}".bar."${baz}" or qux), which is relatively straightforward for the getAttrs/hasAttrs cases but is more complex for the listToAttrs case due to rules about duplicate attribute definitions. For attribute sets with dynamic attribute names, duplicate static attributes are detected at parse time while duplicate dynamic attributes are detected when the attribute set is forced. So, for example, { a = null; a.b = null; "${"c"}" = true; } will be a parse-time error, while { a = {}; "${"a"}".b = null; c = true; } will be an eval-time error (technically that case could theoretically be detected at parse time, but the general case would require full evaluation). Moreover, duplicate dynamic attributes are not allowed even in cases where they would be with static attributes ({ a.b.d = true; a.b.c = false; } is legal, but { a."${"b"}".d = true; a."${"b"}".c = false; } is not). This restriction might be relaxed in the future in cases where the static variant would not be an error, but it is not obvious that that is desirable. Finally, recursive attribute sets with dynamic attributes have the static attributes in scope but not the dynamic ones. So rec { a = true; "${"b"}" = a; } is equivalent to { a = true; b = true; } but rec { "${"a"}" = true; b = a; } would be an error or use a from the surrounding scope if it exists. Note that the getAttr, getAttr or default, and hasAttr are all implemented purely in the parser as syntactic sugar, while attribute sets with dynamic attribute names required changes to the AST to be implemented cleanly. This is an alternative solution to and closes #167 Signed-off-by: Shea Levy --- src/libexpr/eval.cc | 24 ++- src/libexpr/nixexpr.cc | 9 + src/libexpr/nixexpr.hh | 10 +- src/libexpr/parser.y | 218 ++++++++++++++++++++++--- tests/lang/eval-okay-dynamic-attrs.exp | 1 + tests/lang/eval-okay-dynamic-attrs.nix | 17 ++ 6 files changed, 256 insertions(+), 23 deletions(-) create mode 100644 tests/lang/eval-okay-dynamic-attrs.exp create mode 100644 tests/lang/eval-okay-dynamic-attrs.nix diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 538325601b..cd3edecaa7 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -247,6 +247,11 @@ LocalNoInlineNoReturn(void throwEvalError(const char * s, const string & s2, con throw EvalError(format(s) % s2 % s3); } +LocalNoInlineNoReturn(void throwEvalError(const char * s, const Symbol & sym, const Pos & p1, const Pos & p2)) +{ + throw EvalError(format(s) % sym % p1 % p2); +} + LocalNoInlineNoReturn(void throwTypeError(const char * s)) { throw TypeError(s); @@ -557,12 +562,14 @@ void ExprPath::eval(EvalState & state, Env & env, Value & v) void ExprAttrs::eval(EvalState & state, Env & env, Value & v) { state.mkAttrs(v, attrs.size()); + Env *dynamicEnv = &env; if (recursive) { /* Create a new environment that contains the attributes in this `rec'. */ Env & env2(state.allocEnv(attrs.size())); env2.up = &env; + dynamicEnv = &env2; AttrDefs::iterator overrides = attrs.find(state.sOverrides); bool hasOverrides = overrides != attrs.end(); @@ -605,9 +612,24 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v) } } - else { + else foreach (AttrDefs::iterator, i, attrs) v.attrs->push_back(Attr(i->first, i->second.e->maybeThunk(state, env), &i->second.pos)); + + /* dynamic attrs apply *after* rec and __overrides */ + foreach (DynamicAttrDefs::iterator, i, dynamicAttrs) { + Value nameVal; + i->nameExpr->eval(state, *dynamicEnv, nameVal); + state.forceStringNoCtx(nameVal); + Symbol nameSym = state.symbols.create(nameVal.string.s); + Bindings::iterator j = v.attrs->find(nameSym); + if (j != v.attrs->end()) + throwEvalError("dynamic attribute `%1%' at %2% already defined at %3%", nameSym, i->pos, *j->pos); + + i->valueExpr->setName(nameSym); + /* Keep sorted order so find can catch duplicates */ + v.attrs->insert(lower_bound(v.attrs->begin(), v.attrs->end(), Attr(nameSym, 0)), + Attr(nameSym, i->valueExpr->maybeThunk(state, *dynamicEnv), &i->pos)); } } diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index c7ac967968..a7ce58c4d9 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -61,6 +61,8 @@ void ExprAttrs::show(std::ostream & str) str << "inherit " << i->first << " " << "; "; else str << i->first << " = " << *i->second.e << "; "; + foreach (DynamicAttrDefs::iterator, i, dynamicAttrs) + str << "\"${" << *i->nameExpr << "}\" = " << *i->valueExpr << "; "; str << "}"; } @@ -227,8 +229,10 @@ void ExprOpHasAttr::bindVars(const StaticEnv & env) void ExprAttrs::bindVars(const StaticEnv & env) { + const StaticEnv *dynamicEnv = &env; if (recursive) { StaticEnv newEnv(false, &env); + dynamicEnv = &newEnv; unsigned int displ = 0; foreach (AttrDefs::iterator, i, attrs) @@ -241,6 +245,11 @@ void ExprAttrs::bindVars(const StaticEnv & env) else foreach (AttrDefs::iterator, i, attrs) i->second.e->bindVars(env); + + foreach (DynamicAttrDefs::iterator, i, dynamicAttrs) { + i->nameExpr->bindVars(*dynamicEnv); + i->valueExpr->bindVars(*dynamicEnv); + } } void ExprList::bindVars(const StaticEnv & env) diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index 2ae2e2e96f..92c2ca8dc5 100644 --- a/src/libexpr/nixexpr.hh +++ b/src/libexpr/nixexpr.hh @@ -163,6 +163,14 @@ struct ExprAttrs : Expr }; typedef std::map AttrDefs; AttrDefs attrs; + struct DynamicAttrDef { + Expr * nameExpr; + Expr * valueExpr; + Pos pos; + DynamicAttrDef(Expr * nameExpr, Expr * valueExpr, const Pos & pos) : nameExpr(nameExpr), valueExpr(valueExpr), pos(pos) { }; + }; + typedef std::vector DynamicAttrDefs; + DynamicAttrDefs dynamicAttrs; ExprAttrs() : recursive(false) { }; COMMON_METHODS }; @@ -251,7 +259,7 @@ struct ExprOpNot : Expr struct ExprBuiltin : Expr { Symbol name; - ExprBuiltin(Symbol name) : name(name) { }; + ExprBuiltin(const Symbol & name) : name(name) { }; COMMON_METHODS }; diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index d30882ac84..aa08e1a63e 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -39,6 +39,15 @@ namespace nix { { }; }; + struct AttrName + { + Symbol symbol; + Expr *expr; + AttrName(const Symbol & s) : symbol(s) {}; + AttrName(Expr *e) : expr(e) {}; + }; + + typedef std::vector AttrNames; } #define YY_DECL int yylex \ @@ -243,7 +252,8 @@ void yyerror(YYLTYPE * loc, yyscan_t scanner, ParseData * data, const char * err char * id; // !!! -> Symbol char * path; char * uri; - std::vector * attrNames; + std::vector * attrpath; + std::vector * attrlist; std::vector * string_parts; } @@ -253,8 +263,10 @@ void yyerror(YYLTYPE * loc, yyscan_t scanner, ParseData * data, const char * err %type binds %type formals %type formal -%type attrs attrpath -%type string_parts ind_string_parts +%type attrpath +%type attrs +%type string_parts_interpolated ind_string_parts +%type string_parts string_attr %type attr %token ID ATTRPATH %token STR IND_STR @@ -300,7 +312,11 @@ expr_function | WITH expr ';' expr_function { $$ = new ExprWith(CUR_POS, $2, $4); } | LET binds IN expr_function - { $$ = new ExprLet($2, $4); } + { if (!$2->dynamicAttrs.empty()) + throw ParseError(format("dynamic attributes not allowed in let at %1%") + % CUR_POS); + $$ = new ExprLet($2, $4); + } | expr_if ; @@ -322,7 +338,39 @@ expr_op | expr_op OR expr_op { $$ = new ExprOpOr($1, $3); } | expr_op IMPL expr_op { $$ = new ExprOpImpl($1, $3); } | expr_op UPDATE expr_op { $$ = new ExprOpUpdate($1, $3); } - | expr_op '?' attrpath { $$ = new ExprOpHasAttr($1, *$3); } + | expr_op '?' attrpath + { AttrPath path; + vector::iterator i; + $$ = $1; + // All attrpaths have at least one attr + assert(!$3->empty()); + for (i = $3->begin(); i + 1 != $3->end(); i++) { + if (i->symbol.set()) { + path.push_back(i->symbol); + } else { + if (!path.empty()) { + $$ = new ExprSelect($$, path, new ExprAttrs()); + path.clear(); + } + $$ = new ExprIf( + new ExprOpAnd( + new ExprApp(new ExprBuiltin(data->symbols.create("isAttrs")), $$), + new ExprApp(new ExprApp(new ExprBuiltin(data->symbols.create("hasAttr")), i->expr), $$)), + new ExprApp(new ExprApp(new ExprBuiltin(data->symbols.create("getAttr")), i->expr), $$), + new ExprAttrs()); + } + } + if (i->symbol.set()) { + path.push_back(i->symbol); + $$ = new ExprOpHasAttr($$, path); + } else { + if (!path.empty()) + $$ = new ExprSelect($$, path, new ExprAttrs()); + $$ = new ExprOpAnd( + new ExprApp(new ExprBuiltin(data->symbols.create("isAttrs")), $$), + new ExprApp(new ExprApp(new ExprBuiltin(data->symbols.create("hasAttr")), i->expr), $$)); + } + } | expr_op '+' expr_op { vector * l = new vector; l->push_back($1); @@ -344,9 +392,58 @@ expr_app expr_select : expr_simple '.' attrpath - { $$ = new ExprSelect($1, *$3, 0); } + { AttrPath path; + $$ = $1; + foreach (vector::iterator, i, *$3) { + if (i->symbol.set()) { + path.push_back(i->symbol); + } else { + if (!path.empty()) { + $$ = new ExprSelect($$, path, 0); + path.clear(); + } + $$ = new ExprApp(new ExprApp(new ExprBuiltin(data->symbols.create("getAttr")), i->expr), $$); + } + } + if (!path.empty()) + $$ = new ExprSelect($$, path, 0); + } | expr_simple '.' attrpath OR_KW expr_select - { $$ = new ExprSelect($1, *$3, $5); } + { AttrPath path; + vector::iterator i; + $$ = $1; + // All attrpaths have at least one attr + assert(!$3->empty()); + for (i = $3->begin(); i + 1 != $3->end(); i++) { + if (i->symbol.set()) { + path.push_back(i->symbol); + } else { + if (!path.empty()) { + $$ = new ExprSelect($$, path, new ExprAttrs()); + path.clear(); + } + $$ = new ExprIf( + new ExprOpAnd( + new ExprApp(new ExprBuiltin(data->symbols.create("isAttrs")), $$), + new ExprApp(new ExprApp(new ExprBuiltin(data->symbols.create("hasAttr")), i->expr), $$)), + new ExprApp(new ExprApp(new ExprBuiltin(data->symbols.create("getAttr")), i->expr), $$), + new ExprAttrs()); + } + } + if (i->symbol.set()) { + path.push_back(i->symbol); + $$ = new ExprSelect($$, path, $5); + } else { + if (!path.empty()) + $$ = new ExprSelect($$, path, new ExprAttrs()); + $$ = new ExprIf( + new ExprOpAnd( + new ExprApp(new ExprBuiltin(data->symbols.create("isAttrs")), $$), + new ExprApp(new ExprApp(new ExprBuiltin(data->symbols.create("hasAttr")), i->expr), $$)), + new ExprApp(new ExprApp(new ExprBuiltin(data->symbols.create("getAttr")), i->expr), $$), + $5); + } + } | /* Backwards compatibility: because Nixpkgs has a rarely used function named ‘or’, allow stuff like ‘map or [...]’. */ expr_simple OR_KW @@ -362,12 +459,7 @@ expr_simple $$ = new ExprVar(CUR_POS, data->symbols.create($1)); } | INT { $$ = new ExprInt($1); } - | '"' string_parts '"' { - /* For efficiency, and to simplify parse trees a bit. */ - if ($2->empty()) $$ = new ExprString(data->symbols.create("")); - else if ($2->size() == 1 && dynamic_cast($2->front())) $$ = $2->front(); - else $$ = new ExprConcatStrings(true, $2); - } + | '"' string_parts '"' { $$ = $2; } | IND_STRING_OPEN ind_string_parts IND_STRING_CLOSE { $$ = stripIndentation(data->symbols, *$2); } @@ -400,9 +492,27 @@ expr_simple ; string_parts - : string_parts STR { $$ = $1; $1->push_back($2); } - | string_parts DOLLAR_CURLY expr '}' { backToString(scanner); $$ = $1; $1->push_back($3); } - | { $$ = new vector; } + : STR + | string_parts_interpolated { $$ = new ExprConcatStrings(true, $1); } + | { $$ = new ExprString(data->symbols.create("")) } + ; + +string_parts_interpolated + : string_parts_interpolated STR { $$ = $1; $1->push_back($2); } + | string_parts_interpolated DOLLAR_CURLY expr '}' { backToString(scanner); $$ = $1; $1->push_back($3); } + | STR DOLLAR_CURLY expr '}' + { + backToString(scanner); + $$ = new vector; + $$->push_back($1); + $$->push_back($3); + } + | DOLLAR_CURLY expr '}' + { + backToString(scanner); + $$ = new vector; + $$->push_back($2); + } ; ind_string_parts @@ -412,7 +522,43 @@ ind_string_parts ; binds - : binds attrpath '=' expr ';' { $$ = $1; addAttr($$, *$2, $4, makeCurPos(@2, data)); } + : binds attrpath '=' expr ';' + { + ExprAttrs *curAttrs = $1; + AttrPath path; + vector::iterator i; + // All attrpaths have at least one attr + assert(!$2->empty()); + for (i = $2->begin(); i + 1 < $2->end(); i++) { + if (i->symbol.set()) { + path.push_back(i->symbol); + } else { + ExprAttrs *temp; + if (!path.empty()) { + temp = curAttrs; + curAttrs = new ExprAttrs; + addAttr(temp, path, curAttrs, makeCurPos(@2, data)); + } + path.clear(); + + temp = curAttrs; + curAttrs = new ExprAttrs; + temp->dynamicAttrs.push_back(ExprAttrs::DynamicAttrDef(i->expr, curAttrs, makeCurPos(@2, data))); + } + } + if (i->symbol.set()) { + path.push_back(i->symbol); + addAttr(curAttrs, path, $4, makeCurPos(@2, data)); + } else { + if (!path.empty()) { + ExprAttrs *temp = curAttrs; + curAttrs = new ExprAttrs; + addAttr(temp, path, curAttrs, makeCurPos(@2, data)); + } + curAttrs->dynamicAttrs.push_back(ExprAttrs::DynamicAttrDef(i->expr, $4, makeCurPos(@2, data))); + } + $$ = $1; + } | binds INHERIT attrs ';' { $$ = $1; foreach (AttrPath::iterator, i, *$3) { @@ -436,19 +582,49 @@ binds attrs : attrs attr { $$ = $1; $1->push_back(data->symbols.create($2)); /* !!! dangerous */ } + | attrs string_attr + { $$ = $1; + ExprString *str = dynamic_cast($2); + if (str) { + $$->push_back(str->s); + delete str; + } else + throw ParseError(format("dynamic attributes not allowed in inherit at %1%") + % makeCurPos(@2, data)); + } | { $$ = new vector; } ; attrpath - : attrpath '.' attr { $$ = $1; $1->push_back(data->symbols.create($3)); } - | attr { $$ = new vector; $$->push_back(data->symbols.create($1)); } + : attrpath '.' attr { $$ = $1; $1->push_back(AttrName(data->symbols.create($3))); } + | attrpath '.' string_attr + { $$ = $1; + ExprString *str = dynamic_cast($3); + if (str) { + $$->push_back(AttrName(str->s)); + delete str; + } else + $$->push_back(AttrName($3)); + } + | attr { $$ = new vector; $$->push_back(AttrName(data->symbols.create($1))); } + | string_attr + { $$ = new vector; + ExprString *str = dynamic_cast($1); + if (str) { + $$->push_back(AttrName(str->s)); + delete str; + } else + $$->push_back(AttrName($1)); + } ; attr : ID { $$ = $1; } | OR_KW { $$ = "or"; } - | '"' STR '"' - { $$ = strdup(((string) ((ExprString *) $2)->s).c_str()); delete $2; } + ; + +string_attr + : '"' string_parts '"' { $$ = $2; } ; expr_list diff --git a/tests/lang/eval-okay-dynamic-attrs.exp b/tests/lang/eval-okay-dynamic-attrs.exp new file mode 100644 index 0000000000..df8750afc0 --- /dev/null +++ b/tests/lang/eval-okay-dynamic-attrs.exp @@ -0,0 +1 @@ +{ binds = true; hasAttrs = true; multiAttrs = true; recBinds = true; selectAttrs = true; selectOrAttrs = true; } diff --git a/tests/lang/eval-okay-dynamic-attrs.nix b/tests/lang/eval-okay-dynamic-attrs.nix new file mode 100644 index 0000000000..ee02ac7e65 --- /dev/null +++ b/tests/lang/eval-okay-dynamic-attrs.nix @@ -0,0 +1,17 @@ +let + aString = "a"; + + bString = "b"; +in { + hasAttrs = { a.b = null; } ? "${aString}".b; + + selectAttrs = { a.b = true; }.a."${bString}"; + + selectOrAttrs = { }."${aString}" or true; + + binds = { "${aString}"."${bString}c" = true; }.a.bc; + + recBinds = rec { "${bString}" = a; a = true; }.b; + + multiAttrs = { "${aString}" = true; "${bString}" = false; }.a; +} From 6f3a51809a2603574a16573bd46b95e4ff5233bd Mon Sep 17 00:00:00 2001 From: Shea Levy Date: Tue, 31 Dec 2013 17:57:10 -0500 Subject: [PATCH 3/4] Fold dynamic binds handling into addAttr Since addAttr has to iterate through the AttrPath we pass it, it makes more sense to just iterate through the AttrNames in addAttr instead. As an added bonus, this allows attrsets where two dynamic attribute paths have the same static leading part (see added test case for an example that failed previously). Signed-off-by: Shea Levy --- src/libexpr/parser.y | 90 +++++++++--------------- tests/lang/eval-okay-dynamic-attrs-2.exp | 1 + tests/lang/eval-okay-dynamic-attrs-2.nix | 1 + 3 files changed, 37 insertions(+), 55 deletions(-) create mode 100644 tests/lang/eval-okay-dynamic-attrs-2.exp create mode 100644 tests/lang/eval-okay-dynamic-attrs-2.nix diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index aa08e1a63e..3bee3b010c 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -89,31 +89,47 @@ static void dupAttr(Symbol attr, const Pos & pos, const Pos & prevPos) } -static void addAttr(ExprAttrs * attrs, AttrPath & attrPath, +static void addAttr(ExprAttrs * attrs, AttrNames & attrNames, Expr * e, const Pos & pos) { - unsigned int n = 0; - foreach (AttrPath::const_iterator, i, attrPath) { - n++; - ExprAttrs::AttrDefs::iterator j = attrs->attrs.find(*i); - if (j != attrs->attrs.end()) { - if (!j->second.inherited) { - ExprAttrs * attrs2 = dynamic_cast(j->second.e); - if (!attrs2 || n == attrPath.size()) dupAttr(attrPath, pos, j->second.pos); - attrs = attrs2; - } else - dupAttr(attrPath, pos, j->second.pos); - } else { - if (n == attrPath.size()) - attrs->attrs[*i] = ExprAttrs::AttrDef(e, pos); - else { + AttrPath path; + AttrNames::iterator i; + // All attrpaths have at least one attr + assert(!attrNames.empty()); + for (i = attrNames.begin(); i + 1 < attrNames.end(); i++) { + if (i->symbol.set()) { + path.push_back(i->symbol); + ExprAttrs::AttrDefs::iterator j = attrs->attrs.find(i->symbol); + if (j != attrs->attrs.end()) { + if (!j->second.inherited) { + ExprAttrs * attrs2 = dynamic_cast(j->second.e); + if (!attrs2) dupAttr(path, pos, j->second.pos); + attrs = attrs2; + } else + dupAttr(path, pos, j->second.pos); + } else { + path.clear(); ExprAttrs * nested = new ExprAttrs; - attrs->attrs[*i] = ExprAttrs::AttrDef(nested, pos); + attrs->attrs[i->symbol] = ExprAttrs::AttrDef(nested, pos); attrs = nested; } + } else { + ExprAttrs *nested = new ExprAttrs; + attrs->dynamicAttrs.push_back(ExprAttrs::DynamicAttrDef(i->expr, nested, pos)); + attrs = nested; } } - e->setName(attrPath.back()); + if (i->symbol.set()) { + ExprAttrs::AttrDefs::iterator j = attrs->attrs.find(i->symbol); + if (j != attrs->attrs.end()) { + dupAttr(path, pos, j->second.pos); + } else { + attrs->attrs[i->symbol] = ExprAttrs::AttrDef(e, pos); + e->setName(i->symbol); + } + } else { + attrs->dynamicAttrs.push_back(ExprAttrs::DynamicAttrDef(i->expr, e, pos)); + } } @@ -522,43 +538,7 @@ ind_string_parts ; binds - : binds attrpath '=' expr ';' - { - ExprAttrs *curAttrs = $1; - AttrPath path; - vector::iterator i; - // All attrpaths have at least one attr - assert(!$2->empty()); - for (i = $2->begin(); i + 1 < $2->end(); i++) { - if (i->symbol.set()) { - path.push_back(i->symbol); - } else { - ExprAttrs *temp; - if (!path.empty()) { - temp = curAttrs; - curAttrs = new ExprAttrs; - addAttr(temp, path, curAttrs, makeCurPos(@2, data)); - } - path.clear(); - - temp = curAttrs; - curAttrs = new ExprAttrs; - temp->dynamicAttrs.push_back(ExprAttrs::DynamicAttrDef(i->expr, curAttrs, makeCurPos(@2, data))); - } - } - if (i->symbol.set()) { - path.push_back(i->symbol); - addAttr(curAttrs, path, $4, makeCurPos(@2, data)); - } else { - if (!path.empty()) { - ExprAttrs *temp = curAttrs; - curAttrs = new ExprAttrs; - addAttr(temp, path, curAttrs, makeCurPos(@2, data)); - } - curAttrs->dynamicAttrs.push_back(ExprAttrs::DynamicAttrDef(i->expr, $4, makeCurPos(@2, data))); - } - $$ = $1; - } + : binds attrpath '=' expr ';' { $$ = $1; addAttr($$, *$2, $4, makeCurPos(@2, data)); } | binds INHERIT attrs ';' { $$ = $1; foreach (AttrPath::iterator, i, *$3) { diff --git a/tests/lang/eval-okay-dynamic-attrs-2.exp b/tests/lang/eval-okay-dynamic-attrs-2.exp new file mode 100644 index 0000000000..27ba77ddaf --- /dev/null +++ b/tests/lang/eval-okay-dynamic-attrs-2.exp @@ -0,0 +1 @@ +true diff --git a/tests/lang/eval-okay-dynamic-attrs-2.nix b/tests/lang/eval-okay-dynamic-attrs-2.nix new file mode 100644 index 0000000000..6d57bf8549 --- /dev/null +++ b/tests/lang/eval-okay-dynamic-attrs-2.nix @@ -0,0 +1 @@ +{ a."${"b"}" = true; a."${"c"}" = false; }.a.b From cd49fe4f9b338242e1e404fd4dbb0a3ebc1c3a12 Mon Sep 17 00:00:00 2001 From: Shea Levy Date: Tue, 31 Dec 2013 23:56:26 +0000 Subject: [PATCH 4/4] Don't use any syntactic sugar for dynamic attrs This doesn't change any functionality but moves some behavior out of the parser and into the evaluator in order to simplify the code. Signed-off-by: Shea Levy --- src/libexpr/eval.cc | 20 +++++- src/libexpr/nixexpr.cc | 21 ++++-- src/libexpr/nixexpr.hh | 13 +++- src/libexpr/parser.y | 142 +++++++---------------------------------- 4 files changed, 68 insertions(+), 128 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index cd3edecaa7..3d8ee99340 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -129,6 +129,18 @@ string showType(const Value & v) } +Symbol getName(const AttrName & name, EvalState & state, Env & env) { + if (name.symbol.set()) { + return name.symbol; + } else { + Value nameValue; + name.expr->eval(state, env, nameValue); + state.forceStringNoCtx(nameValue); + return state.symbols.create(nameValue.string.s); + } +} + + EvalState::EvalState() : sWith(symbols.create("")) , sOutPath(symbols.create("outPath")) @@ -683,17 +695,18 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v) foreach (AttrPath::const_iterator, i, attrPath) { nrLookups++; Bindings::iterator j; + Symbol name = getName(*i, state, env); if (def) { state.forceValue(*vAttrs); if (vAttrs->type != tAttrs || - (j = vAttrs->attrs->find(*i)) == vAttrs->attrs->end()) + (j = vAttrs->attrs->find(name)) == vAttrs->attrs->end()) { def->eval(state, env, v); return; } } else { state.forceAttrs(*vAttrs); - if ((j = vAttrs->attrs->find(*i)) == vAttrs->attrs->end()) + if ((j = vAttrs->attrs->find(name)) == vAttrs->attrs->end()) throwEvalError("attribute `%1%' missing", showAttrPath(attrPath)); } vAttrs = j->value; @@ -724,8 +737,9 @@ void ExprOpHasAttr::eval(EvalState & state, Env & env, Value & v) foreach (AttrPath::const_iterator, i, attrPath) { state.forceValue(*vAttrs); Bindings::iterator j; + Symbol name = getName(*i, state, env); if (vAttrs->type != tAttrs || - (j = vAttrs->attrs->find(*i)) == vAttrs->attrs->end()) + (j = vAttrs->attrs->find(name)) == vAttrs->attrs->end()) { mkBool(v, false); return; diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index a7ce58c4d9..9f0bc2630d 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -155,12 +155,19 @@ std::ostream & operator << (std::ostream & str, const Pos & pos) string showAttrPath(const AttrPath & attrPath) { - string s; + std::ostringstream out; + bool first = true; foreach (AttrPath::const_iterator, i, attrPath) { - if (!s.empty()) s += '.'; - s += *i; + if (!first) + out << '.'; + else + first = false; + if (i->symbol.set()) + out << i->symbol; + else + out << "\"${" << *i->expr << "}\""; } - return s; + return out.str(); } @@ -220,11 +227,17 @@ void ExprSelect::bindVars(const StaticEnv & env) { e->bindVars(env); if (def) def->bindVars(env); + foreach (AttrPath::iterator, i, attrPath) + if (!i->symbol.set()) + i->expr->bindVars(env); } void ExprOpHasAttr::bindVars(const StaticEnv & env) { e->bindVars(env); + foreach (AttrPath::iterator, i, attrPath) + if (!i->symbol.set()) + i->expr->bindVars(env); } void ExprAttrs::bindVars(const StaticEnv & env) diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index 92c2ca8dc5..bc6993477c 100644 --- a/src/libexpr/nixexpr.hh +++ b/src/libexpr/nixexpr.hh @@ -50,10 +50,19 @@ struct Env; struct Value; struct EvalState; struct StaticEnv; +struct Expr; /* An attribute path is a sequence of attribute names. */ -typedef vector AttrPath; +struct AttrName +{ + Symbol symbol; + Expr *expr; + AttrName(const Symbol & s) : symbol(s) {}; + AttrName(Expr *e) : expr(e) {}; +}; + +typedef std::vector AttrPath; string showAttrPath(const AttrPath & attrPath); @@ -138,7 +147,7 @@ struct ExprSelect : Expr Expr * e, * def; AttrPath attrPath; ExprSelect(Expr * e, const AttrPath & attrPath, Expr * def) : e(e), def(def), attrPath(attrPath) { }; - ExprSelect(Expr * e, const Symbol & name) : e(e), def(0) { attrPath.push_back(name); }; + ExprSelect(Expr * e, const Symbol & name) : e(e), def(0) { attrPath.push_back(AttrName(name)); }; COMMON_METHODS }; diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 3bee3b010c..28972cf724 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -39,15 +39,6 @@ namespace nix { { }; }; - struct AttrName - { - Symbol symbol; - Expr *expr; - AttrName(const Symbol & s) : symbol(s) {}; - AttrName(Expr *e) : expr(e) {}; - }; - - typedef std::vector AttrNames; } #define YY_DECL int yylex \ @@ -83,32 +74,28 @@ static void dupAttr(const AttrPath & attrPath, const Pos & pos, const Pos & prev static void dupAttr(Symbol attr, const Pos & pos, const Pos & prevPos) { - AttrPath attrPath; attrPath.push_back(attr); throw ParseError(format("attribute `%1%' at %2% already defined at %3%") - % showAttrPath(attrPath) % pos % prevPos); + % attr % pos % prevPos); } -static void addAttr(ExprAttrs * attrs, AttrNames & attrNames, +static void addAttr(ExprAttrs * attrs, AttrPath & attrPath, Expr * e, const Pos & pos) { - AttrPath path; - AttrNames::iterator i; + AttrPath::iterator i; // All attrpaths have at least one attr - assert(!attrNames.empty()); - for (i = attrNames.begin(); i + 1 < attrNames.end(); i++) { + assert(!attrPath.empty()); + for (i = attrPath.begin(); i + 1 < attrPath.end(); i++) { if (i->symbol.set()) { - path.push_back(i->symbol); ExprAttrs::AttrDefs::iterator j = attrs->attrs.find(i->symbol); if (j != attrs->attrs.end()) { if (!j->second.inherited) { ExprAttrs * attrs2 = dynamic_cast(j->second.e); - if (!attrs2) dupAttr(path, pos, j->second.pos); + if (!attrs2) dupAttr(attrPath, pos, j->second.pos); attrs = attrs2; } else - dupAttr(path, pos, j->second.pos); + dupAttr(attrPath, pos, j->second.pos); } else { - path.clear(); ExprAttrs * nested = new ExprAttrs; attrs->attrs[i->symbol] = ExprAttrs::AttrDef(nested, pos); attrs = nested; @@ -122,7 +109,7 @@ static void addAttr(ExprAttrs * attrs, AttrNames & attrNames, if (i->symbol.set()) { ExprAttrs::AttrDefs::iterator j = attrs->attrs.find(i->symbol); if (j != attrs->attrs.end()) { - dupAttr(path, pos, j->second.pos); + dupAttr(attrPath, pos, j->second.pos); } else { attrs->attrs[i->symbol] = ExprAttrs::AttrDef(e, pos); e->setName(i->symbol); @@ -268,8 +255,7 @@ void yyerror(YYLTYPE * loc, yyscan_t scanner, ParseData * data, const char * err char * id; // !!! -> Symbol char * path; char * uri; - std::vector * attrpath; - std::vector * attrlist; + std::vector * attrNames; std::vector * string_parts; } @@ -279,8 +265,7 @@ void yyerror(YYLTYPE * loc, yyscan_t scanner, ParseData * data, const char * err %type binds %type formals %type formal -%type attrpath -%type attrs +%type attrs attrpath %type string_parts_interpolated ind_string_parts %type string_parts string_attr %type attr @@ -354,39 +339,7 @@ expr_op | expr_op OR expr_op { $$ = new ExprOpOr($1, $3); } | expr_op IMPL expr_op { $$ = new ExprOpImpl($1, $3); } | expr_op UPDATE expr_op { $$ = new ExprOpUpdate($1, $3); } - | expr_op '?' attrpath - { AttrPath path; - vector::iterator i; - $$ = $1; - // All attrpaths have at least one attr - assert(!$3->empty()); - for (i = $3->begin(); i + 1 != $3->end(); i++) { - if (i->symbol.set()) { - path.push_back(i->symbol); - } else { - if (!path.empty()) { - $$ = new ExprSelect($$, path, new ExprAttrs()); - path.clear(); - } - $$ = new ExprIf( - new ExprOpAnd( - new ExprApp(new ExprBuiltin(data->symbols.create("isAttrs")), $$), - new ExprApp(new ExprApp(new ExprBuiltin(data->symbols.create("hasAttr")), i->expr), $$)), - new ExprApp(new ExprApp(new ExprBuiltin(data->symbols.create("getAttr")), i->expr), $$), - new ExprAttrs()); - } - } - if (i->symbol.set()) { - path.push_back(i->symbol); - $$ = new ExprOpHasAttr($$, path); - } else { - if (!path.empty()) - $$ = new ExprSelect($$, path, new ExprAttrs()); - $$ = new ExprOpAnd( - new ExprApp(new ExprBuiltin(data->symbols.create("isAttrs")), $$), - new ExprApp(new ExprApp(new ExprBuiltin(data->symbols.create("hasAttr")), i->expr), $$)); - } - } + | expr_op '?' attrpath { $$ = new ExprOpHasAttr($1, *$3); } | expr_op '+' expr_op { vector * l = new vector; l->push_back($1); @@ -408,58 +361,9 @@ expr_app expr_select : expr_simple '.' attrpath - { AttrPath path; - $$ = $1; - foreach (vector::iterator, i, *$3) { - if (i->symbol.set()) { - path.push_back(i->symbol); - } else { - if (!path.empty()) { - $$ = new ExprSelect($$, path, 0); - path.clear(); - } - $$ = new ExprApp(new ExprApp(new ExprBuiltin(data->symbols.create("getAttr")), i->expr), $$); - } - } - if (!path.empty()) - $$ = new ExprSelect($$, path, 0); - } + { $$ = new ExprSelect($1, *$3, 0); } | expr_simple '.' attrpath OR_KW expr_select - { AttrPath path; - vector::iterator i; - $$ = $1; - // All attrpaths have at least one attr - assert(!$3->empty()); - for (i = $3->begin(); i + 1 != $3->end(); i++) { - if (i->symbol.set()) { - path.push_back(i->symbol); - } else { - if (!path.empty()) { - $$ = new ExprSelect($$, path, new ExprAttrs()); - path.clear(); - } - $$ = new ExprIf( - new ExprOpAnd( - new ExprApp(new ExprBuiltin(data->symbols.create("isAttrs")), $$), - new ExprApp(new ExprApp(new ExprBuiltin(data->symbols.create("hasAttr")), i->expr), $$)), - new ExprApp(new ExprApp(new ExprBuiltin(data->symbols.create("getAttr")), i->expr), $$), - new ExprAttrs()); - } - } - if (i->symbol.set()) { - path.push_back(i->symbol); - $$ = new ExprSelect($$, path, $5); - } else { - if (!path.empty()) - $$ = new ExprSelect($$, path, new ExprAttrs()); - $$ = new ExprIf( - new ExprOpAnd( - new ExprApp(new ExprBuiltin(data->symbols.create("isAttrs")), $$), - new ExprApp(new ExprApp(new ExprBuiltin(data->symbols.create("hasAttr")), i->expr), $$)), - new ExprApp(new ExprApp(new ExprBuiltin(data->symbols.create("getAttr")), i->expr), $$), - $5); - } - } + { $$ = new ExprSelect($1, *$3, $5); } | /* Backwards compatibility: because Nixpkgs has a rarely used function named ‘or’, allow stuff like ‘map or [...]’. */ expr_simple OR_KW @@ -542,37 +446,37 @@ binds | binds INHERIT attrs ';' { $$ = $1; foreach (AttrPath::iterator, i, *$3) { - if ($$->attrs.find(*i) != $$->attrs.end()) - dupAttr(*i, makeCurPos(@3, data), $$->attrs[*i].pos); + if ($$->attrs.find(i->symbol) != $$->attrs.end()) + dupAttr(i->symbol, makeCurPos(@3, data), $$->attrs[i->symbol].pos); Pos pos = makeCurPos(@3, data); - $$->attrs[*i] = ExprAttrs::AttrDef(new ExprVar(CUR_POS, *i), pos, true); + $$->attrs[i->symbol] = ExprAttrs::AttrDef(new ExprVar(CUR_POS, i->symbol), pos, true); } } | binds INHERIT '(' expr ')' attrs ';' { $$ = $1; /* !!! Should ensure sharing of the expression in $4. */ - foreach (vector::iterator, i, *$6) { - if ($$->attrs.find(*i) != $$->attrs.end()) - dupAttr(*i, makeCurPos(@6, data), $$->attrs[*i].pos); - $$->attrs[*i] = ExprAttrs::AttrDef(new ExprSelect($4, *i), makeCurPos(@6, data)); + foreach (AttrPath::iterator, i, *$6) { + if ($$->attrs.find(i->symbol) != $$->attrs.end()) + dupAttr(i->symbol, makeCurPos(@6, data), $$->attrs[i->symbol].pos); + $$->attrs[i->symbol] = ExprAttrs::AttrDef(new ExprSelect($4, i->symbol), makeCurPos(@6, data)); } } | { $$ = new ExprAttrs; } ; attrs - : attrs attr { $$ = $1; $1->push_back(data->symbols.create($2)); /* !!! dangerous */ } + : attrs attr { $$ = $1; $1->push_back(AttrName(data->symbols.create($2))); } | attrs string_attr { $$ = $1; ExprString *str = dynamic_cast($2); if (str) { - $$->push_back(str->s); + $$->push_back(AttrName(str->s)); delete str; } else throw ParseError(format("dynamic attributes not allowed in inherit at %1%") % makeCurPos(@2, data)); } - | { $$ = new vector; } + | { $$ = new AttrPath; } ; attrpath