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

Extra assertions for mutators and epilogues #1182

Merged
merged 1 commit into from
Aug 2, 2024
Merged

Conversation

wks
Copy link
Collaborator

@wks wks commented Aug 1, 2024

This commit adds assertions about the number of mutators and the list of mutators returned from the VM binding.

Also asserts that epilogues are not silently dropped before they are executed. It could happen if a buggy VM binding returns a number of mutators that does not match the actual list of mutators.

This should detect bugs like mmtk/mmtk-ruby#84

This commit adds assertions about the number of mutators and the list of
mutators returned from the VM binding.

Also asserts that epilogues are not silently dropped before they are
executed.  It could happen if a buggy VM binding returns a number of
mutators that does not match the actual list of mutators.

This should detect bugs like mmtk/mmtk-ruby#84
wks added a commit to wks/mmtk-ruby that referenced this pull request Aug 1, 2024
Updated the `ruby` repo revision to fix a bug where the number of
mutators returned from `ActivePlan::number_of_mutators` does not match
the number of mutators returned from `ActivePlan::mutators`.

Also updated the `mmtk-core` repo revision.  More assertions are added
so that such bugs can be detected earlier.

mmtk-core PR: mmtk/mmtk-core#1182
ruby PR: mmtk/ruby#84

Fixes: mmtk#84
@wks
Copy link
Collaborator Author

wks commented Aug 1, 2024

binding-refs
RUBY_BINDING_REPO=wks/mmtk-ruby
RUBY_BINDING_REF=fix/mutator-count

@wks wks added the PR-extended-testing Run extended tests for the pull request label Aug 1, 2024
@wks wks requested a review from qinsoon August 1, 2024 06:23
Copy link
Member

@qinsoon qinsoon left a comment

Choose a reason for hiding this comment

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

LGTM

src/util/epilogue.rs Show resolved Hide resolved
@wks wks added this pull request to the merge queue Aug 1, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 1, 2024
@wks wks added this pull request to the merge queue Aug 1, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 1, 2024
@wks wks added this pull request to the merge queue Aug 2, 2024
Merged via the queue into mmtk:master with commit f785236 Aug 2, 2024
31 checks passed
@wks wks deleted the fix/mutator-count branch August 2, 2024 03:20
mmtkgc-bot added a commit to mmtk/mmtk-ruby that referenced this pull request Aug 2, 2024
Updated the `ruby` repo revision to fix a bug where the number of
mutators returned from `ActivePlan::number_of_mutators` does not match
the number of mutators returned from `ActivePlan::mutators`.

Also updated the `mmtk-core` repo revision. More assertions are added so
that such bugs can be detected earlier.

mmtk-core PR: mmtk/mmtk-core#1182
ruby PR: mmtk/ruby#84

Fixes: #84

---------

Co-authored-by: mmtkgc-bot <mmtkgc.bot@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR-extended-testing Run extended tests for the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants