Switch on uncurried #6864

cristianoc · 2024-07-08T13:06:58Z

Build the compiler in uncurried mode, following on from #6764

This PR only changes the way the compiler is built, so it's easier to take a look at any differences in the generated code.

No Curry runtime is used in the library/tests now.

cristianoc · 2024-07-09T08:09:40Z

As one datapoint, the rescript-vscode tests give identical results on this PR w.r.t. master.

cknitt · 2024-07-09T18:00:34Z

jscomp/build_tests/super_errors/expected/arity_mismatch.res.expected

  3 [2m│[0m 

-  This uncurried function has type (. ~f: 'a => 'a, unit) => int
+  This uncurried function has type (~f: 'a => 'a, unit) => int


In a separate PR, we should change "This uncurried function has ..." to "This function has ...".

cknitt · 2024-07-09T18:09:24Z

jscomp/gentype_tests/typescript-react-example/src/Records.res.js

-let getOpt = Belt_Option.mapWithDefault;
+function getOpt(opt, $$default, foo) {
+  return Belt_Option.mapWithDefault(opt, $$default, foo);
+}


Looking at this and at the ImmutableArray example, it seems that some optimization was lost here.

This is the ReScript code:

let getOpt = (opt, default, foo) => opt->Option.mapWithDefault(default, foo)

cknitt · 2024-07-09T18:10:38Z

jscomp/test/a_scope_bug.js

-let even = odd;
+function even(y) {
+  return odd(y);
+}


cknitt · 2024-07-09T18:12:57Z

jscomp/test/bdd.js

  let h = function (i, j) {
    if (i === j) {
-      return mkNode("Zero", i, "One");
+      return mkVar(i);


Again inlining lost

cknitt · 2024-07-09T18:16:43Z

jscomp/test/bs_poly_map_test.js

  }));
-  return Belt_Set.fromArray(Belt_MapDict.keysToArray(m.data), Icmp);
+  let x = Belt_MapDict.keysToArray(m.data);
+  return Belt_Set.fromArray(x, Icmp);


A temporary variable x is created here now which wasn't the case before.

cknitt · 2024-07-09T18:17:20Z

jscomp/test/bs_poly_mutable_set_test.js

 b("File \"bs_poly_mutable_set_test.res\", line 95, characters 4-11", Belt_MutableSet.subset(bb, v));

-b("File \"bs_poly_mutable_set_test.res\", line 96, characters 4-11", Belt_MutableSet.isEmpty(Belt_MutableSet.intersect(aa, bb)));
+let d$1 = Belt_MutableSet.intersect(aa, bb);


cristianoc · 2024-07-14T06:47:02Z

@cknitt @zth I have now run out of things to test this. Let me know if you think of anything else to check.

cknitt · 2024-07-14T08:10:53Z

To test, I tried to compile a large project of ours.
This failed because Js_typed_array was removed in a previous PR, but rescript-webapi depends on it.

zth · 2024-07-14T08:18:14Z

To test, I tried to compile a large project of ours. This failed because Js_typed_array was removed in a previous PR, but rescript-webapi depends on it.

I assume it depends on the types only? Should be easy to add back the types.

cknitt · 2024-07-14T08:39:01Z

Yes, patching it to replace Js.typed_array with Js_typed_array2 gets things to build again.

However, the result doesn't run.

The compiler now emits

          ...
          title: JsxRuntime.jsx((function (prim) {
            return ReactIntl.FormattedMessage;
          }), {
          ...

instead of

          ...
          title: JsxRuntime.jsx(ReactIntl.FormattedMessage, {
          ...

Would have to investigate where this was introduced, can look into it tomorrow.

cristianoc · 2024-07-15T03:54:01Z

My guess is that either some external is wrong, or there's a compiler bug on the arity of the external.
And that the same isso exists on master when compiling in uncurried mode, but not before the PR that handles arity in externals.

cknitt · 2024-07-15T06:10:50Z

@cristianoc You guessed correctly! I created #6880 for this.

cristianoc · 2024-07-16T08:17:57Z

@cristianoc You guessed correctly! I created #6880 for this.

Let me know what other issues are left now.

zth · 2024-07-16T11:02:06Z

@cknitt could you add back the types needed in a separate PR perhaps?

Add a `-curried` option, and give an error whenever neither `-curried` not `-uncurried` are used. Then change the build and test files to always specify curried/uncurried.

cknitt · 2024-07-17T09:30:11Z

@cknitt could you add back the types needed in a separate PR perhaps?

Yes, will do.

cknitt · 2024-07-17T13:45:17Z

Let me know what other issues are left now.

No other issues left from my point of view.

cknitt · 2024-07-18T12:07:29Z

@cknitt could you add back the types needed in a separate PR perhaps?

#6891

cristianoc requested review from bloodyowl, cknitt and zth July 8, 2024 13:18

cristianoc force-pushed the audit-curried branch from 3e057f5 to 0a94cf7 Compare July 8, 2024 15:36

cristianoc force-pushed the switch-on-uncurried branch 2 times, most recently from 382d49b to e6309ba Compare July 8, 2024 15:38

cknitt reviewed Jul 9, 2024

View reviewed changes

cristianoc force-pushed the audit-curried branch from 0a94cf7 to 3ec26e2 Compare July 10, 2024 11:04

cristianoc force-pushed the switch-on-uncurried branch from e6309ba to a8ee1d5 Compare July 10, 2024 11:05

cristianoc force-pushed the audit-curried branch from 3ec26e2 to 970e6fc Compare July 12, 2024 09:42

cristianoc force-pushed the switch-on-uncurried branch from a8ee1d5 to c21f7d0 Compare July 12, 2024 09:43

cristianoc force-pushed the audit-curried branch from 970e6fc to be5eec8 Compare July 12, 2024 13:33

cristianoc force-pushed the switch-on-uncurried branch from c21f7d0 to 62f162e Compare July 12, 2024 13:34

cristianoc force-pushed the audit-curried branch from be5eec8 to 71f0b69 Compare July 13, 2024 12:41

cristianoc force-pushed the switch-on-uncurried branch from 62f162e to 94c4ccf Compare July 13, 2024 12:42

cristianoc force-pushed the audit-curried branch from 71f0b69 to a6018e2 Compare July 13, 2024 12:47

cristianoc force-pushed the switch-on-uncurried branch from 94c4ccf to bbf66b6 Compare July 13, 2024 12:48

cristianoc force-pushed the audit-curried branch 2 times, most recently from e418d93 to c57264c Compare July 13, 2024 18:06

cristianoc force-pushed the switch-on-uncurried branch from bbf66b6 to 2410c1c Compare July 13, 2024 18:06

cristianoc force-pushed the audit-curried branch from c57264c to da89285 Compare July 14, 2024 04:03

cristianoc force-pushed the switch-on-uncurried branch from 2410c1c to 31f0783 Compare July 14, 2024 04:07

cristianoc force-pushed the audit-curried branch from da89285 to 23f2d70 Compare July 14, 2024 06:37

cristianoc force-pushed the switch-on-uncurried branch from 31f0783 to 8c84d67 Compare July 14, 2024 06:38

cristianoc changed the base branch from audit-curried to master July 14, 2024 06:42

cristianoc mentioned this pull request Jul 14, 2024

Audit what files are compiled in curried mode. #6853

Closed

cknitt mentioned this pull request Jul 14, 2024

Optimizations lost in uncurried mode #6879

Closed

3 tasks

cristianoc force-pushed the switch-on-uncurried branch from d8a21bd to a460b1f Compare July 16, 2024 08:17

cristianoc added 8 commits July 16, 2024 17:04

Fix issues with the arity of externals.

6a6a9f7

Audit what files are compiled in curried mode.

11d5787

Add a `-curried` option, and give an error whenever neither `-curried` not `-uncurried` are used. Then change the build and test files to always specify curried/uncurried.

Remove @uncurry needed to turn on uncurried for others.

3e4dd91

Don't rely on Curry in unit tests.

651888d

Switch the compiler to build in uncurried mode.

d1854ba

Update compiler artifacts.

6f1f563

Remove temporary -curried option.

2175004

Update CHANGELOG.md

49ac039

cristianoc force-pushed the switch-on-uncurried branch from a460b1f to 49ac039 Compare July 16, 2024 15:04

cknitt approved these changes Jul 17, 2024

View reviewed changes

cristianoc merged commit 7d6b8d9 into master Jul 17, 2024

cristianoc deleted the switch-on-uncurried branch July 17, 2024 14:30

Switch on uncurried #6864

Switch on uncurried #6864

Uh oh!

Conversation

cristianoc commented Jul 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cristianoc commented Jul 9, 2024

Uh oh!

cknitt Jul 9, 2024

Choose a reason for hiding this comment

Uh oh!

cknitt Jul 9, 2024

Choose a reason for hiding this comment

Uh oh!

cknitt Jul 9, 2024

Choose a reason for hiding this comment

Uh oh!

cknitt Jul 9, 2024

Choose a reason for hiding this comment

Uh oh!

cknitt Jul 9, 2024

Choose a reason for hiding this comment

Uh oh!

cknitt Jul 9, 2024

Choose a reason for hiding this comment

Uh oh!

cristianoc commented Jul 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cknitt commented Jul 14, 2024

Uh oh!

zth commented Jul 14, 2024

Uh oh!

cknitt commented Jul 14, 2024

Uh oh!

cristianoc commented Jul 15, 2024

Uh oh!

cknitt commented Jul 15, 2024

Uh oh!

cristianoc commented Jul 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zth commented Jul 16, 2024

Uh oh!

cknitt commented Jul 17, 2024

Uh oh!

cknitt commented Jul 17, 2024

Uh oh!

cknitt commented Jul 18, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cristianoc commented Jul 8, 2024 •

edited

Loading

cristianoc commented Jul 14, 2024 •

edited

Loading

cristianoc commented Jul 16, 2024 •

edited

Loading