Skip to content
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

Shortcircuit getTypeOfExpression for arithmetic operators when the type is trivially deducible by syntactically contained literals #30945

Conversation

weswigham
Copy link
Member

Fixes #29926

In this PR, we use the transform flag aggregation walk to categorize a numeric arithmetic expression (operators which don't conditionally do stringy coercion like +) which contains number or bigint literals somewhere in its expression tree. We then use this in the checker to short-circuit expression type calculation for these trees, preventing us from needing to walk the flow control graph (or anything else, really) to determine types. Technically this information could be re-derived on demand in the checker; but by calculating it during the transform flags walk it's pretty much free.

The primary change due to this is that in certain cases where previously we'd issue an error and make the expression the errorType (eg, when doing 2n - false), we now may return the bigint type instead. I've also changed checkBinaryLikeExpression proper to match. There's no real reason to return the errorType over the bigintType in these cases, since all it does is allow you to mask a followup number/bigint mixing bug with a prior one (really the type should be never to be pedantic, since the expression will throw at runtime; but the downstream effects of that would be terrible during editing), and the error can only arise in these cases when at least one of the arguments is a bigint. Plus the error type effectively disables completions on the expression result, when the fix is almost always going to be "convert some number somewhere into a bigint to the expression evaluates without error"; so allowing completions to continue assuming such a fix seems reasonable to me.

@weswigham
Copy link
Member Author

Technically we'd much rather be caching more in getFlowTypeOfNode (since this still leaves open the possibility of expressions without literals following the control flow graph), however we don't generally know if it's safe to do so with the context available, so cannot. A general fix here, IMO, probably involves multilevel type information caching in inference and intelligent cache hierarchies (ideally, tracking if a type was injected via inference or not and caching permanently or temporarily based on if the inputs to an operation use those injected types at all).

@weswigham
Copy link
Member Author

#31003 doesn;t have any perf downsides anymore and is a much better fix.

@weswigham weswigham closed this Apr 29, 2019
@weswigham weswigham deleted the optimize-binary-expression-type-lookup branch April 29, 2019 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extremely slow javascript compile (small file)
1 participant