diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index 063ff07537b..dbc74faf9a7 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -663,32 +663,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()); - } -} - } diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index bdf4e214a59..7868834f195 100644 --- a/src/libexpr/nixexpr.hh +++ b/src/libexpr/nixexpr.hh @@ -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 \ @@ -358,16 +354,10 @@ struct ExprCall : Expr Expr * fun; std::vector args; PosIdx pos; - std::optional cursedOrEndPos; // used during parsing to warn about https://github.com/NixOS/nix/issues/11118 ExprCall(const PosIdx & pos, Expr * fun, std::vector && args) - : fun(fun), args(args), pos(pos), cursedOrEndPos({}) - { } - ExprCall(const PosIdx & pos, Expr * fun, std::vector && 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 }; diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 944c7b1af31..eee0c78ceda 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -177,6 +177,9 @@ static Expr * makeCall(PosIdx pos, Expr * fn, Expr * arg) { %nonassoc '?' %nonassoc NEGATE +%precedence '.' +%precedence OR_KW + %% start: expr { state->result = $1; }; @@ -264,28 +267,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 ; @@ -297,6 +287,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; } @@ -481,7 +474,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; } ; diff --git a/tests/functional/lang/eval-okay-deprecate-cursed-or.err.exp b/tests/functional/lang/eval-okay-deprecate-cursed-or.err.exp deleted file mode 100644 index 4a656827a25..00000000000 --- a/tests/functional/lang/eval-okay-deprecate-cursed-or.err.exp +++ /dev/null @@ -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 diff --git a/tests/functional/lang/eval-okay-deprecate-cursed-or.exp b/tests/functional/lang/eval-okay-deprecate-cursed-or.exp deleted file mode 100644 index 573541ac970..00000000000 --- a/tests/functional/lang/eval-okay-deprecate-cursed-or.exp +++ /dev/null @@ -1 +0,0 @@ -0 diff --git a/tests/functional/lang/eval-okay-deprecate-cursed-or.nix b/tests/functional/lang/eval-okay-deprecate-cursed-or.nix deleted file mode 100644 index a4f9e747f89..00000000000 --- a/tests/functional/lang/eval-okay-deprecate-cursed-or.nix +++ /dev/null @@ -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 diff --git a/tests/unit/libexpr/trivial.cc b/tests/unit/libexpr/trivial.cc index e455a571b18..aa6bf374f67 100644 --- a/tests/unit/libexpr/trivial.cc +++ b/tests/unit/libexpr/trivial.cc @@ -244,7 +244,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 */