Add brief documentation of overlay classes #2947
Conversation
owen-mc-diffblue
requested review from
cesaro,
cristina-david,
peterschrammel,
pkesseli,
smowton and
thk123
as code owners
owen-mc-diffblue
force-pushed
the
doc/overlay-classes
branch
from
d37bbdd to
25fb9be
Compare
There was a problem hiding this comment.
Passed Diffblue compatibility checks (cbmc commit: 25fb9be).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/84714306
| /// its list of annotations. | ||
| /// | ||
| /// An overlay class is a class with the annotation | ||
| /// \@OverlayClassImplementation. They serve the following purpose. When the JVM |
There was a problem hiding this comment.
There should also be some documentation for the other two annotations (IgnoredMethodImplementation and OverlayMethodImplementation). Could you add a TODO for these here or wherever would be a good place to document them?
There was a problem hiding this comment.
I've added documentation for them
owen-mc-diffblue
force-pushed
the
doc/overlay-classes
branch
from
25fb9be to
d1b8314
Compare
| /// `ID_overlay_method` in its list of annotations. | ||
| /// | ||
| /// An overlay method is a method with the annotation | ||
| /// \@OverlayMethodImplementation. They should only appear in |
There was a problem hiding this comment.
👍 Thanks for explaining this, I wasn't sure if @OverlayClassImplementation automatically turned all methods into overlay methods, of if within an overlay class methods could still be declared to be overlay methods or not.
| /// An overlay method is a method with the annotation | ||
| /// \@OverlayMethodImplementation. They should only appear in | ||
| /// [overlay classes](\ref java_class_loader.cpp::is_overlay_class). They | ||
| /// will be loaded by JBMC instead of the corresponding method in the |
There was a problem hiding this comment.
🐝 Could you explain what "the corresponding method" is (i.e. method with the same signature in a class of the same name that was loaded before)?
| /// [overlay classes](\ref java_class_loader.cpp::is_overlay_class), in | ||
| /// particular for methods which must exist in the overlay class so that | ||
| /// it will compile, e.g. default constructors, but which we do not want | ||
| /// to overlay the corresponding method in the |
owen-mc-diffblue
force-pushed
the
doc/overlay-classes
branch
from
d1b8314 to
ae0bb0a
Compare
|
I've improved the documentation and also added a test for ignored methods. I've done it in the context of an overlay class, although they do not need to be used in that context, because that is what they were created for. |
There was a problem hiding this comment.
Passed Diffblue compatibility checks (cbmc commit: ae0bb0a).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/84818067
Future improvements: - move this documentation to a better place
owen-mc-diffblue
force-pushed
the
doc/overlay-classes
branch
from
ae0bb0a to
3a2a898
Compare
owen-mc-diffblue
force-pushed
the
doc/overlay-classes
branch
from
3a2a898 to
4130006
Compare
There was a problem hiding this comment.
This PR failed Diffblue compatibility checks (cbmc commit: 3a2a898).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/84832959
Status will be re-evaluated on next push.
Please contact @peterschrammel, @thk123, or @allredj for support.
Common spurious failures:
- the cbmc commit has disappeared in the mean time (e.g. in a force-push)
- the author is not in the list of contributors (e.g. first-time contributors).
There was a problem hiding this comment.
Passed Diffblue compatibility checks (cbmc commit: 4130006).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/84833676
It could be expanded in future, but it's better than what was there
before