From cd49fe4f9b338242e1e404fd4dbb0a3ebc1c3a12 Mon Sep 17 00:00:00 2001 From: Shea Levy Date: Tue, 31 Dec 2013 23:56:26 +0000 Subject: [PATCH] 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 cd3edecaa..3d8ee9934 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 a7ce58c4d..9f0bc2630 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 92c2ca8dc..bc6993477 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 3bee3b010..28972cf72 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