Skip to content

Commit

Permalink
libexpr: make "or"-as-variable less bogus
Browse files Browse the repository at this point in the history
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 '.'.
  • Loading branch information
rhendric committed Jul 16, 2024
1 parent 9300f85 commit e30ae3f
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 6 deletions.
10 changes: 6 additions & 4 deletions src/libexpr/parser.y
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,9 @@ static void setDocPosition(const LexerState & lexerState, ExprLambda * lambda, P
%nonassoc '?'
%nonassoc NEGATE

%precedence '.'
%precedence OR_KW

%%

start: expr { state->result = $1; };
Expand Down Expand Up @@ -248,10 +251,6 @@ expr_select
{ $$ = 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 [...]’. */
expr_simple OR_KW
{ $$ = new ExprCall(CUR_POS, $1, {new ExprVar(CUR_POS, state->s.or_)}); }
| expr_simple
;

Expand All @@ -263,6 +262,9 @@ expr_simple
else
$$ = new ExprVar(CUR_POS, state->symbols.create($1));
}
| /* Backwards compatibility: because Nixpkgs has a rarely used
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; }
Expand Down
10 changes: 8 additions & 2 deletions tests/unit/libexpr/trivial.cc
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,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->value, IsIntEq(1));
}

TEST_F(TrivialExpressionTest, orHasCorrectPrecedence) {
auto v = eval("let inherit (builtins) add; or = 2; in add 1 or");
ASSERT_THAT(*v->value, IsIntEq(3));
}
} /* namespace nix */

0 comments on commit e30ae3f

Please sign in to comment.