From 160420b8d4ebd8e539fbd3e8e60e727baf1a4a40 Mon Sep 17 00:00:00 2001 From: Lucy Date: Tue, 25 Feb 2025 00:32:53 -0500 Subject: [PATCH 1/2] Change `Value::globals()`, `Value::world()`, and `Value::null()` to be `const` variables instead. Also mark various functions as `const fn` --- Cargo.toml | 1 + auxcov/src/lib.rs | 7 +-- auxtools-impl/src/lib.rs | 8 ++-- auxtools/src/hooks.rs | 6 +-- auxtools/src/list.rs | 2 +- auxtools/src/proc.rs | 14 +----- auxtools/src/raw_types/misc.rs | 5 +- auxtools/src/raw_types/procs.rs | 1 + auxtools/src/raw_types/strings.rs | 2 +- auxtools/src/string.rs | 2 +- auxtools/src/value.rs | 78 +++++++++++++++++-------------- auxtools/src/weak_value.rs | 2 +- debug_server/src/assemble_env.rs | 2 +- debug_server/src/lib.rs | 4 +- debug_server/src/server.rs | 4 +- tests/auxtest/src/lib.rs | 2 +- tests/auxtest/src/lists.rs | 2 +- 17 files changed, 71 insertions(+), 71 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 4517eaa..c6eda8b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,6 +24,7 @@ correctness = { level = "warn", priority = -1 } perf = { level = "warn", priority = -1 } style = { level = "warn", priority = -1 } suspicious = { level = "warn", priority = -1 } +missing_const_for_fn = "warn" missing_safety_doc = "allow" missing_transmute_annotations = "allow" type_complexity = "allow" diff --git a/auxcov/src/lib.rs b/auxcov/src/lib.rs index d74feff..931166e 100644 --- a/auxcov/src/lib.rs +++ b/auxcov/src/lib.rs @@ -54,14 +54,14 @@ fn start_code_coverage(coverage_file: Value) { return Err(runtime!("A code coverage context for {} already exists!", coverage_file_string)); } - Ok(Value::null()) + Ok(Value::NULL) } #[hook("/proc/stop_code_coverage")] fn stop_code_coverage(coverage_file: Value) { let coverage_file_string = coverage_file.as_string()?; - let mut result = Ok(Value::null()); + let mut result = Ok(Value::NULL); with_tracker_option( |tracker| { let inner_result = tracker.finalize_context(&coverage_file_string); @@ -70,7 +70,7 @@ fn stop_code_coverage(coverage_file: Value) { if !had_entry { Err(runtime!("A code coverage context for {} does not exist!", coverage_file_string)) } else { - Ok(Value::null()) + Ok(Value::NULL) } } Err(error) => Err(runtime!("A error occurred while trying to save the coverage file: {}", error)) @@ -82,4 +82,5 @@ fn stop_code_coverage(coverage_file: Value) { result } +#[allow(clippy::missing_const_for_fn)] pub fn anti_dce_stub() {} diff --git a/auxtools-impl/src/lib.rs b/auxtools-impl/src/lib.rs index 5dc8fd3..f9d89a6 100644 --- a/auxtools-impl/src/lib.rs +++ b/auxtools-impl/src/lib.rs @@ -185,7 +185,7 @@ pub fn pin_dll(attr: TokenStream) -> TokenStream { /// if let Some(num) = num.as_number() { /// Value::from(num * 2.0); /// } -/// Value::null() +/// Value::NULL /// } /// ``` /// @@ -196,7 +196,7 @@ pub fn pin_dll(attr: TokenStream) -> TokenStream { /// fn on_honked(honker: Value) { /// src.call("gib", &[]); /// honker.call("laugh", &[]); -/// Value::null() +/// Value::NULL /// } /// ``` #[proc_macro_attribute] @@ -255,14 +255,14 @@ pub fn hook(attr: TokenStream, item: TokenStream) -> TokenStream { } let _default_null = quote! { #[allow(unreachable_code)] - auxtools::Value::null() + auxtools::Value::NULL }; let result = quote! { #cthook_prelude #signature { if #args_len > args.len() { for i in 0..#args_len - args.len() { - args.push(auxtools::Value::null()) + args.push(auxtools::Value::NULL) } } let (#arg_names) = (#proc_arg_unpacker); diff --git a/auxtools/src/hooks.rs b/auxtools/src/hooks.rs index 3a4f4a3..295fe60 100644 --- a/auxtools/src/hooks.rs +++ b/auxtools/src/hooks.rs @@ -47,7 +47,7 @@ struct Detours { } impl Detours { - pub fn new() -> Self { + pub const fn new() -> Self { Self { runtime_detour: None, call_proc_detour: None @@ -55,7 +55,7 @@ impl Detours { } } -thread_local!(static DETOURS: RefCell = RefCell::new(Detours::new())); +thread_local!(static DETOURS: RefCell = const { RefCell::new(Detours::new()) }); pub enum HookFailure { NotInitialized, @@ -197,7 +197,7 @@ extern "C" fn call_proc_by_id_hook( .unwrap() .call(&[&Value::from_string(format!("{} HookPath: {}", e.message.as_str(), path.as_str())).unwrap()]) .unwrap(); - Some(Value::null().raw) + Some(Value::NULL.raw) } } } diff --git a/auxtools/src/list.rs b/auxtools/src/list.rs index f313580..0e94646 100644 --- a/auxtools/src/list.rs +++ b/auxtools/src/list.rs @@ -97,7 +97,7 @@ impl List { self.len() == 0 } - pub fn is_list(value: &Value) -> bool { + pub const fn is_list(value: &Value) -> bool { matches!( value.raw.tag, raw_types::values::ValueTag::List diff --git a/auxtools/src/proc.rs b/auxtools/src/proc.rs index 5213497..c9db1b3 100644 --- a/auxtools/src/proc.rs +++ b/auxtools/src/proc.rs @@ -131,19 +131,7 @@ impl Proc { let args: Vec<_> = args.iter().map(|e| e.raw).collect(); - if raw_types::funcs::call_proc_by_id( - &mut ret, - Value::null().raw, - 0, - self.id, - 0, - Value::null().raw, - args.as_ptr(), - args.len(), - 0, - 0 - ) == 1 - { + if raw_types::funcs::call_proc_by_id(&mut ret, Value::NULL.raw, 0, self.id, 0, Value::NULL.raw, args.as_ptr(), args.len(), 0, 0) == 1 { return Ok(Value::from_raw_owned(ret)); } } diff --git a/auxtools/src/raw_types/misc.rs b/auxtools/src/raw_types/misc.rs index 21d0edc..f6509f7 100644 --- a/auxtools/src/raw_types/misc.rs +++ b/auxtools/src/raw_types/misc.rs @@ -1,3 +1,4 @@ +#![allow(non_camel_case_types)] use std::ffi::c_void; use super::strings; @@ -111,13 +112,13 @@ pub struct ParametersData { } impl Parameters_V1 { - pub fn count(&self) -> usize { + pub const fn count(&self) -> usize { (self.params_count_mul_4 / 4) as usize } } impl Parameters_V2 { - pub fn count(&self) -> usize { + pub const fn count(&self) -> usize { (self.params_count_mul_4 / 4) as usize } } diff --git a/auxtools/src/raw_types/procs.rs b/auxtools/src/raw_types/procs.rs index d84b339..2dab3ad 100644 --- a/auxtools/src/raw_types/procs.rs +++ b/auxtools/src/raw_types/procs.rs @@ -1,3 +1,4 @@ +#![allow(clippy::missing_const_for_fn)] use std::sync::OnceLock; use super::{misc, strings, values}; diff --git a/auxtools/src/raw_types/strings.rs b/auxtools/src/raw_types/strings.rs index 8e21881..a2a890c 100644 --- a/auxtools/src/raw_types/strings.rs +++ b/auxtools/src/raw_types/strings.rs @@ -5,7 +5,7 @@ use std::os::raw::c_char; pub struct StringId(pub u32); impl StringId { - pub fn valid(&self) -> bool { + pub const fn valid(&self) -> bool { self.0 != 0xFFFF } } diff --git a/auxtools/src/string.rs b/auxtools/src/string.rs index 01c43d1..0d5c6f2 100644 --- a/auxtools/src/string.rs +++ b/auxtools/src/string.rs @@ -52,7 +52,7 @@ impl StringRef { } } - pub fn get_id(&self) -> raw_types::strings::StringId { + pub const fn get_id(&self) -> raw_types::strings::StringId { unsafe { self.value.raw.data.string } } diff --git a/auxtools/src/value.rs b/auxtools/src/value.rs index 16e0099..c2eec1b 100644 --- a/auxtools/src/value.rs +++ b/auxtools/src/value.rs @@ -38,6 +38,31 @@ impl Drop for Value { } impl Value { + /// Equivalent to DM's `global.vars`. + pub const GLOBAL: Self = Self { + raw: raw_types::values::Value { + tag: raw_types::values::ValueTag::World, + data: raw_types::values::ValueData { id: 1 } + }, + phantom: PhantomData {} + }; + /// Equivalent to DM's null. + pub const NULL: Self = Self { + raw: raw_types::values::Value { + tag: raw_types::values::ValueTag::Null, + data: raw_types::values::ValueData { number: 0.0 } + }, + phantom: PhantomData {} + }; + /// Equivalent to DM's `world`. + pub const WORLD: Self = Self { + raw: raw_types::values::Value { + tag: raw_types::values::ValueTag::World, + data: raw_types::values::ValueData { id: 0 } + }, + phantom: PhantomData {} + }; + /// Creates a new value from raw tag and data. /// Use if you know what you are doing. pub unsafe fn new(tag: raw_types::values::ValueTag, data: raw_types::values::ValueData) -> Value { @@ -51,41 +76,26 @@ impl Value { } /// Equivalent to DM's `global.vars`. - pub fn globals() -> Value { - Value { - raw: raw_types::values::Value { - tag: raw_types::values::ValueTag::World, - data: raw_types::values::ValueData { id: 1 } - }, - phantom: PhantomData {} - } + #[deprecated(note = "please use the `GLOBAL` const instead")] + pub const fn globals() -> Value { + Self::GLOBAL } /// Equivalent to DM's `world`. - pub fn world() -> Value { - Value { - raw: raw_types::values::Value { - tag: raw_types::values::ValueTag::World, - data: raw_types::values::ValueData { id: 0 } - }, - phantom: PhantomData {} - } + #[deprecated(note = "please use the `WORLD` const instead")] + pub const fn world() -> Value { + Self::WORLD } /// Equivalent to DM's `null`. - pub fn null() -> Value { - Value { - raw: raw_types::values::Value { - tag: raw_types::values::ValueTag::Null, - data: raw_types::values::ValueData { number: 0.0 } - }, - phantom: PhantomData {} - } + #[deprecated(note = "please use the `NULL` const instead")] + pub const fn null() -> Value { + Self::NULL } /// Gets a turf by ID, without bounds checking. Use turf_by_id if you're not /// sure about how to check the bounds. - pub unsafe fn turf_by_id_unchecked(id: u32) -> Value { + pub const unsafe fn turf_by_id_unchecked(id: u32) -> Value { Value { raw: raw_types::values::Value { tag: raw_types::values::ValueTag::Turf, @@ -97,10 +107,9 @@ impl Value { /// Gets a turf by ID, with bounds checking. pub fn turf_by_id(id: u32) -> DMResult { - let world = Value::world(); - let max_x = world.get_number(crate::byond_string!("maxx"))? as u32; - let max_y = world.get_number(crate::byond_string!("maxy"))? as u32; - let max_z = world.get_number(crate::byond_string!("maxz"))? as u32; + let max_x = Self::WORLD.get_number(crate::byond_string!("maxx"))? as u32; + let max_y = Self::WORLD.get_number(crate::byond_string!("maxy"))? as u32; + let max_z = Self::WORLD.get_number(crate::byond_string!("maxz"))? as u32; if (0..max_x * max_y * max_z).contains(&(id - 1)) { Ok(unsafe { Value::turf_by_id_unchecked(id) }) } else { @@ -110,10 +119,9 @@ impl Value { /// Gets a turf by coordinates. pub fn turf(x: u32, y: u32, z: u32) -> DMResult { - let world = Value::world(); - let max_x = world.get_number(crate::byond_string!("maxx"))? as u32; - let max_y = world.get_number(crate::byond_string!("maxy"))? as u32; - let max_z = world.get_number(crate::byond_string!("maxz"))? as u32; + let max_x = Self::WORLD.get_number(crate::byond_string!("maxx"))? as u32; + let max_y = Self::WORLD.get_number(crate::byond_string!("maxy"))? as u32; + let max_z = Self::WORLD.get_number(crate::byond_string!("maxz"))? as u32; let x = x - 1; // thanks byond let y = y - 1; let z = z - 1; @@ -227,7 +235,7 @@ impl Value { if raw_types::funcs::call_datum_proc_by_name( &mut ret, - Value::null().raw, + Value::NULL.raw, 2, name_ref.value.raw.data.string, self.raw, @@ -347,7 +355,7 @@ impl Value { /// same as from_raw but does not increment the reference count (assumes we /// already own this reference) - pub unsafe fn from_raw_owned(v: raw_types::values::Value) -> Value { + pub const unsafe fn from_raw_owned(v: raw_types::values::Value) -> Value { Value { raw: v, phantom: PhantomData {} diff --git a/auxtools/src/weak_value.rs b/auxtools/src/weak_value.rs index 64dad31..b4943e5 100644 --- a/auxtools/src/weak_value.rs +++ b/auxtools/src/weak_value.rs @@ -83,7 +83,7 @@ impl WeakValue { pub fn upgrade_or_null(&self) -> Value { match self.upgrade() { Some(v) => v, - None => Value::null() + None => Value::NULL } } } diff --git a/debug_server/src/assemble_env.rs b/debug_server/src/assemble_env.rs index ed65037..ec6ddef 100644 --- a/debug_server/src/assemble_env.rs +++ b/debug_server/src/assemble_env.rs @@ -46,7 +46,7 @@ impl dmasm::assembler::AssembleEnv for AssembleEnv { let res = proc.call(&[&path]).unwrap().as_list().unwrap().get(1).unwrap(); - if res == Value::null() { + if res == Value::NULL { return None; } diff --git a/debug_server/src/lib.rs b/debug_server/src/lib.rs index e118e18..4047d11 100644 --- a/debug_server/src/lib.rs +++ b/debug_server/src/lib.rs @@ -71,7 +71,7 @@ fn enable_debugging(mode: Value, port: Value) { let server = match mode.as_str() { "NONE" => { - return Ok(Value::null()); + return Ok(Value::NULL); } "LAUNCHED" => server::Server::connect(&addr).map_err(|e| runtime!("Couldn't create debug server: {}", e))?, @@ -99,5 +99,5 @@ fn enable_debugging(mode: Value, port: Value) { INSTRUCTION_HOOKS.get_mut().push(Box::new(debug_server_instruction_hook)); } - Ok(Value::null()) + Ok(Value::NULL) } diff --git a/debug_server/src/server.rs b/debug_server/src/server.rs index 58493dc..ea99cc8 100644 --- a/debug_server/src/server.rs +++ b/debug_server/src/server.rs @@ -193,7 +193,7 @@ impl Server { }) } - pub fn is_in_eval(&self) -> bool { + pub const fn is_in_eval(&self) -> bool { self.in_eval } @@ -635,7 +635,7 @@ impl Server { let arguments = Variables::Arguments { frame: frame_id }; let locals = Variables::Locals { frame: frame_id }; - let globals = Variables::ObjectVars(Value::globals()); + let globals = Variables::ObjectVars(Value::GLOBAL); let response = Response::Scopes { arguments: Some(state.get_ref(arguments)), diff --git a/tests/auxtest/src/lib.rs b/tests/auxtest/src/lib.rs index 77d553b..eda00c5 100644 --- a/tests/auxtest/src/lib.rs +++ b/tests/auxtest/src/lib.rs @@ -18,5 +18,5 @@ fn inc_counter() { #[hook("/proc/auxtest_out")] fn out(msg: Value) { eprintln!("\n{}", msg.as_string()?); - Ok(Value::null()) + Ok(Value::NULL) } diff --git a/tests/auxtest/src/lists.rs b/tests/auxtest/src/lists.rs index 4cb42d7..4160a73 100644 --- a/tests/auxtest/src/lists.rs +++ b/tests/auxtest/src/lists.rs @@ -47,7 +47,7 @@ fn test_lists() { } for n in 1..=6 { - if list_b.get(n)? != Value::null() { + if list_b.get(n)? != Value::NULL { return Err(runtime!("test_lists: list_b[{}] != null", n)); } } From 673574fed9609ebea3c4f83197d484f7f4674a28 Mon Sep 17 00:00:00 2001 From: Lucy Date: Tue, 25 Feb 2025 00:38:32 -0500 Subject: [PATCH 2/2] My gut feeling says this is better. Idk why, but my gut feeling is usually correct, so I'm doing this. --- auxtools/src/value.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/auxtools/src/value.rs b/auxtools/src/value.rs index c2eec1b..50e5c02 100644 --- a/auxtools/src/value.rs +++ b/auxtools/src/value.rs @@ -107,9 +107,10 @@ impl Value { /// Gets a turf by ID, with bounds checking. pub fn turf_by_id(id: u32) -> DMResult { - let max_x = Self::WORLD.get_number(crate::byond_string!("maxx"))? as u32; - let max_y = Self::WORLD.get_number(crate::byond_string!("maxy"))? as u32; - let max_z = Self::WORLD.get_number(crate::byond_string!("maxz"))? as u32; + let world = Self::WORLD; + let max_x = world.get_number(crate::byond_string!("maxx"))? as u32; + let max_y = world.get_number(crate::byond_string!("maxy"))? as u32; + let max_z = world.get_number(crate::byond_string!("maxz"))? as u32; if (0..max_x * max_y * max_z).contains(&(id - 1)) { Ok(unsafe { Value::turf_by_id_unchecked(id) }) } else { @@ -119,9 +120,10 @@ impl Value { /// Gets a turf by coordinates. pub fn turf(x: u32, y: u32, z: u32) -> DMResult { - let max_x = Self::WORLD.get_number(crate::byond_string!("maxx"))? as u32; - let max_y = Self::WORLD.get_number(crate::byond_string!("maxy"))? as u32; - let max_z = Self::WORLD.get_number(crate::byond_string!("maxz"))? as u32; + let world = Self::WORLD; + let max_x = world.get_number(crate::byond_string!("maxx"))? as u32; + let max_y = world.get_number(crate::byond_string!("maxy"))? as u32; + let max_z = world.get_number(crate::byond_string!("maxz"))? as u32; let x = x - 1; // thanks byond let y = y - 1; let z = z - 1;