Apply style check on auxiliary crates (macros and dummyvm) (#913)

This commit was originally backed out of #905. Add clippy check and
cargo fmt check for auxiliary crates (`macros` and `dummyvm`).

---------

Co-authored-by: Yi Lin <qinsoon@gmail.com>

.github/scripts/ci-style.sh

@@ -2,6 +2,8 @@
 
 export RUSTFLAGS="-D warnings"
 
+# --- Check main crate ---
+
 # check base
 cargo clippy
 # check all features
@@ -10,8 +12,6 @@ for_all_features "cargo clippy"
 for_all_features "cargo clippy --release"
 # check for tests
 for_all_features "cargo clippy --tests"
-# check for dummyvm
-cargo clippy --manifest-path=vmbindings/dummyvm/Cargo.toml
 
 # target-specific features
 if [[ $arch == "x86_64" && $os == "linux" ]]; then
@@ -20,5 +20,14 @@ if [[ $arch == "x86_64" && $os == "linux" ]]; then
     cargo clippy --tests --features perf_counter
 fi
 
-# check format
-cargo fmt -- --check
+# --- Check auxiliary crate ---
+
+style_check_auxiliary_crate() {
+    crate_path=$1
+
+    cargo clippy --manifest-path=$crate_path/Cargo.toml
+    cargo fmt --manifest-path=$crate_path/Cargo.toml -- --check
+}
+
+style_check_auxiliary_crate macros
+style_check_auxiliary_crate vmbindings/dummyvm

macros/src/lib.rs

@@ -1,17 +1,17 @@
 extern crate proc_macro;
-extern crate syn;
 extern crate proc_macro_error;
 extern crate quote;
+extern crate syn;
 
 use proc_macro::TokenStream;
-use proc_macro_error::proc_macro_error;
-use syn::{parse_macro_input};
 use proc_macro_error::abort_call_site;
+use proc_macro_error::proc_macro_error;
 use quote::quote;
+use syn::parse_macro_input;
 use syn::DeriveInput;
 
-mod util;
 mod plan_trace_object_impl;
+mod util;
 
 const DEBUG_MACRO_OUTPUT: bool = false;
 
@@ -37,15 +37,22 @@ pub fn derive_plan_trace_object(input: TokenStream) -> TokenStream {
     let output = if let syn::Data::Struct(syn::DataStruct {
         fields: syn::Fields::Named(ref fields),
         ..
-    }) = input.data {
+    }) = input.data
+    {
         let spaces = util::get_fields_with_attribute(fields, "trace");
         let post_scan_spaces = util::get_fields_with_attribute(fields, "post_scan");
         let fallback = util::get_unique_field_with_attribute(fields, "fallback_trace");
 
-        let trace_object_function = plan_trace_object_impl::generate_trace_object(&spaces, &fallback, &ty_generics);
-        let post_scan_object_function = plan_trace_object_impl::generate_post_scan_object(&post_scan_spaces, &fallback, &ty_generics);
-        let may_move_objects_function = plan_trace_object_impl::generate_may_move_objects(&spaces, &fallback, &ty_generics);
-        quote!{
+        let trace_object_function =
+            plan_trace_object_impl::generate_trace_object(&spaces, &fallback, &ty_generics);
+        let post_scan_object_function = plan_trace_object_impl::generate_post_scan_object(
+            &post_scan_spaces,
+            &fallback,
+            &ty_generics,
+        );
+        let may_move_objects_function =
+            plan_trace_object_impl::generate_may_move_objects(&spaces, &fallback, &ty_generics);
+        quote! {
             impl #impl_generics crate::plan::PlanTraceObject #ty_generics for #ident #ty_generics #where_clause {
                 #trace_object_function
 

macros/src/plan_trace_object_impl.rs

@@ -1,6 +1,6 @@
+use proc_macro2::TokenStream as TokenStream2;
 use quote::quote;
 use syn::{Field, TypeGenerics};
-use proc_macro2::TokenStream as TokenStream2;
 
 use crate::util;
 
@@ -12,7 +12,7 @@ pub(crate) fn generate_trace_object<'a>(
     // Generate a check with early return for each space
     let space_field_handler = space_fields.iter().map(|f| {
         let f_ident = f.ident.as_ref().unwrap();
-        let ref f_ty = f.ty;
+        let f_ty = &f.ty;
 
         // Figure out copy
         let trace_attr = util::get_field_attribute(f, "trace").unwrap();
@@ -42,7 +42,7 @@ pub(crate) fn generate_trace_object<'a>(
     // Generate a fallback to the parent plan
     let parent_field_delegator = if let Some(f) = parent_field {
         let f_ident = f.ident.as_ref().unwrap();
-        let ref f_ty = f.ty;
+        let f_ty = &f.ty;
         quote! {
             <#f_ty as PlanTraceObject #ty_generics>::trace_object::<Q, KIND>(&self.#f_ident, __mmtk_queue, __mmtk_objref, __mmtk_worker)
         }
@@ -70,7 +70,7 @@ pub(crate) fn generate_post_scan_object<'a>(
 ) -> TokenStream2 {
     let scan_field_handler = post_scan_object_fields.iter().map(|f| {
         let f_ident = f.ident.as_ref().unwrap();
-        let ref f_ty = f.ty;
+        let f_ty = &f.ty;
 
         quote! {
             if self.#f_ident.in_space(__mmtk_objref) {
@@ -84,7 +84,7 @@ pub(crate) fn generate_post_scan_object<'a>(
     // Generate a fallback to the parent plan
     let parent_field_delegator = if let Some(f) = parent_field {
         let f_ident = f.ident.as_ref().unwrap();
-        let ref f_ty = f.ty;
+        let f_ty = &f.ty;
         quote! {
             <#f_ty as PlanTraceObject #ty_generics>::post_scan_object(&self.#f_ident, __mmtk_objref)
         }
@@ -110,15 +110,15 @@ pub(crate) fn generate_may_move_objects<'a>(
 ) -> TokenStream2 {
     // If any space or the parent may move objects, the plan may move objects
     let space_handlers = space_fields.iter().map(|f| {
-        let ref f_ty = f.ty;
+        let f_ty = &f.ty;
 
         quote! {
             || <#f_ty as PolicyTraceObject #ty_generics>::may_move_objects::<KIND>()
         }
     });
 
     let parent_handler = if let Some(p) = parent_field {
-        let ref p_ty = p.ty;
+        let p_ty = &p.ty;
 
         quote! {
             || <#p_ty as PlanTraceObject #ty_generics>::may_move_objects::<KIND>()

vmbindings/dummyvm/src/active_plan.rs

@@ -1,14 +1,14 @@
-use mmtk::Plan;
-use mmtk::vm::ActivePlan;
-use mmtk::util::opaque_pointer::*;
-use mmtk::Mutator;
 use crate::DummyVM;
 use crate::SINGLETON;
+use mmtk::util::opaque_pointer::*;
+use mmtk::vm::ActivePlan;
+use mmtk::Mutator;
+use mmtk::Plan;
 
-pub struct VMActivePlan<> {}
+pub struct VMActivePlan {}
 
 impl ActivePlan<DummyVM> for VMActivePlan {
-    fn global() -> &'static dyn Plan<VM=DummyVM> {
+    fn global() -> &'static dyn Plan<VM = DummyVM> {
         SINGLETON.get_plan()
     }
 

vmbindings/dummyvm/src/api.rs

@@ -1,25 +1,31 @@
 // All functions here are extern function. There is no point for marking them as unsafe.
 #![allow(clippy::not_unsafe_ptr_arg_deref)]
 
+use crate::DummyVM;
+use crate::BUILDER;
+use crate::SINGLETON;
 use libc::c_char;
-use std::sync::atomic::Ordering;
-use std::ffi::CStr;
 use mmtk::memory_manager;
-use mmtk::AllocationSemantics;
-use mmtk::util::{ObjectReference, Address};
-use mmtk::util::opaque_pointer::*;
 use mmtk::scheduler::{GCController, GCWorker};
+use mmtk::util::opaque_pointer::*;
+use mmtk::util::{Address, ObjectReference};
+use mmtk::AllocationSemantics;
 use mmtk::Mutator;
-use crate::DummyVM;
-use crate::SINGLETON;
-use crate::BUILDER;
+use std::ffi::CStr;
+use std::sync::atomic::Ordering;
 
 #[no_mangle]
 pub extern "C" fn mmtk_init(heap_size: usize) {
     // set heap size first
     {
         let mut builder = BUILDER.lock().unwrap();
-        let success = builder.options.gc_trigger.set(mmtk::util::options::GCTriggerSelector::FixedHeapSize(heap_size));
+        let success =
+            builder
+                .options
+                .gc_trigger
+                .set(mmtk::util::options::GCTriggerSelector::FixedHeapSize(
+                    heap_size,
+                ));
         assert!(success, "Failed to set heap size to {}", heap_size);
     }
 
@@ -43,18 +49,37 @@ pub extern "C" fn mmtk_destroy_mutator(mutator: *mut Mutator<DummyVM>) {
 }
 
 #[no_mangle]
-pub extern "C" fn mmtk_alloc(mutator: *mut Mutator<DummyVM>, size: usize,
-                    align: usize, offset: usize, mut semantics: AllocationSemantics) -> Address {
-    if size >= SINGLETON.get_plan().constraints().max_non_los_default_alloc_bytes {
+pub extern "C" fn mmtk_alloc(
+    mutator: *mut Mutator<DummyVM>,
+    size: usize,
+    align: usize,
+    offset: usize,
+    mut semantics: AllocationSemantics,
+) -> Address {
+    if size
+        >= SINGLETON
+            .get_plan()
+            .constraints()
+            .max_non_los_default_alloc_bytes
+    {
         semantics = AllocationSemantics::Los;
     }
     memory_manager::alloc::<DummyVM>(unsafe { &mut *mutator }, size, align, offset, semantics)
 }
 
 #[no_mangle]
-pub extern "C" fn mmtk_post_alloc(mutator: *mut Mutator<DummyVM>, refer: ObjectReference,
-                                        bytes: usize, mut semantics: AllocationSemantics) {
-    if bytes >= SINGLETON.get_plan().constraints().max_non_los_default_alloc_bytes {
+pub extern "C" fn mmtk_post_alloc(
+    mutator: *mut Mutator<DummyVM>,
+    refer: ObjectReference,
+    bytes: usize,
+    mut semantics: AllocationSemantics,
+) {
+    if bytes
+        >= SINGLETON
+            .get_plan()
+            .constraints()
+            .max_non_los_default_alloc_bytes
+    {
         semantics = AllocationSemantics::Los;
     }
     memory_manager::post_alloc::<DummyVM>(unsafe { &mut *mutator }, refer, bytes, semantics)
@@ -66,7 +91,10 @@ pub extern "C" fn mmtk_will_never_move(object: ObjectReference) -> bool {
 }
 
 #[no_mangle]
-pub extern "C" fn mmtk_start_control_collector(tls: VMWorkerThread, controller: &'static mut GCController<DummyVM>) {
+pub extern "C" fn mmtk_start_control_collector(
+    tls: VMWorkerThread,
+    controller: &'static mut GCController<DummyVM>,
+) {
     memory_manager::start_control_collector(&SINGLETON, tls, controller);
 }
 
@@ -106,7 +134,7 @@ pub extern "C" fn mmtk_total_bytes() -> usize {
 }
 
 #[no_mangle]
-pub extern "C" fn mmtk_is_live_object(object: ObjectReference) -> bool{
+pub extern "C" fn mmtk_is_live_object(object: ObjectReference) -> bool {
     memory_manager::is_live_object(object)
 }
 
@@ -166,7 +194,11 @@ pub extern "C" fn mmtk_process(name: *const c_char, value: *const c_char) -> boo
     let name_str: &CStr = unsafe { CStr::from_ptr(name) };
     let value_str: &CStr = unsafe { CStr::from_ptr(value) };
     let mut builder = BUILDER.lock().unwrap();
-    memory_manager::process(&mut builder, name_str.to_str().unwrap(), value_str.to_str().unwrap())
+    memory_manager::process(
+        &mut builder,
+        name_str.to_str().unwrap(),
+        value_str.to_str().unwrap(),
+    )
 }
 
 #[no_mangle]
@@ -201,7 +233,11 @@ pub extern "C" fn mmtk_calloc(num: usize, size: usize) -> Address {
 
 #[no_mangle]
 #[cfg(feature = "malloc_counted_size")]
-pub extern "C" fn mmtk_realloc_with_old_size(addr: Address, size: usize, old_size: usize) -> Address {
+pub extern "C" fn mmtk_realloc_with_old_size(
+    addr: Address,
+    size: usize,
+    old_size: usize,
+) -> Address {
     memory_manager::realloc_with_old_size::<DummyVM>(&SINGLETON, addr, size, old_size)
 }
 #[no_mangle]

vmbindings/dummyvm/src/object_model.rs

@@ -1,7 +1,7 @@
+use crate::DummyVM;
 use mmtk::util::copy::{CopySemantics, GCWorkerCopyContext};
 use mmtk::util::{Address, ObjectReference};
 use mmtk::vm::*;
-use crate::DummyVM;
 
 pub struct VMObjectModel {}
 
@@ -11,10 +11,13 @@ pub const OBJECT_REF_OFFSET: usize = 4;
 
 impl ObjectModel<DummyVM> for VMObjectModel {
     const GLOBAL_LOG_BIT_SPEC: VMGlobalLogBitSpec = VMGlobalLogBitSpec::in_header(0);
-    const LOCAL_FORWARDING_POINTER_SPEC: VMLocalForwardingPointerSpec = VMLocalForwardingPointerSpec::in_header(0);
-    const LOCAL_FORWARDING_BITS_SPEC: VMLocalForwardingBitsSpec = VMLocalForwardingBitsSpec::in_header(0);
+    const LOCAL_FORWARDING_POINTER_SPEC: VMLocalForwardingPointerSpec =
+        VMLocalForwardingPointerSpec::in_header(0);
+    const LOCAL_FORWARDING_BITS_SPEC: VMLocalForwardingBitsSpec =
+        VMLocalForwardingBitsSpec::in_header(0);
     const LOCAL_MARK_BIT_SPEC: VMLocalMarkBitSpec = VMLocalMarkBitSpec::in_header(0);
-    const LOCAL_LOS_MARK_NURSERY_SPEC: VMLocalLOSMarkNurserySpec = VMLocalLOSMarkNurserySpec::in_header(0);
+    const LOCAL_LOS_MARK_NURSERY_SPEC: VMLocalLOSMarkNurserySpec =
+        VMLocalLOSMarkNurserySpec::in_header(0);
 
     const OBJECT_REF_OFFSET_LOWER_BOUND: isize = OBJECT_REF_OFFSET as isize;
 

vmbindings/dummyvm/src/reference_glue.rs

@@ -1,7 +1,7 @@
-use mmtk::vm::ReferenceGlue;
-use mmtk::util::ObjectReference;
-use mmtk::util::opaque_pointer::VMWorkerThread;
 use crate::DummyVM;
+use mmtk::util::opaque_pointer::VMWorkerThread;
+use mmtk::util::ObjectReference;
+use mmtk::vm::ReferenceGlue;
 
 pub struct VMReferenceGlue {}
 

vmbindings/dummyvm/src/scanning.rs

@@ -1,5 +1,5 @@
-use crate::DummyVM;
 use crate::edges::DummyVMEdge;
+use crate::DummyVM;
 use mmtk::util::opaque_pointer::*;
 use mmtk::util::ObjectReference;
 use mmtk::vm::EdgeVisitor;

vmbindings/dummyvm/src/tests/allocate_align_offset.rs

@@ -1,11 +1,11 @@
 // GITHUB-CI: MMTK_PLAN=all
 
 use crate::api;
+use crate::tests::fixtures::{MutatorFixture, SerialFixture};
 use crate::DummyVM;
-use crate::tests::fixtures::{SerialFixture, MutatorFixture};
+use log::info;
 use mmtk::plan::AllocationSemantics;
 use mmtk::vm::VMBinding;
-use log::info;
 
 lazy_static! {
     static ref MUTATOR: SerialFixture<MutatorFixture> = SerialFixture::new();
@@ -21,7 +21,12 @@ pub fn allocate_alignment() {
         while align <= max {
             info!("Test allocation with alignment {}", align);
             let addr = api::mmtk_alloc(fixture.mutator, 8, align, 0, AllocationSemantics::Default);
-            assert!(addr.is_aligned_to(align), "Expected allocation alignment {}, returned address is {:?}", align, addr);
+            assert!(
+                addr.is_aligned_to(align),
+                "Expected allocation alignment {}, returned address is {:?}",
+                align,
+                addr
+            );
             align *= 2;
         }
     })
@@ -36,9 +41,23 @@ pub fn allocate_offset() {
         info!("Allowed alignment between {} and {}", min, max);
         let mut align = min;
         while align <= max {
-            info!("Test allocation with alignment {} and offset {}", align, OFFSET);
-            let addr = api::mmtk_alloc(fixture.mutator, 8, align, OFFSET, AllocationSemantics::Default);
-            assert!((addr + OFFSET).is_aligned_to(align), "Expected allocation alignment {}, returned address is {:?}", align, addr);
+            info!(
+                "Test allocation with alignment {} and offset {}",
+                align, OFFSET
+            );
+            let addr = api::mmtk_alloc(
+                fixture.mutator,
+                8,
+                align,
+                OFFSET,
+                AllocationSemantics::Default,
+            );
+            assert!(
+                (addr + OFFSET).is_aligned_to(align),
+                "Expected allocation alignment {}, returned address is {:?}",
+                align,
+                addr
+            );
             align *= 2;
         }
     })