report inherit attr errors at the duplicate name

previously we reported the error at the beginning of the binding
block (for plain inherits) or the beginning of the attr list (for
inherit-from), effectively hiding where exactly the error happened.

this also carries over to runtime positions of attributes in sets as
reported by unsafeGetAttrPos. we're not worried about this changing
observable eval behavior because it *is* marked unsafe, and the new
behavior is much more useful.

(cherry picked from commit 1edd6fada53553b89847ac3981ac28025857ca02)
Change-Id: I2f50eb9f3dc3977db4eb3e3da96f1cb37ccd5174
This commit is contained in:
eldritch horrors 2024-01-29 06:19:23 +01:00
parent d826427f02
commit 9cf92c012d
9 changed files with 43 additions and 23 deletions

View file

@ -0,0 +1,6 @@
---
synopsis: fix duplicate attribute error positions for `inherit`
prs: 9874
---
When an inherit caused a duplicate attribute error the position of the error was not reported correctly, placing the error with the inherit itself or at the start of the bindings block instead of the offending attribute name.

View file

@ -85,6 +85,7 @@ void yyerror(YYLTYPE * loc, yyscan_t scanner, ParserState * state, const char *
nix::StringToken uri; nix::StringToken uri;
nix::StringToken str; nix::StringToken str;
std::vector<nix::AttrName> * attrNames; std::vector<nix::AttrName> * attrNames;
std::vector<std::pair<nix::AttrName, nix::PosIdx>> * inheritAttrs;
std::vector<std::pair<nix::PosIdx, nix::Expr *>> * string_parts; std::vector<std::pair<nix::PosIdx, nix::Expr *>> * string_parts;
std::vector<std::pair<nix::PosIdx, std::variant<nix::Expr *, nix::StringToken>>> * ind_string_parts; std::vector<std::pair<nix::PosIdx, std::variant<nix::Expr *, nix::StringToken>>> * ind_string_parts;
} }
@ -95,7 +96,8 @@ void yyerror(YYLTYPE * loc, yyscan_t scanner, ParserState * state, const char *
%type <attrs> binds %type <attrs> binds
%type <formals> formals %type <formals> formals
%type <formal> formal %type <formal> formal
%type <attrNames> attrs attrpath %type <attrNames> attrpath
%type <inheritAttrs> attrs
%type <string_parts> string_parts_interpolated %type <string_parts> string_parts_interpolated
%type <ind_string_parts> ind_string_parts %type <ind_string_parts> ind_string_parts
%type <e> path_start string_parts string_attr %type <e> path_start string_parts string_attr
@ -307,13 +309,12 @@ binds
: binds attrpath '=' expr ';' { $$ = $1; state->addAttr($$, std::move(*$2), $4, state->at(@2)); delete $2; } : binds attrpath '=' expr ';' { $$ = $1; state->addAttr($$, std::move(*$2), $4, state->at(@2)); delete $2; }
| binds INHERIT attrs ';' | binds INHERIT attrs ';'
{ $$ = $1; { $$ = $1;
for (auto & i : *$3) { for (auto & [i, iPos] : *$3) {
if ($$->attrs.find(i.symbol) != $$->attrs.end()) if ($$->attrs.find(i.symbol) != $$->attrs.end())
state->dupAttr(i.symbol, state->at(@3), $$->attrs[i.symbol].pos); state->dupAttr(i.symbol, iPos, $$->attrs[i.symbol].pos);
auto pos = state->at(@3);
$$->attrs.emplace( $$->attrs.emplace(
i.symbol, i.symbol,
ExprAttrs::AttrDef(new ExprVar(CUR_POS, i.symbol), pos, ExprAttrs::AttrDef::Kind::Inherited)); ExprAttrs::AttrDef(new ExprVar(iPos, i.symbol), iPos, ExprAttrs::AttrDef::Kind::Inherited));
} }
delete $3; delete $3;
} }
@ -323,14 +324,14 @@ binds
$$->inheritFromExprs = std::make_unique<std::vector<Expr *>>(); $$->inheritFromExprs = std::make_unique<std::vector<Expr *>>();
$$->inheritFromExprs->push_back($4); $$->inheritFromExprs->push_back($4);
auto from = new nix::ExprInheritFrom(state->at(@4), $$->inheritFromExprs->size() - 1); auto from = new nix::ExprInheritFrom(state->at(@4), $$->inheritFromExprs->size() - 1);
for (auto & i : *$6) { for (auto & [i, iPos] : *$6) {
if ($$->attrs.find(i.symbol) != $$->attrs.end()) if ($$->attrs.find(i.symbol) != $$->attrs.end())
state->dupAttr(i.symbol, state->at(@6), $$->attrs[i.symbol].pos); state->dupAttr(i.symbol, iPos, $$->attrs[i.symbol].pos);
$$->attrs.emplace( $$->attrs.emplace(
i.symbol, i.symbol,
ExprAttrs::AttrDef( ExprAttrs::AttrDef(
new ExprSelect(CUR_POS, from, i.symbol), new ExprSelect(iPos, from, i.symbol),
state->at(@6), iPos,
ExprAttrs::AttrDef::Kind::InheritedFrom)); ExprAttrs::AttrDef::Kind::InheritedFrom));
} }
delete $6; delete $6;
@ -339,12 +340,12 @@ binds
; ;
attrs attrs
: attrs attr { $$ = $1; $1->push_back(AttrName(state->symbols.create($2))); } : attrs attr { $$ = $1; $1->emplace_back(AttrName(state->symbols.create($2)), state->at(@2)); }
| attrs string_attr | attrs string_attr
{ $$ = $1; { $$ = $1;
ExprString * str = dynamic_cast<ExprString *>($2); ExprString * str = dynamic_cast<ExprString *>($2);
if (str) { if (str) {
$$->push_back(AttrName(state->symbols.create(str->s))); $$->emplace_back(AttrName(state->symbols.create(str->s)), state->at(@2));
delete str; delete str;
} else } else
throw ParseError({ throw ParseError({
@ -352,7 +353,7 @@ attrs
.pos = state->positions[state->at(@2)] .pos = state->positions[state->at(@2)]
}); });
} }
| { $$ = new AttrPath; } | { $$ = new std::vector<std::pair<AttrName, PosIdx>>; }
; ;
attrpath attrpath

View file

@ -1,4 +1,4 @@
error: attribute 'a' already defined at /pwd/lang/eval-fail-dupAttr-inherit.nix:1:15 error: attribute 'a' already defined at /pwd/lang/eval-fail-dupAttr-inherit.nix:1:16
at /pwd/lang/eval-fail-dupAttr-inherit.nix:1:19: at /pwd/lang/eval-fail-dupAttr-inherit.nix:1:19:
1| { inherit ({}) a; a.b = 1; } 1| { inherit ({}) a; a.b = 1; }
| ^ | ^

View file

@ -0,0 +1 @@
[ { column = 17; file = "/pwd/lang/eval-okay-inherit-attr-pos.nix"; line = 4; } { column = 19; file = "/pwd/lang/eval-okay-inherit-attr-pos.nix"; line = 4; } { column = 21; file = "/pwd/lang/eval-okay-inherit-attr-pos.nix"; line = 5; } { column = 23; file = "/pwd/lang/eval-okay-inherit-attr-pos.nix"; line = 5; } ]

View file

@ -0,0 +1,12 @@
let
d = 0;
x = 1;
y = { inherit d x; };
z = { inherit (y) d x; };
in
[
(builtins.unsafeGetAttrPos "d" y)
(builtins.unsafeGetAttrPos "x" y)
(builtins.unsafeGetAttrPos "d" z)
(builtins.unsafeGetAttrPos "x" z)
]

View file

@ -1,5 +1,5 @@
error: attribute 'x' already defined at «stdin»:9:5 error: attribute 'x' already defined at «stdin»:9:5
at «stdin»:10:17: at «stdin»:10:18:
9| x = 789; 9| x = 789;
10| inherit (as) x; 10| inherit (as) x;
| ^ | ^

View file

@ -1,5 +1,5 @@
error: attribute 'x' already defined at «stdin»:9:5 error: attribute 'x' already defined at «stdin»:9:5
at «stdin»:10:17: at «stdin»:10:18:
9| x = 789; 9| x = 789;
10| inherit (as) x; 10| inherit (as) x;
| ^ | ^

View file

@ -1,5 +1,5 @@
error: attribute 'x' already defined at «stdin»:6:12 error: attribute 'x' already defined at «stdin»:6:13
at «stdin»:7:12: at «stdin»:7:13:
6| inherit x; 6| inherit x;
7| inherit x; 7| inherit x;
| ^ | ^

View file

@ -1,6 +1,6 @@
error: undefined variable 'gcc' error: undefined variable 'gcc'
at «stdin»:8:12: at «stdin»:9:13:
7|
8| body = ({ 8| body = ({
| ^
9| inherit gcc; 9| inherit gcc;
| ^
10| }).gcc;