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.
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.
- links to
-
Review(lworld)
openjdk/valhalla/1787