[lworld] RFR: 8373507: [lworld] Add override keyword to *Klass classes
Marc Chevalier
mchevalier at openjdk.org
Mon Dec 15 10:43:20 UTC 2025
On Thu, 11 Dec 2025 11:51:00 GMT, Axel Boldt-Christmas <aboldtch at openjdk.org> wrote:
> This is a enhancement which should also go into mainline, however since lworld is changing a lot of the Klass type hierarchy it seems more important to just get this into this repo.
>
> Using override provides better visibility and compiler support for suspicious virtual methods. It makes it clears when reading a class header file if a new virtual interface is introduced or if it simply overridden.
>
> It catches when the virtual hierarchy is different for different build configurations (like `is_flatArray_klass_slow` which is on the `Klass` in debug and on `FlatArrayKlass` in release. This is obviously a bug and `is_flatArray_klass_slow` should not be available in a release build) or suspicious virtual functions (like `InstanceKlass::restore_unshareable_info(ClassLoaderData*,Handle,PackageEntry*, JavaThread*)` which is only overridden by `InlineKlass` and only delegates to `InstanceKlass`, so it could just not have overridden this function).
>
> We should aim to migrate all our C++ classes to use `override` or `final` rather than virtual to catch more bugs and have more expressive header files. But it seems extra prudent here as lworld is moving around a lot of things in this area.
Alright, I probably wasn't quite clear about the scenario I had in mind, but it doesn't matter too much. It sounds like we agree overall.
I've checked it builds even on Mac. I like it. Thanks!
Is there plans to put more override to other class hierarchies? It's just a bit sad it makes history (git log/blame) more messy while not being semantically very relevant.
-------------
Marked as reviewed by mchevalier (Committer).
PR Review: https://git.openjdk.org/valhalla/pull/1787#pullrequestreview-3577519317
More information about the valhalla-dev
mailing list