Skip to content

Commit

Permalink
Add support for extend primop
Browse files Browse the repository at this point in the history
Nickel has change some primops to contain a forward slash `/`, which
wasn't supported by the parser (they would be parsed as modulo instead).

This commit extend the `builtin` rule to accepts a larger class of valid
builtin names, including those new ones, and hopefully potential future
ones as well.
  • Loading branch information
yannham committed Jun 3, 2024
1 parent c23aa86 commit 795a119
Show file tree
Hide file tree
Showing 6 changed files with 1,704 additions and 1,621 deletions.
3 changes: 1 addition & 2 deletions corpus/basic.txt
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,7 @@ identifier (builtin function)
(applicative
(record_operand
(atom
(builtin
(ident))))))))
(builtin)))))))

================================================================================
number int
Expand Down
59 changes: 59 additions & 0 deletions corpus/primop.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
================================================================================
Normal primop
================================================================================

%plus% 1 2

--------------------------------------------------------------------------------

(term
(uni_term
(infix_expr
(applicative
(applicative
(applicative
(record_operand
(atom
(builtin))))
(record_operand
(atom
(num_literal))))
(record_operand
(atom
(num_literal)))))))

================================================================================
Slashed primop
================================================================================

%array/length% []

--------------------------------------------------------------------------------

(term
(uni_term
(infix_expr
(applicative
(applicative
(record_operand
(atom
(builtin))))
(record_operand
(atom))))))

================================================================================
Extended primop (currently inexistent)
================================================================================

%__some/primop.foo.bar'%

--------------------------------------------------------------------------------

(term
(uni_term
(infix_expr
(applicative
(record_operand
(atom
(builtin)))))))

10 changes: 8 additions & 2 deletions grammar.js
Original file line number Diff line number Diff line change
Expand Up @@ -463,9 +463,15 @@ module.exports = grammar({
//grammar.lalrpop: 509
// Different from lalrpop grammar, we parse all possible builtins, not just
// the defined ones.
builtin: $ => seq(
builtin: _ => seq(
"%",
$.ident,
// We are a bit more liberal with what can go in a builtin function than
// for identifiers, because builtins are properly delimited by `%`.
// Upstream Nickel added `/` as a valid character already, so there's a
// precendent for extensions (although it's not very likely), so we try to
// be a bit future-proof. We just make sure the builtin starts with either
// a letter or an undescore, to ensure reasonable names.
/_*[a-zA-Z][a-zA-Z0-9./_'-]*/,
"%",
),

Expand Down
4 changes: 2 additions & 2 deletions src/grammar.json
Original file line number Diff line number Diff line change
Expand Up @@ -1909,8 +1909,8 @@
"value": "%"
},
{
"type": "SYMBOL",
"name": "ident"
"type": "PATTERN",
"value": "_*[a-zA-Z][a-zA-Z0-9./_'-]*"
},
{
"type": "STRING",
Expand Down
12 changes: 1 addition & 11 deletions src/node-types.json
Original file line number Diff line number Diff line change
Expand Up @@ -239,17 +239,7 @@
{
"type": "builtin",
"named": true,
"fields": {},
"children": {
"multiple": false,
"required": true,
"types": [
{
"type": "ident",
"named": true
}
]
}
"fields": {}
},
{
"type": "chunk_expr",
Expand Down
Loading

0 comments on commit 795a119

Please sign in to comment.