Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libexpr: deprecate the bogus "or"-as-variable #11560

Merged
merged 1 commit into from
Oct 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions src/libexpr/nixexpr.cc
Original file line number Diff line number Diff line change
Expand Up @@ -663,4 +663,32 @@ 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());
}
}

}
12 changes: 11 additions & 1 deletion src/libexpr/nixexpr.hh
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,10 @@ 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 \
Expand Down Expand Up @@ -354,10 +358,16 @@ 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)
: 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)
{ }
PosIdx getPos() const override { return pos; }
virtual void resetCursedOr() override;
virtual void warnIfCursedOr(const SymbolTable & symbols, const PosTable & positions) override;
COMMON_METHODS
};

Expand Down
23 changes: 16 additions & 7 deletions src/libexpr/parser.y
Original file line number Diff line number Diff line change
Expand Up @@ -264,19 +264,28 @@ expr_op
;

expr_app
: expr_app expr_select { $$ = makeCall(CUR_POS, $1, $2); }
| expr_select
: 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_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; }
| /* Backwards compatibility: because Nixpkgs has a rarely used
function named ‘or’, allow stuff like ‘map or [...]’. */
{ $$ = 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_)}); }
{ $$ = new ExprCall(CUR_POS, $1, {new ExprVar(CUR_POS, state->s.or_)}, state->positions.add(state->origin, @$.endOffset)); }
| expr_simple
;

Expand Down Expand Up @@ -472,7 +481,7 @@ string_attr
;

expr_list
: expr_list expr_select { $$ = $1; $1->elems.push_back($2); /* !!! dangerous */ }
: expr_list expr_select { $$ = $1; $1->elems.push_back($2); /* !!! dangerous */; $2->warnIfCursedOr(state->symbols, state->positions); }
| { $$ = new ExprList; }
;

Expand Down
12 changes: 12 additions & 0 deletions tests/functional/lang/eval-okay-deprecate-cursed-or.err.exp
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
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 change: 1 addition & 0 deletions tests/functional/lang/eval-okay-deprecate-cursed-or.exp
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
0
11 changes: 11 additions & 0 deletions tests/functional/lang/eval-okay-deprecate-cursed-or.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
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
Loading