Skip to content
New issue

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

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

Already on GitHub? # to your account

Better error messages in case of TooLarge* exceptions in GenBCode #14943

Merged
merged 2 commits into from
Apr 19, 2022

Conversation

dos65
Copy link
Contributor

@dos65 dos65 commented Apr 14, 2022

Just a bit more informative messages for MethodTooLargeException and ClassTooLargeException instead of unhandled error with a large stacktrace.

Just a bit more informative messages for `MethodTooLargeException` and `ClassTooLargeException` instead of unhandled error with large stacktrace.
@michelou
Copy link
Contributor

michelou commented Apr 14, 2022

@dos65 On line 580 I read "Limit is 64KB" and on line 584 I read "Limit is 64K".
I suggest you to write in both cases "Limit is 64K." (with "." at the end).

Some references :

Reminder. The 64K limit is defined in Section "4.11 Limitations of the Java Virtual Machine" of the JVM Specification - Java SE 8 Edition (February 2015).

@dos65
Copy link
Contributor Author

dos65 commented Apr 14, 2022

@michelou I think it would be better to keep 64KB for MethodTooLargeException because that the actual unit of measure in this case. Also, error provides that actual size of method in bytes.

@michelou
Copy link
Contributor

@michelou I think it would be better to keep 64KB for MethodTooLargeException because that the actual unit of measure in this case. Also, error provides that actual size of method in bytes.

I agree, that's fine as long as both are the same.

Copy link
Contributor

@nicolasstucki nicolasstucki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise LGTM

@nicolasstucki nicolasstucki enabled auto-merge April 19, 2022 15:33
@nicolasstucki nicolasstucki merged commit ef16034 into scala:main Apr 19, 2022
@nicolasstucki
Copy link
Contributor

Fixes #14213

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants