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

[GR-47449] Verify return types of substituted methods #7340

Merged
merged 7 commits into from
Sep 6, 2023

Conversation

graalvmbot
Copy link
Collaborator

The return types of substitution methods must match the return type of the target method. Incompatible types can lead to failures in the compiled code, e.g., unexpected NullPointerException, which are hard to debug. Note that none of the changed substitutions suffered from these problems because although there were mismatches, the specified types were usually a super type of the target return types (e.g. Object instead of the concrete type). Still, this verification helps catching problems in new substitutions early.

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Sep 5, 2023
@@ -375,6 +375,7 @@ def extra_image_build_argument(self, benchmark, args):
'-H:+AllowFoldMethods',
'-H:-UseServiceLoaderFeature',
'-H:+AllowDeprecatedBuilderClassesOnImageClasspath', # needs to be removed once GR-41746 is fixed
'-H:+DisableSubstitutionReturnTypeCheck', # remove once Quarkus fixed their substitutions (GR-48152)
Copy link
Member

Choose a reason for hiding this comment

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

@zakkak I had to add this for our Quarkus jobs because in the (quite old) version we are using there were substitutions with return type mismatches. The offending substitution seems to be gone on master, but I have not checked all the others. Maybe things are fine, maybe not. Just a heads up. :)

Copy link
Collaborator

@zakkak zakkak Sep 11, 2023

Choose a reason for hiding this comment

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

Hi @zapster, thanks for the heads up.

It looks like there are some failures on Quarkus main as well which I am working on in quarkusio/quarkus#35847 and quarkusio/quarkus#35956

@graalvmbot graalvmbot force-pushed the je/svm-check-substitution-return-type-GR-47449 branch from fa39f59 to bcf1261 Compare September 6, 2023 07:06
@graalvmbot graalvmbot merged commit 0ceaa57 into master Sep 6, 2023
@graalvmbot graalvmbot deleted the je/svm-check-substitution-return-type-GR-47449 branch September 6, 2023 16:07
zakkak added a commit to zakkak/quarkus that referenced this pull request Sep 11, 2023
Resolves issues with new strict checking introduced with
oracle/graal#7340
zakkak added a commit to zakkak/quarkus that referenced this pull request Sep 12, 2023
Resolves issues with new strict checking introduced with
oracle/graal#7340
franz1981 pushed a commit to franz1981/quarkus that referenced this pull request Sep 13, 2023
Resolves issues with new strict checking introduced with
oracle/graal#7340
holly-cummins pushed a commit to holly-cummins/quarkus that referenced this pull request Feb 8, 2024
Resolves issues with new strict checking introduced with
oracle/graal#7340
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants