Skip to content

Optimizer: Fail if we cannot inline RuntimeLong #5188

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
merged 1 commit into from
Jun 3, 2025

Conversation

gzm0
Copy link
Contributor

@gzm0 gzm0 commented Jun 1, 2025

If we cannot inline RuntimeLong (the class or its static methods),
something is broken and we should fail.

Extracted and adapted from #5077.

@gzm0 gzm0 requested a review from sjrd June 1, 2025 11:06
@sjrd
Copy link
Member

sjrd commented Jun 1, 2025

I don't understand how the title of the first commit relates to its changes. It looks like the changes are nothing but a refactoring, or possibly taking a shortcut in the optimizer internals.

Could you elaborate?

@gzm0 gzm0 force-pushed the opt-long-cleanup branch from 28f618f to a79e265 Compare June 2, 2025 19:10
@gzm0 gzm0 changed the title Cleanups to the optimizer long inlining Optimizer: Fail if we cannot inline RuntimeLong Jun 2, 2025
@gzm0
Copy link
Contributor Author

gzm0 commented Jun 2, 2025

Ah, ah. I was under the expression that it is actually possible for the expansion to hit the "not-inline" case. It seems this doesn't happen (and I guess shouldn't happen).

I have adjusted this PR to also throw an assertion error and squashed the commit.

PTAL.

If we cannot inline RuntimeLong (the class or its static methods),
something is broken and we should fail.
@gzm0 gzm0 force-pushed the opt-long-cleanup branch from a79e265 to 3a253e3 Compare June 2, 2025 19:13
@sjrd sjrd enabled auto-merge June 2, 2025 19:49
@sjrd sjrd merged commit 7e9dbfc into scala-js:main Jun 3, 2025
3 checks passed
@gzm0 gzm0 deleted the opt-long-cleanup branch June 7, 2025 07:44
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.

2 participants