From a1d42b826add7dd71ee0df222a7ba9174048b7be Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Wed, 9 Nov 2022 20:18:11 +0800 Subject: [PATCH] Simplify Dynamic::as_XXX calls. --- CHANGELOG.md | 1 + src/eval/expr.rs | 3 -- src/eval/stmt.rs | 4 +- src/func/builtin.rs | 16 ++++---- src/types/dynamic.rs | 96 +++++++++++++++++++++++++------------------- 5 files changed, 66 insertions(+), 54 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dbd2fae6..c123b440 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ Bug fixes * `import` statements inside `eval` no longer cause errors in subsequent code. * Functions marked `global` in `import`ed modules with no alias names now work properly. * Incorrect loop optimizations that are too aggressive (e.g. unrolling a `do { ... } until true` with a `break` statement inside) and cause crashes are removed. +* `Dynamic::is` now works properly for shared values. Breaking changes ---------------- diff --git a/src/eval/expr.rs b/src/eval/expr.rs index b691eef1..92d392bd 100644 --- a/src/eval/expr.rs +++ b/src/eval/expr.rs @@ -52,7 +52,6 @@ impl Engine { global: &mut GlobalRuntimeState, caches: &mut Caches, lib: &[SharedModule], - scope: &'s mut Scope, this_ptr: &'s mut Dynamic, expr: &Expr, @@ -136,7 +135,6 @@ impl Engine { global: &mut GlobalRuntimeState, caches: &mut Caches, lib: &[SharedModule], - scope: &'s mut Scope, this_ptr: &'s mut Dynamic, expr: &Expr, @@ -221,7 +219,6 @@ impl Engine { global: &mut GlobalRuntimeState, caches: &mut Caches, lib: &[SharedModule], - scope: &mut Scope, this_ptr: &mut Dynamic, expr: &Expr, diff --git a/src/eval/stmt.rs b/src/eval/stmt.rs index f2d0088a..7aedeb59 100644 --- a/src/eval/stmt.rs +++ b/src/eval/stmt.rs @@ -6,7 +6,7 @@ use crate::ast::{ ASTFlags, BinaryExpr, Expr, Ident, OpAssignment, Stmt, SwitchCasesCollection, TryCatchBlock, }; use crate::func::{get_builtin_op_assignment_fn, get_hasher}; -use crate::types::dynamic::{AccessMode, Union}; +use crate::types::dynamic::AccessMode; use crate::types::RestoreOnDrop; use crate::{ Dynamic, Engine, ImmutableString, Position, RhaiResult, RhaiResultOf, Scope, SharedModule, ERR, @@ -687,7 +687,7 @@ impl Engine { .map(|_| Dynamic::UNIT) .map_err(|result_err| match *result_err { // Re-throw exception - ERR::ErrorRuntime(Dynamic(Union::Unit(..)), pos) => { + ERR::ErrorRuntime(v, pos) if v.is_unit() => { err.set_position(pos); err } diff --git a/src/func/builtin.rs b/src/func/builtin.rs index 8ff37e67..b6461191 100644 --- a/src/func/builtin.rs +++ b/src/func/builtin.rs @@ -690,7 +690,7 @@ pub fn get_builtin_op_assignment_fn(op: &Token, x: &Dynamic, y: &Dynamic) -> Opt PlusAssign => Some(|_ctx, args| { let (first, second) = args.split_first_mut().expect(BUILTIN); let x = &mut *first.write_lock::().expect(BUILTIN); - let y = std::mem::take(second[0]).cast::(); + let y = &*second[0].read_lock::().expect(BUILTIN); #[cfg(not(feature = "unchecked"))] if !x.is_empty() && !y.is_empty() { @@ -704,7 +704,7 @@ pub fn get_builtin_op_assignment_fn(op: &Token, x: &Dynamic, y: &Dynamic) -> Opt MinusAssign => Some(|_, args| { let (first, second) = args.split_first_mut().expect(BUILTIN); let x = &mut *first.write_lock::().expect(BUILTIN); - let y = std::mem::take(second[0]).cast::(); + let y = &*second[0].read_lock::().expect(BUILTIN); Ok((*x -= y).into()) }), _ => None, @@ -718,7 +718,7 @@ pub fn get_builtin_op_assignment_fn(op: &Token, x: &Dynamic, y: &Dynamic) -> Opt return match op { PlusAssign => Some(|_ctx, args| { - let x = std::mem::take(args[1]).cast::(); + let x = std::mem::take(args[1]).into_array().expect(BUILTIN); if x.is_empty() { return Ok(Dynamic::UNIT); @@ -749,7 +749,7 @@ pub fn get_builtin_op_assignment_fn(op: &Token, x: &Dynamic, y: &Dynamic) -> Opt return match op { PlusAssign => Some(|_ctx, args| { - let blob2 = std::mem::take(args[1]).cast::(); + let blob2 = std::mem::take(args[1]).into_blob().expect(BUILTIN); let blob1 = &mut *args[0].write_lock::().expect(BUILTIN); #[cfg(not(feature = "unchecked"))] @@ -951,14 +951,14 @@ pub fn get_builtin_op_assignment_fn(op: &Token, x: &Dynamic, y: &Dynamic) -> Opt return match op { PlusAssign => Some(|_ctx, args| { - let s = std::mem::take(args[1]).cast::(); + let (first, second) = args.split_first_mut().expect(BUILTIN); + let blob = &mut *first.write_lock::().expect(BUILTIN); + let s = &*second[0].read_lock::().expect(BUILTIN); if s.is_empty() { return Ok(Dynamic::UNIT); } - let blob = &mut *args[0].write_lock::().expect(BUILTIN); - #[cfg(not(feature = "unchecked"))] _ctx.engine().raise_err_if_over_data_size_limit(( blob.len() + s.len(), @@ -966,7 +966,7 @@ pub fn get_builtin_op_assignment_fn(op: &Token, x: &Dynamic, y: &Dynamic) -> Opt 0, ))?; - Ok(append_str(blob, &s).into()) + Ok(append_str(blob, s).into()) }), _ => None, }; diff --git a/src/types/dynamic.rs b/src/types/dynamic.rs index a3139dde..3b1a6cee 100644 --- a/src/types/dynamic.rs +++ b/src/types/dynamic.rs @@ -260,10 +260,18 @@ impl Dynamic { } /// Is the value held by this [`Dynamic`] a particular type? /// - /// If the [`Dynamic`] is a shared variant checking is performed on top of its internal value. + /// # Panics or Deadlocks When Value is Shared + /// + /// Under the `sync` feature, this call may deadlock, or [panic](https://doc.rust-lang.org/std/sync/struct.RwLock.html#panics-1). + /// Otherwise, this call panics if the data is currently borrowed for write. #[inline] #[must_use] pub fn is(&self) -> bool { + #[cfg(not(feature = "no_closure"))] + if self.is_shared() { + return TypeId::of::() == self.type_id(); + } + if TypeId::of::() == TypeId::of::<()>() { return matches!(self.0, Union::Unit(..)); } @@ -1819,9 +1827,12 @@ impl Dynamic { #[inline] pub fn as_unit(&self) -> Result<(), &'static str> { match self.0 { - Union::Unit(v, ..) => Ok(v), + Union::Unit(..) => Ok(()), #[cfg(not(feature = "no_closure"))] - Union::Shared(..) => self.read_lock().map(|v| *v).ok_or_else(|| self.type_name()), + Union::Shared(ref cell, ..) => match crate::func::locked_read(cell).0 { + Union::Unit(..) => Ok(()), + _ => Err(cell.type_name()), + }, _ => Err(self.type_name()), } } @@ -1832,7 +1843,10 @@ impl Dynamic { match self.0 { Union::Int(n, ..) => Ok(n), #[cfg(not(feature = "no_closure"))] - Union::Shared(..) => self.read_lock().map(|v| *v).ok_or_else(|| self.type_name()), + Union::Shared(ref cell, ..) => match crate::func::locked_read(cell).0 { + Union::Int(n, ..) => Ok(n), + _ => Err(cell.type_name()), + }, _ => Err(self.type_name()), } } @@ -1846,7 +1860,10 @@ impl Dynamic { match self.0 { Union::Float(n, ..) => Ok(*n), #[cfg(not(feature = "no_closure"))] - Union::Shared(..) => self.read_lock().map(|v| *v).ok_or_else(|| self.type_name()), + Union::Shared(ref cell, ..) => match crate::func::locked_read(cell).0 { + Union::Float(n, ..) => Ok(*n), + _ => Err(cell.type_name()), + }, _ => Err(self.type_name()), } } @@ -1860,7 +1877,10 @@ impl Dynamic { match self.0 { Union::Decimal(ref n, ..) => Ok(**n), #[cfg(not(feature = "no_closure"))] - Union::Shared(..) => self.read_lock().map(|v| *v).ok_or_else(|| self.type_name()), + Union::Shared(ref cell, ..) => match crate::func::locked_read(cell).0 { + Union::Decimal(ref n, ..) => Ok(**n), + _ => Err(cell.type_name()), + }, _ => Err(self.type_name()), } } @@ -1871,7 +1891,10 @@ impl Dynamic { match self.0 { Union::Bool(b, ..) => Ok(b), #[cfg(not(feature = "no_closure"))] - Union::Shared(..) => self.read_lock().map(|v| *v).ok_or_else(|| self.type_name()), + Union::Shared(ref cell, ..) => match crate::func::locked_read(cell).0 { + Union::Bool(b, ..) => Ok(b), + _ => Err(cell.type_name()), + }, _ => Err(self.type_name()), } } @@ -1880,9 +1903,12 @@ impl Dynamic { #[inline] pub fn as_char(&self) -> Result { match self.0 { - Union::Char(n, ..) => Ok(n), + Union::Char(c, ..) => Ok(c), #[cfg(not(feature = "no_closure"))] - Union::Shared(..) => self.read_lock().map(|v| *v).ok_or_else(|| self.type_name()), + Union::Shared(ref cell, ..) => match crate::func::locked_read(cell).0 { + Union::Char(c, ..) => Ok(c), + _ => Err(cell.type_name()), + }, _ => Err(self.type_name()), } } @@ -1917,14 +1943,10 @@ impl Dynamic { match self.0 { Union::Str(s, ..) => Ok(s), #[cfg(not(feature = "no_closure"))] - Union::Shared(ref cell, ..) => { - let value = crate::func::locked_read(cell); - - match value.0 { - Union::Str(ref s, ..) => Ok(s.clone()), - _ => Err((*value).type_name()), - } - } + Union::Shared(ref cell, ..) => match crate::func::locked_read(cell).0 { + Union::Str(ref s, ..) => Ok(s.clone()), + _ => Err(cell.type_name()), + }, _ => Err(self.type_name()), } } @@ -1938,14 +1960,10 @@ impl Dynamic { match self.0 { Union::Array(a, ..) => Ok(*a), #[cfg(not(feature = "no_closure"))] - Union::Shared(ref cell, ..) => { - let value = crate::func::locked_read(cell); - - match value.0 { - Union::Array(ref a, ..) => Ok(a.as_ref().clone()), - _ => Err((*value).type_name()), - } - } + Union::Shared(ref cell, ..) => match crate::func::locked_read(cell).0 { + Union::Array(ref a, ..) => Ok(a.as_ref().clone()), + _ => Err(cell.type_name()), + }, _ => Err(self.type_name()), } } @@ -1973,12 +1991,12 @@ impl Dynamic { v.try_cast::().ok_or(typ) }) .collect(), - Union::Blob(..) if TypeId::of::() == TypeId::of::() => Ok(self.cast::>()), + Union::Blob(b, ..) if TypeId::of::() == TypeId::of::() => { + Ok(reify!(*b => Vec)) + } #[cfg(not(feature = "no_closure"))] Union::Shared(ref cell, ..) => { - let value = crate::func::locked_read(cell); - - match value.0 { + match crate::func::locked_read(cell).0 { Union::Array(ref a, ..) => { a.iter() .map(|v| { @@ -1996,10 +2014,10 @@ impl Dynamic { }) .collect() } - Union::Blob(..) if TypeId::of::() == TypeId::of::() => { - Ok((*value).clone().cast::>()) + Union::Blob(ref b, ..) if TypeId::of::() == TypeId::of::() => { + Ok(reify!(b.clone() => Vec)) } - _ => Err((*value).type_name()), + _ => Err(cell.type_name()), } } _ => Err(self.type_name()), @@ -2013,16 +2031,12 @@ impl Dynamic { #[inline(always)] pub fn into_blob(self) -> Result { match self.0 { - Union::Blob(a, ..) => Ok(*a), + Union::Blob(b, ..) => Ok(*b), #[cfg(not(feature = "no_closure"))] - Union::Shared(ref cell, ..) => { - let value = crate::func::locked_read(cell); - - match value.0 { - Union::Blob(ref a, ..) => Ok(a.as_ref().clone()), - _ => Err((*value).type_name()), - } - } + Union::Shared(ref cell, ..) => match crate::func::locked_read(cell).0 { + Union::Blob(ref b, ..) => Ok(b.as_ref().clone()), + _ => Err(cell.type_name()), + }, _ => Err(self.type_name()), } }