wroclaw-git-backups/nix
1
0
Fork 0
mirror of https://github.com/NixOS/nix synced 2025-06-24 18:01:16 +02:00
Code Activity

libexpr: make "or"-as-variable less bogus

Browse source 
The previous place where OR_KW was inserted into the grammar to allow
expressions like "map or [...]" led to a number of weird outcomes. By
moving it to expr_simple, expressions using "or" as a variable are now
parsed consistently with the rest of the language. Conflicts are
prevented by telling Bison that OR_KW has higher precedence than '.'.
This commit is contained in:
Ryan Hendrickson 2024-07-16 17:54:32 -04:00
parent aa1629ca35
commit 31431de634
7 changed files with 19 additions and 82 deletions
Download patch file Download diff file Expand all files Collapse all files

10
src/libexpr-tests/trivial.cc
View file

@ -295,7 +295,13 @@ namespace nix {
ASSERT_THAT(*b->value, IsIntEq(1));
}
TEST_F(TrivialExpressionTest, orCantBeUsed) {
ASSERT_THROW(eval("let or = 1; in or"), Error);
TEST_F(TrivialExpressionTest, orCanBeUsed) {
auto v = eval("let or = 1; in or");
ASSERT_THAT(v, IsIntEq(1));
}
TEST_F(TrivialExpressionTest, orHasCorrectPrecedence) {
auto v = eval("let inherit (builtins) add; or = 2; in add 1 or");
ASSERT_THAT(v, IsIntEq(3));
}
} /* namespace nix */

12
src/libexpr/include/nix/expr/nixexpr.hh
View file

@ -96,10 +96,6 @@ struct Expr
virtual void setName(Symbol name);
virtual void setDocComment(DocComment docComment) { };
virtual PosIdx getPos() const { return noPos; }
// These are temporary methods to be used only in parser.y
virtual void resetCursedOr() { };
virtual void warnIfCursedOr(const SymbolTable & symbols, const PosTable & positions) { };
};
#define COMMON_METHODS \
@ -361,16 +357,10 @@ struct ExprCall : Expr
Expr * fun;
std::vector<Expr *> args;
PosIdx pos;
std::optional<PosIdx> cursedOrEndPos; // used during parsing to warn about https://github.com/NixOS/nix/issues/11118
ExprCall(const PosIdx & pos, Expr * fun, std::vector<Expr *> && args)
: fun(fun), args(args), pos(pos), cursedOrEndPos({})
{ }
ExprCall(const PosIdx & pos, Expr * fun, std::vector<Expr *> && args, PosIdx && cursedOrEndPos)
: fun(fun), args(args), pos(pos), cursedOrEndPos(cursedOrEndPos)
: fun(fun), args(args), pos(pos)
{ }
PosIdx getPos() const override { return pos; }
virtual void resetCursedOr() override;
virtual void warnIfCursedOr(const SymbolTable & symbols, const PosTable & positions) override;
COMMON_METHODS
};

28
src/libexpr/nixexpr.cc
View file

@ -628,32 +628,4 @@ std::string DocComment::getInnerText(const PosTable & positions) const {
return docStr;
}
/* ‘Cursed or’ handling.
*
* In parser.y, every use of expr_select in a production must call one of the
* two below functions.
*
* To be removed by https://github.com/NixOS/nix/pull/11121
*/
void ExprCall::resetCursedOr()
{
cursedOrEndPos.reset();
}
void ExprCall::warnIfCursedOr(const SymbolTable & symbols, const PosTable & positions)
{
if (cursedOrEndPos.has_value()) {
std::ostringstream out;
out << "at " << positions[pos] << ": "
"This expression uses `or` as an identifier in a way that will change in a future Nix release.\n"
"Wrap this entire expression in parentheses to preserve its current meaning:\n"
" (" << positions[pos].getSnippetUpTo(positions[*cursedOrEndPos]).value_or("could not read expression") << ")\n"
"Give feedback at https://github.com/NixOS/nix/pull/11121";
warn(out.str());
}
}
}

27
src/libexpr/parser.y
View file

@ -177,6 +177,9 @@ static Expr * makeCall(PosIdx pos, Expr * fn, Expr * arg) {
%nonassoc '?'
%nonassoc NEGATE
%precedence '.'
%precedence OR_KW
%%
start: expr {
@ -269,28 +272,15 @@ expr_op
;
expr_app
: expr_app expr_select { $$ = makeCall(CUR_POS, $1, $2); $2->warnIfCursedOr(state->symbols, state->positions); }
| /* Once a ‘cursed or’ reaches this nonterminal, it is no longer cursed,
because the uncursed parse would also produce an expr_app. But we need
to remove the cursed status in order to prevent valid things like
`f (g or)` from triggering the warning. */
expr_select { $$ = $1; $$->resetCursedOr(); }
: expr_app expr_select { $$ = makeCall(CUR_POS, $1, $2); }
| expr_select
;
expr_select
: expr_simple '.' attrpath
{ $$ = new ExprSelect(CUR_POS, $1, std::move(*$3), nullptr); delete $3; }
| expr_simple '.' attrpath OR_KW expr_select
{ $$ = new ExprSelect(CUR_POS, $1, std::move(*$3), $5); delete $3; $5->warnIfCursedOr(state->symbols, state->positions); }
| /* Backwards compatibility: because Nixpkgs has a function named ‘or’,
allow stuff like ‘map or [...]’. This production is problematic (see
https://github.com/NixOS/nix/issues/11118) and will be refactored in the
future by treating `or` as a regular identifier. The refactor will (in
very rare cases, we think) change the meaning of expressions, so we mark
the ExprCall with data (establishing that it is a ‘cursed or’) that can
be used to emit a warning when an affected expression is parsed. */
expr_simple OR_KW
{ $$ = new ExprCall(CUR_POS, $1, {new ExprVar(CUR_POS, state->s.or_)}, state->positions.add(state->origin, @$.endOffset)); }
{ $$ = new ExprSelect(CUR_POS, $1, std::move(*$3), $5); delete $3; }
| expr_simple
;
@ -302,6 +292,9 @@ expr_simple
else
$$ = new ExprVar(CUR_POS, state->symbols.create($1));
}
| /* Backwards compatibility: because Nixpkgs has a function named ‘or’,
allow stuff like ‘map or [...]’. */
OR_KW { $$ = new ExprVar(CUR_POS, state->s.or_); }
| INT_LIT { $$ = new ExprInt($1); }
| FLOAT_LIT { $$ = new ExprFloat($1); }
| '"' string_parts '"' { $$ = $2; }
@ -493,7 +486,7 @@ string_attr
;
expr_list
: expr_list expr_select { $$ = $1; $1->elems.push_back($2); /* !!! dangerous */; $2->warnIfCursedOr(state->symbols, state->positions); }
: expr_list expr_select { $$ = $1; $1->elems.push_back($2); /* !!! dangerous */ }
| { $$ = new ExprList; }
;

12
tests/functional/lang/eval-okay-deprecate-cursed-or.err.exp
View file

@ -1,12 +0,0 @@
warning: at /pwd/lang/eval-okay-deprecate-cursed-or.nix:3:47: This expression uses `or` as an identifier in a way that will change in a future Nix release.
Wrap this entire expression in parentheses to preserve its current meaning:
((x: x) or)
Give feedback at https://github.com/NixOS/nix/pull/11121
warning: at /pwd/lang/eval-okay-deprecate-cursed-or.nix:4:39: This expression uses `or` as an identifier in a way that will change in a future Nix release.
Wrap this entire expression in parentheses to preserve its current meaning:
((x: x + 1) or)
Give feedback at https://github.com/NixOS/nix/pull/11121
warning: at /pwd/lang/eval-okay-deprecate-cursed-or.nix:5:44: This expression uses `or` as an identifier in a way that will change in a future Nix release.
Wrap this entire expression in parentheses to preserve its current meaning:
((x: x) or)
Give feedback at https://github.com/NixOS/nix/pull/11121

1
tests/functional/lang/eval-okay-deprecate-cursed-or.exp
View file

@ -1 +0,0 @@
0

11
tests/functional/lang/eval-okay-deprecate-cursed-or.nix
View file

@ -1,11 +0,0 @@
let
# These are cursed and should warn
cursed0 = builtins.length (let or = 1; in [ (x: x) or ]);
cursed1 = let or = 1; in (x: x * 2) (x: x + 1) or;
cursed2 = let or = 1; in { a = 2; }.a or (x: x) or;
# These are uses of `or` as an identifier that are not cursed
allowed0 = let or = (x: x); in map or [];
allowed1 = let f = (x: x); or = f; in f (f or);
in
0