Move Option stdlib optimizations into typed pipeline #7921
Conversation
| return category.name; | ||
| let __res_option_opt; | ||
| if (incidentId !== undefined) { | ||
| let incidentId$1 = Primitive_option.valFromOption(incidentId); |
There was a problem hiding this comment.
Output looks worse than the Parsetree version.
These Primitive_option.valFromOption should not be needed.
| function testPrimitive() { | ||
| console.log(42); | ||
| let value = 42; | ||
| console.log(value); |
There was a problem hiding this comment.
Why did we lose the optimization here?
|
As we are doing the transformation on the typed level now, shouldn't we add some type checking tests, too? |
| ```rescript | ||
| Js.String.charCodeAt(0, `😺`) == 0xd83d->Belt.Int.toFloat | ||
| Js.String.codePointAt(0, `😺`) == Some(0x1f63a) |
There was a problem hiding this comment.
Are you removing this because it's not really an example for charCodeAt or because the test failed? (Actually this should be correct / the test should work.)
There was a problem hiding this comment.
Don't know it failed before already. Not sure.
There was a problem hiding this comment.
You may need to remove tests/docstring_tests/generated_mocha_test.res and re-run the tests.
cristianoc
force-pushed
the
option-stdlib-typed-opt
branch
from
9202879 to
fd1a69d
Compare
| let category = categoryId !== undefined ? Belt_MapString.get(categories, categoryId) : undefined; | ||
| if (category !== undefined) { | ||
| return category.name; | ||
| let __res_option_opt = incidentId !== undefined ? Belt_MapString.get(incidents, incidentId) : undefined; |
There was a problem hiding this comment.
Now the output is the same except for worse variable naming. 🙂
Can we produce the identical output?
cknitt
force-pushed
the
options-stdlib-opt
branch
from
f1692ff to
4496782
Compare
cknitt
force-pushed
the
option-stdlib-typed-opt
branch
from
97812a7 to
5ac5fc9
Compare
cknitt
force-pushed
the
options-stdlib-opt
branch
from
4496782 to
50dbc3c
Compare
# Conflicts: # packages/@rescript/runtime/Stdlib_DataView.resi
cknitt
force-pushed
the
option-stdlib-typed-opt
branch
from
5ac5fc9 to
5171806
Compare
|
Rebased after the Docstring test fixes |
Summary
Testing