Remove unsound casting functions
The casting functions in `unsafe.rs` were unsound (i.e., they allowed safe code to cause undefined behavior). While they did appear to be used in a way that wouldn't cause UB the fact that there exists unsound functions is unsettling. This commit removes those functions and replaces it with a macro that performs the same reification - the difference is that the macro call will also include the checks which are required to prevent UB. A macro was chosen instead of a function for two reasons: 1. A macro can keep the same code generation whereas a function would require going through an `Option` which has negative impacts on code generation (niche values cause poor DCE). 2. There exist other `unsafe` code blocks in the crate and an attempt to make Rhai 100% safe is completely out-of-scope for this merge request, so we may as well use `unsafe` in the macro. Regarding (2) above, I may come back at a later date with a 100% safe `reify` function but only once the other `unsafe` blocks are removed. For posterity, said function would look something like: ```rust fn reify<A: Any, C>(value: A) -> Option<C> { let mut v = Some(value); let v: &mut dyn Any = &mut v; v.downcast_mut::<Option<C>>().map(Option::take) } ```
This commit is contained in:
@@ -5,10 +5,9 @@
|
||||
use super::call::FnCallArgs;
|
||||
use super::callable_function::CallableFunction;
|
||||
use super::native::{FnAny, SendSync};
|
||||
use crate::r#unsafe::unsafe_cast;
|
||||
use crate::tokenizer::Position;
|
||||
use crate::types::dynamic::{DynamicWriteLock, Variant};
|
||||
use crate::{Dynamic, NativeCallContext, RhaiResultOf, ERR};
|
||||
use crate::{Dynamic, NativeCallContext, RhaiResultOf, ERR, reify};
|
||||
#[cfg(feature = "no_std")]
|
||||
use std::prelude::v1::*;
|
||||
use std::{any::TypeId, mem};
|
||||
@@ -46,11 +45,12 @@ pub fn by_value<T: Variant + Clone>(data: &mut Dynamic) -> T {
|
||||
// If T is `&str`, data must be `ImmutableString`, so map directly to it
|
||||
data.flatten_in_place();
|
||||
let ref_str = data.as_str_ref().expect("&str");
|
||||
let ref_t = unsafe { mem::transmute::<_, &T>(&ref_str) };
|
||||
let ref_t = reify!(ref_str, |ref_t: &T| ref_t, || unreachable!());
|
||||
ref_t.clone()
|
||||
} else if TypeId::of::<T>() == TypeId::of::<String>() {
|
||||
// If T is `String`, data must be `ImmutableString`, so map directly to it
|
||||
unsafe_cast(mem::take(data).into_string().expect("`ImmutableString`"))
|
||||
let t = mem::take(data).into_string().expect("`ImmutableString`");
|
||||
reify!(t, |t: T| t, || unreachable!())
|
||||
} else {
|
||||
// We consume the argument and then replace it with () - the argument is not supposed to be used again.
|
||||
// This way, we avoid having to clone the argument again, because it is already a clone when passed here.
|
||||
|
Reference in New Issue
Block a user