Skip to content

Conversation

@wks
Copy link
Collaborator

@wks wks commented Jun 17, 2024

This PR fixes new warnings generated by Clippy after upgrading Rust to 1.79.

  • Only define ImmixSpaceArg::mixed_age when the "vo_bit" feature is enabled. This eliminates a dead code warning.
  • Use the associated constant <integer_type>::MAX (and MIN, too) instead of the deprecated max_value() method and the ::std::<integer_type>::MAX constant.
  • Annotate the destination type of some transmute calls.
  • Use the safe std::ptr::NonNull::from() converter instead of transmute.
  • Initialize array elements of type Atomic<MapState> from constant.
  • Remove MutatorContext::barrier_impl. It is not used by any bindings right now, and it depends on the layout of &dyn Trait. Since Barrier has Downcast as a supertrait, if a binding needs a reference to a concrete Barrier implementation, it should just use downcast.

The warning against the transmute call in mock_vm.rs is suppressed because it is almost impossible to annotate.

Also fixes compilation problems:

  • Made benchmarks related to the "mock_test" feature conditionally compiled so that we don't get compilation error when running cargo clippy --benches or cargo clippy --all-targets.

@wks wks changed the title Fix cargo warnings for Rust 1.79 Fix clippy warnings for Rust 1.79 Jun 17, 2024
@wks
Copy link
Collaborator Author

wks commented Jun 17, 2024

Public API Check failed because I removed MutatorContext::barrier_impl(). DEFAULT_STRESS_FACTOR is not changed, but cargo-public-api thought its type was _.

@wks wks requested a review from qinsoon June 17, 2024 07:16
rust-toolchain Outdated
@@ -1 +1 @@
1.77.0
stable
Copy link
Member

Choose a reason for hiding this comment

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

You might have accidentally committed this change. The PR shows no intention to change the pinned Rust version.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oops. My bad. I reverted this.

///
/// # Safety
/// The safety of this function is ensured by a down-cast check.
unsafe fn barrier_impl<B: Barrier<VM>>(&mut self) -> &mut B;
Copy link
Member

Choose a reason for hiding this comment

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

I agree a binding can implement this method totally on the binding side if they need to. We can remove it.

The comments for write barriers methods mention using a specialized slow-path but they do not specifically mention this method. The comments seem to make sense even if we remove this method from our API. No change is required -- I just mention it in case you would like to improve the comments related to the function.

/// * Implement fast-path on the VM side, and do a specialized slow-path call.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes. The comment mentioned slow path, but it does not mention how to make the "specialized slow-path call". Calling methods of the Barrier trait is only one step of it. I think it is obvious to use downcast to eliminate a dynamic call for a trait that supports it.

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

@wks wks enabled auto-merge June 17, 2024 08:22
@wks wks added this pull request to the merge queue Jun 17, 2024
Merged via the queue into mmtk:master with commit 3be73b8 Jun 17, 2024
@wks wks deleted the fix/cargo-1.79 branch June 17, 2024 09:43
k-sareen pushed a commit to k-sareen/mmtk-core that referenced this pull request Jan 3, 2025
This PR fixes new warnings generated by Clippy after upgrading Rust to
1.79.

- Only define `ImmixSpaceArg::mixed_age` when the "vo_bit" feature is
enabled. This eliminates a dead code warning.
- Use the associated constant `<integer_type>::MAX` (and `MIN`, too)
instead of the deprecated `max_value()` method and the
`::std::<integer_type>::MAX` constant.
-   Annotate the destination type of some `transmute` calls.
- Use the safe `std::ptr::NonNull::from()` converter instead of
`transmute`.
-   Initialize array elements of type `Atomic<MapState>` from constant.
- Remove `MutatorContext::barrier_impl`. It is not used by any bindings
right now, and it depends on the layout of `&dyn Trait`. Since `Barrier`
has `Downcast` as a supertrait, if a binding needs a reference to a
concrete `Barrier` implementation, it should just use downcast.

The warning against the `transmute` call in `mock_vm.rs` is suppressed
because it is almost impossible to annotate.

Also fixes compilation problems:

- Made benchmarks related to the "mock_test" feature conditionally
compiled so that we don't get compilation error when running `cargo
clippy --benches` or `cargo clippy --all-targets`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants