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

Add override to virtual methods of TR_J9SharedCache and TR_J9VMBase #20710

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

cjjdespres
Copy link
Contributor

These overrides are helpful when changing the interfaces of classes, so that warnings (at least in clang) are generated if the methods of the base classes aren't changed to compensate.

@cjjdespres
Copy link
Contributor Author

Attn @mpirvu. A few times now I've changed something in these classes in a way that conflicts with the base classes. This will at least provide some warning when I do that.

Copy link
Contributor

@mpirvu mpirvu left a comment

Choose a reason for hiding this comment

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

The change looks good.
Why only some of the methods were added the override specifier?

@mpirvu mpirvu self-assigned this Nov 29, 2024
@mpirvu
Copy link
Contributor

mpirvu commented Nov 29, 2024

jenkins compile all jdk8,jdk23

@cjjdespres
Copy link
Contributor Author

Why only some of the methods were added the override specifier?

Those are the only methods that are actually overridden from the base classes. The rest are declared for the first time in TR_J9VMBase and TR_J9SharedCache. I haven't checked manually, though - those are just the ones that clang complained about.

@mpirvu
Copy link
Contributor

mpirvu commented Nov 30, 2024

jenkins compile xlinux jdk23

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

Successfully merging this pull request may close these issues.

2 participants