Skip to content

Hack to allow concat to work even when an Array isn't assignable to ReadonlyArray #20455

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
1 commit merged into from
Dec 7, 2017

Conversation

ghost
Copy link

@ghost ghost commented Dec 4, 2017

Fixes #20268
Uses T[] | ReadonlyArray<T> instead of just ReadonlyArray<T>. There are probably a bunch of other places where we would need to do this to fix this error in general, but better to wait on #20454 for those instead.

@ghost ghost requested a review from rbuckton December 4, 2017 21:05
@RyanCavanaugh
Copy link
Member

Have you run this on RWC?

@ghost
Copy link
Author

ghost commented Dec 4, 2017

Will do tomorrow. This makes the signature more lenient (assuming covariant arrays) so I wouldn't expect any new RWC failures.

@ghost ghost force-pushed the array_concat branch 2 times, most recently from 42ef827 to 5d5bbf0 Compare December 5, 2017 17:26
@ghost ghost force-pushed the array_concat branch from 5d5bbf0 to dcc5815 Compare December 7, 2017 16:38
@ghost
Copy link
Author

ghost commented Dec 7, 2017

There are some rwc failures on master, but I compared this branch to master and didn't see any differences.

@ghost ghost merged commit 5e5b770 into master Dec 7, 2017
ghost pushed a commit that referenced this pull request Dec 7, 2017
ghost pushed a commit that referenced this pull request Dec 7, 2017
@ahejlsberg
Copy link
Member

@mhegazy @sandersn This PR appears to account for 50% of the slowdown we've seen in the checker between 2.6.2 and 2.7. It single-handedly slows down the check phase of compiling the compiler by about 7%. I haven't researched exactly why, but I suspect it has to do with us performing lots of structural comparisons of array types.

The ideal way to fix this issue would be to have all the callback function types in Array<T> provide a ReadonlyArray<T> (modifying the array in a callback is surely a bad thing), but obviously that would be a breaking change.

@mhegazy
Copy link
Contributor

mhegazy commented Feb 1, 2018

@ahejlsberg has a possible fir to reorder the #21462, can you take a look.

@mhegazy mhegazy deleted the array_concat branch February 1, 2018 19:38
@microsoft microsoft locked and limited conversation to collaborators Jul 3, 2018
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

polymorphic arguments validation error
3 participants