From 84ce7ac76feab6e9a5c074bd1b9550ae543d1db8 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 6 May 2010 16:46:48 +0000 Subject: [PATCH] * Store attribute positions in the AST and report duplicate attribute errors with position info. * For all positions, use the position of the first character of the first token, rather than the last character of the first token plus one. --- src/libexpr/eval.cc | 16 +++--------- src/libexpr/lexer.l | 13 ++++++---- src/libexpr/nixexpr.cc | 13 ++++++---- src/libexpr/nixexpr.hh | 10 ++++++-- src/libexpr/parser.y | 55 ++++++++++++++++++++---------------------- src/nix-env/nix-env.cc | 5 ++-- 6 files changed, 56 insertions(+), 56 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index b4f12c8a9c..cb124ab8ba 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -192,11 +192,6 @@ LocalNoInlineNoReturn(void throwAssertionError(const char * s, const Pos & pos)) throw AssertionError(format(s) % pos); } -LocalNoInline(void addErrorPrefix(Error & e, const char * s)) -{ - e.addPrefix(s); -} - LocalNoInline(void addErrorPrefix(Error & e, const char * s, const string & s2)) { e.addPrefix(format(s) % s2); @@ -207,11 +202,6 @@ LocalNoInline(void addErrorPrefix(Error & e, const char * s, const Pos & pos)) e.addPrefix(format(s) % pos); } -LocalNoInline(void addErrorPrefix(Error & e, const char * s, const string & s2, const string & s3)) -{ - e.addPrefix(format(s) % s2 % s3); -} - void mkString(Value & v, const char * s) { @@ -426,7 +416,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v) foreach (Attrs::iterator, i, attrs) { Value & v2 = (*v.attrs)[i->first]; mkCopy(v2, env2.values[displ]); - mkThunk(env2.values[displ++], env2, i->second); + mkThunk(env2.values[displ++], env2, i->second.first); } /* The inherited attributes, on the other hand, are @@ -443,7 +433,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v) else { foreach (Attrs::iterator, i, attrs) { Value & v2 = (*v.attrs)[i->first]; - mkThunk(v2, env, i->second); + mkThunk(v2, env, i->second.first); } foreach (list::iterator, i, inherited) { @@ -466,7 +456,7 @@ void ExprLet::eval(EvalState & state, Env & env, Value & v) /* The recursive attributes are evaluated in the new environment. */ foreach (ExprAttrs::Attrs::iterator, i, attrs->attrs) - mkThunk(env2.values[displ++], env2, i->second); + mkThunk(env2.values[displ++], env2, i->second.first); /* The inherited attributes, on the other hand, are evaluated in the original environment. */ diff --git a/src/libexpr/lexer.l b/src/libexpr/lexer.l index 00de57a7ba..f29f9b6843 100644 --- a/src/libexpr/lexer.l +++ b/src/libexpr/lexer.l @@ -19,13 +19,16 @@ namespace nix { static void initLoc(YYLTYPE * loc) { - loc->first_line = 1; - loc->first_column = 1; + loc->first_line = loc->last_line = 1; + loc->first_column = loc->last_column = 1; } static void adjustLoc(YYLTYPE * loc, const char * s, size_t len) { + loc->first_line = loc->last_line; + loc->first_column = loc->last_column; + while (len--) { switch (*s++) { case '\r': @@ -33,11 +36,11 @@ static void adjustLoc(YYLTYPE * loc, const char * s, size_t len) s++; /* fall through */ case '\n': - ++loc->first_line; - loc->first_column = 1; + ++loc->last_line; + loc->last_column = 1; break; default: - ++loc->first_column; + ++loc->last_column; } } } diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index a9c83108e9..af0632a94d 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -58,7 +58,7 @@ void ExprAttrs::show(std::ostream & str) foreach (list::iterator, i, inherited) str << "inherit " << i->name << "; "; foreach (Attrs::iterator, i, attrs) - str << i->first << " = " << *i->second << "; "; + str << i->first << " = " << *i->second.first << "; "; str << "}"; } @@ -94,7 +94,7 @@ void ExprLet::show(std::ostream & str) foreach (list::iterator, i, attrs->inherited) str << "inherit " << i->name << "; "; foreach (ExprAttrs::Attrs::iterator, i, attrs->attrs) - str << i->first << " = " << *i->second << "; "; + str << i->first << " = " << *i->second.first << "; "; str << "in " << *body; } @@ -138,6 +138,9 @@ std::ostream & operator << (std::ostream & str, const Pos & pos) } +Pos noPos; + + /* Computing levels/displacements for variables. */ void Expr::bindVars(const StaticEnv & env) @@ -218,12 +221,12 @@ void ExprAttrs::bindVars(const StaticEnv & env) } foreach (ExprAttrs::Attrs::iterator, i, attrs) - i->second->bindVars(newEnv); + i->second.first->bindVars(newEnv); } else { foreach (ExprAttrs::Attrs::iterator, i, attrs) - i->second->bindVars(env); + i->second.first->bindVars(env); foreach (list::iterator, i, inherited) i->bind(env); @@ -270,7 +273,7 @@ void ExprLet::bindVars(const StaticEnv & env) } foreach (ExprAttrs::Attrs::iterator, i, attrs->attrs) - i->second->bindVars(newEnv); + i->second.first->bindVars(newEnv); body->bindVars(newEnv); } diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index 2d328d382c..36cb4e53cd 100644 --- a/src/libexpr/nixexpr.hh +++ b/src/libexpr/nixexpr.hh @@ -23,8 +23,13 @@ struct Pos { string file; unsigned int line, column; + Pos() : line(0), column(0) { }; + Pos(const string & file, unsigned int line, unsigned int column) + : file(file), line(line), column(column) { }; }; +extern Pos noPos; + std::ostream & operator << (std::ostream & str, const Pos & pos); @@ -125,10 +130,11 @@ struct ExprOpHasAttr : Expr struct ExprAttrs : Expr { bool recursive; - typedef std::map Attrs; + typedef std::pair Attr; + typedef std::map Attrs; Attrs attrs; list inherited; - set attrNames; // used during parsing + std::map attrNames; // used during parsing ExprAttrs() : recursive(false) { }; COMMON_METHODS }; diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 66da769402..99980240f8 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -61,18 +61,18 @@ static string showAttrPath(const vector & attrPath) } -static void dupAttr(const vector & attrPath, const Pos & pos) +static void dupAttr(const vector & attrPath, const Pos & pos, const Pos & prevPos) { - throw ParseError(format("attribute `%1%' at %2% already defined at ") - % showAttrPath(attrPath) % pos); + throw ParseError(format("attribute `%1%' at %2% already defined at %3%") + % showAttrPath(attrPath) % pos % prevPos); } -static void dupAttr(Symbol attr, const Pos & pos) +static void dupAttr(Symbol attr, const Pos & pos, const Pos & prevPos) { vector attrPath; attrPath.push_back(attr); - throw ParseError(format("attribute `%1%' at %2% already defined at ") - % showAttrPath(attrPath) % pos); + throw ParseError(format("attribute `%1%' at %2% already defined at %3%") + % showAttrPath(attrPath) % pos % prevPos); } @@ -82,19 +82,20 @@ static void addAttr(ExprAttrs * attrs, const vector & attrPath, unsigned int n = 0; foreach (vector::const_iterator, i, attrPath) { n++; - if (attrs->attrs[*i]) { - ExprAttrs * attrs2 = dynamic_cast(attrs->attrs[*i]); - if (!attrs2 || n == attrPath.size()) dupAttr(attrPath, pos); + ExprAttrs::Attrs::iterator j = attrs->attrs.find(*i); + if (j != attrs->attrs.end()) { + ExprAttrs * attrs2 = dynamic_cast(j->second.first); + if (!attrs2 || n == attrPath.size()) dupAttr(attrPath, pos, j->second.second); attrs = attrs2; } else { if (attrs->attrNames.find(*i) != attrs->attrNames.end()) - dupAttr(attrPath, pos); - attrs->attrNames.insert(*i); + dupAttr(attrPath, pos, attrs->attrNames[*i]); + attrs->attrNames[*i] = pos; if (n == attrPath.size()) - attrs->attrs[*i] = e; + attrs->attrs[*i] = ExprAttrs::Attr(e, pos); else { ExprAttrs * nested = new ExprAttrs; - attrs->attrs[*i] = nested; + attrs->attrs[*i] = ExprAttrs::Attr(nested, pos); attrs = nested; } } @@ -205,16 +206,12 @@ void backToString(yyscan_t scanner); void backToIndString(yyscan_t scanner); -static Pos makeCurPos(YYLTYPE * loc, ParseData * data) +static Pos makeCurPos(const YYLTYPE & loc, ParseData * data) { - Pos pos; - pos.file = data->path; - pos.line = loc->first_line; - pos.column = loc->first_column; - return pos; + return Pos(data->path, loc.first_line, loc.first_column); } -#define CUR_POS makeCurPos(yylocp, data) +#define CUR_POS makeCurPos(*yylocp, data) } @@ -223,7 +220,7 @@ static Pos makeCurPos(YYLTYPE * loc, ParseData * data) void yyerror(YYLTYPE * loc, yyscan_t scanner, ParseData * data, const char * error) { data->error = (format("%1%, at %2%") - % error % makeCurPos(loc, data)).str(); + % error % makeCurPos(*loc, data)).str(); } @@ -374,14 +371,14 @@ ind_string_parts ; binds - : binds attrpath '=' expr ';' { $$ = $1; addAttr($$, *$2, $4, CUR_POS); } + : binds attrpath '=' expr ';' { $$ = $1; addAttr($$, *$2, $4, makeCurPos(@2, data)); } | binds INHERIT ids ';' { $$ = $1; foreach (vector::iterator, i, *$3) { if ($$->attrNames.find(*i) != $$->attrNames.end()) - dupAttr(*i, CUR_POS); + dupAttr(*i, makeCurPos(@3, data), $$->attrNames[*i]); $$->inherited.push_back(*i); - $$->attrNames.insert(*i); + $$->attrNames[*i] = makeCurPos(@3, data); } } | binds INHERIT '(' expr ')' ids ';' @@ -389,11 +386,11 @@ binds /* !!! Should ensure sharing of the expression in $4. */ foreach (vector::iterator, i, *$6) { if ($$->attrNames.find(*i) != $$->attrNames.end()) - dupAttr(*i, CUR_POS); - $$->attrs[*i] = new ExprSelect($4, *i); - $$->attrNames.insert(*i); - } - } + dupAttr(*i, makeCurPos(@6, data), $$->attrNames[*i]); + $$->attrs[*i] = ExprAttrs::Attr(new ExprSelect($4, *i), makeCurPos(@6, data)); + $$->attrNames[*i] = makeCurPos(@6, data); + }} + | { $$ = new ExprAttrs; } ; diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc index 4a9df454d4..12a256a091 100644 --- a/src/nix-env/nix-env.cc +++ b/src/nix-env/nix-env.cc @@ -132,7 +132,7 @@ static void getAllExprs(EvalState & state, if (hasSuffix(attrName, ".nix")) attrName = string(attrName, 0, attrName.size() - 4); attrs.attrs[state.symbols.create(attrName)] = - parseExprFromFile(state, absPath(path2)); + ExprAttrs::Attr(parseExprFromFile(state, absPath(path2)), noPos); } else /* `path2' is a directory (with no default.nix in it); @@ -153,7 +153,8 @@ static Expr * loadSourceExpr(EvalState & state, const Path & path) for a user to have a ~/.nix-defexpr directory that includes some system-wide directory). */ ExprAttrs * attrs = new ExprAttrs; - attrs->attrs[state.symbols.create("_combineChannels")] = new ExprList(); + attrs->attrs[state.symbols.create("_combineChannels")] = + ExprAttrs::Attr(new ExprList(), noPos); getAllExprs(state, path, *attrs); return attrs; }