cljs.core/IFn implementations generate duplicated code

Description

Currently when defining types that implement cljs.core/IFn a protocol fn is generated as well as a .call method that repeats the entire code instead of dispatching to the generated protocol fn.

.call repeats all code instead of calling this.cljs$core$IFn$_invoke$arity$<n>. Multiple arities repeat the code for each arity.

Should be something like

Environment

None

Activity

Show:
Mike Fikes
December 6, 2018, 12:45 AM

We've had a diagnostic in place via CLJS-2134 since 1.9.660.

Comment from David in Slack:

"I think 1.9.660 is probably long enough go to not be too concerned about it
I'm ok with the patch and we'll see what the feedback is"

Perhaps we can just remove the unit test that was added with as part of the patch for this ticket?

Thomas Heller
December 4, 2018, 11:47 PM

Correction: The test for currently fails.

I can adjust the patch to allow the variadic behavior but as it is officially not supported and generates a warning since 1.9.660 it may be time to remove support entirely?

Thomas Heller
December 4, 2018, 11:16 PM

Updated patch fixes bad implementation using max fixed arity assuming that all lower arities would always be present.

Also fixed all self-host issues and tests seem to pass fine now.

Thomas Heller
December 4, 2018, 8:23 PM

Not a big deal but the cljs.core code size goes from 168.8KB to 163.2KB (non-gzip) in my :advanced test build so it definitely reduces the amount of code generated overall.

Thomas Heller
December 4, 2018, 8:04 PM

Attached patch adjusts the generated .call fn as planned.

The generated code now is more compact and does not repeat.

I'm unsure why the self-host tests fail.

Assignee

Unassigned

Reporter

Thomas Heller

Labels

None

Approval

None

Patch

Code and Test

Priority

Major