Ast types first #7233
Ast types first #7233
Conversation
cristianoc
force-pushed
the
ast_types_first
branch
from
ed61afb to
1410016
Compare
Normalize function definitions to be of the form `(type a b c, x, y) => ...`, moving all the types first if they are not already.
cristianoc
force-pushed
the
ast_types_first
branch
from
1410016 to
4340995
Compare
| @@ -17,6 +17,11 @@ | |||
| - Editor: Fix issue where pipe completions would not trigger with generic type arguments. https://github.com/rescript-lang/rescript/pull/7231 | |||
| - Fix leftover assert false in code for `null != undefined`. https://github.com/rescript-lang/rescript/pull/7232 | |||
|
|
|||
| #### :house: Internal | |||
|
|
|||
| - AST cleanup: Prepare for ast async cleanup: Refactor code for "@res.async" payload handling and clean up handling of type and term parameters, so that now each `=>` in a function definition corresponds to a function. https://github.com/rescript-lang/rescript/pull/7223 | |||
There was a problem hiding this comment.
missing from previous PR
| @@ -1,14 +1,12 @@ | |||
| let f = (type t, xs: list<t>) => () | |||
| let f = @attr (type t, xs: list<t>) => () | |||
| let f = (type t, xs: list<t>) => (type s, ys: list<s>) => () | |||
| let f = @attr (type t, xs: list<t>) => @attr2 (type s, ys: list<s>) => () | |||
| let f = (type t s, xs: list<t>, ys: list<s>) => () | |||
There was a problem hiding this comment.
This was incorrect before. It's a single function.
But if one toggles between types and terms, there's no way to know if trailing types belong to the current function or the next one. Moving all types first removes this ambiguity.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'Syntax Benchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.05.
| Benchmark suite | Current: 0745025 | Previous: e1b7fb7 | Ratio |
|---|---|---|---|
Parse Napkinscript.res - time/run |
41.63185150666666 ms |
39.28006235333333 ms |
1.06 |
This comment was automatically generated by workflow using github-action-benchmark.
|
|
||
| type fundef_term_param = { | ||
| attrs: Parsetree.attributes; | ||
| p_label: Asttypes.arg_label; |
There was a problem hiding this comment.
Why are you now using the p_ prefix for label and pos?
There was a problem hiding this comment.
There are other types with the same labels, and it becomes ambiguous.
AST: always put type parameters first in function definitions.
Normalize function definitions to be of the form
(type a b c, x, y) => ..., moving all the types first if they are not already.