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

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

Merged
merged 5 commits into from
Aug 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions .github/scripts/ci-style.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

export RUSTFLAGS="-D warnings"

# --- Check main crate ---

# check base
cargo clippy
# check all features
Expand All @@ -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
Expand All @@ -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
25 changes: 16 additions & 9 deletions macros/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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

Expand Down
14 changes: 7 additions & 7 deletions macros/src/plan_trace_object_impl.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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();
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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) {
Expand All @@ -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)
}
Expand All @@ -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>()
Expand Down
12 changes: 6 additions & 6 deletions vmbindings/dummyvm/src/active_plan.rs
Original file line number Diff line number Diff line change
@@ -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()
}

Expand Down
74 changes: 55 additions & 19 deletions vmbindings/dummyvm/src/api.rs
Original file line number Diff line number Diff line change
@@ -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);
}

Expand All @@ -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)
Expand All @@ -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);
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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]
Expand Down
11 changes: 7 additions & 4 deletions vmbindings/dummyvm/src/object_model.rs
Original file line number Diff line number Diff line change
@@ -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 {}

Expand All @@ -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;

Expand Down
6 changes: 3 additions & 3 deletions vmbindings/dummyvm/src/reference_glue.rs
Original file line number Diff line number Diff line change
@@ -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 {}

Expand Down
2 changes: 1 addition & 1 deletion vmbindings/dummyvm/src/scanning.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
31 changes: 25 additions & 6 deletions vmbindings/dummyvm/src/tests/allocate_align_offset.rs
Original file line number Diff line number Diff line change
@@ -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();
Expand All @@ -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;
}
})
Expand All @@ -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;
}
})
Expand Down
Loading
Loading