Improve the situation with numerical conversion

This is a somewhat involved change with two breaking API changes:

1) Lua::coerce_xxx methods now return Option (this is easier and faster than
dealing with Result)
2) rlua numeric conversions now allow more loss of precision
conversions (e.g. 1.5f32 to 1i32)

The logic for the first breaking change is that mostly the coerce methods are
probably used internally, and they make sense as low-level fallible casts and
are now used as such, and there's no reason to confuse things with a Result with
a large error type and force the user to match on the error which will hopefully
only be FromLuaConversionError anyway.

The logic for the second change is that it matches the behavior of
num_traits::cast, and is more consistent in that *some* loss of precision
conversions were previously allowed (e.g. f64 to f32).

The problem is that now, Lua::coerce_integer and Lua::unpack::<i64> have
different behavior when given, for example, the number 1.5.  I still think this
is the best option, though, because the Lua::coerce_xxx methods represent how
Lua works internally and the standard C API cast functions that Lua provides,
and the ToLua / FromLua code represents the most common form of fallible Rust
numeric conversion.

I could revert this change and turn `Lua::eval::<i64>("1.5", None)` back into an
error, but it seems inconsistent to allow f64 -> f32 loss of precision but not
f64 -> i64 loss of precision.
This commit is contained in:
kyren 2018-09-26 20:41:07 -04:00
parent c3d0110722
commit 58ce05ff9a
3 changed files with 124 additions and 68 deletions

View File

@ -34,7 +34,13 @@ impl<'lua> ToLua<'lua> for String<'lua> {
impl<'lua> FromLua<'lua> for String<'lua> { impl<'lua> FromLua<'lua> for String<'lua> {
fn from_lua(value: Value<'lua>, lua: &'lua Lua) -> Result<String<'lua>> { fn from_lua(value: Value<'lua>, lua: &'lua Lua) -> Result<String<'lua>> {
let ty = value.type_name();
lua.coerce_string(value) lua.coerce_string(value)
.ok_or_else(|| Error::FromLuaConversionError {
from: ty,
to: "String",
message: Some("expected string or number".to_string()),
})
} }
} }
@ -145,8 +151,8 @@ impl<'lua> FromLua<'lua> for Error {
Value::Error(err) => Ok(err), Value::Error(err) => Ok(err),
val => Ok(Error::RuntimeError( val => Ok(Error::RuntimeError(
lua.coerce_string(val) lua.coerce_string(val)
.and_then(|s| Ok(s.to_str()?.to_owned())) .and_then(|s| Some(s.to_str().ok()?.to_owned()))
.unwrap_or_else(|_| "<unprintable error>".to_owned()), .unwrap_or_else(|| "<unprintable error>".to_owned()),
)), )),
} }
} }
@ -195,7 +201,15 @@ impl<'lua> ToLua<'lua> for StdString {
impl<'lua> FromLua<'lua> for StdString { impl<'lua> FromLua<'lua> for StdString {
fn from_lua(value: Value<'lua>, lua: &'lua Lua) -> Result<Self> { fn from_lua(value: Value<'lua>, lua: &'lua Lua) -> Result<Self> {
Ok(lua.coerce_string(value)?.to_str()?.to_owned()) let ty = value.type_name();
Ok(lua
.coerce_string(value)
.ok_or_else(|| Error::FromLuaConversionError {
from: ty,
to: "String",
message: Some("expected string or number".to_string()),
})?.to_str()?
.to_owned())
} }
} }
@ -209,21 +223,39 @@ macro_rules! lua_convert_int {
($x:ty) => { ($x:ty) => {
impl<'lua> ToLua<'lua> for $x { impl<'lua> ToLua<'lua> for $x {
fn to_lua(self, _: &'lua Lua) -> Result<Value<'lua>> { fn to_lua(self, _: &'lua Lua) -> Result<Value<'lua>> {
cast(self) if let Some(i) = cast(self) {
.ok_or_else(|| Error::ToLuaConversionError { Ok(Value::Integer(i))
from: stringify!($x), } else {
to: "integer", cast(self)
message: Some("integer out of range".to_owned()), .ok_or_else(|| Error::ToLuaConversionError {
}).map(Value::Integer) from: stringify!($x),
to: "number",
message: Some("out of range".to_owned()),
}).map(Value::Number)
}
} }
} }
impl<'lua> FromLua<'lua> for $x { impl<'lua> FromLua<'lua> for $x {
fn from_lua(value: Value<'lua>, lua: &'lua Lua) -> Result<Self> { fn from_lua(value: Value<'lua>, lua: &'lua Lua) -> Result<Self> {
cast(lua.coerce_integer(value)?).ok_or_else(|| Error::FromLuaConversionError { let ty = value.type_name();
from: "integer", (if let Some(i) = lua.coerce_integer(value.clone()) {
cast(i)
} else {
cast(
lua.coerce_number(value)
.ok_or_else(|| Error::FromLuaConversionError {
from: ty,
to: stringify!($x),
message: Some(
"expected number or string coercible to number".to_string(),
),
})?,
)
}).ok_or_else(|| Error::FromLuaConversionError {
from: ty,
to: stringify!($x), to: stringify!($x),
message: Some("integer out of range".to_owned()), message: Some("out of range".to_owned()),
}) })
} }
} }
@ -253,7 +285,19 @@ macro_rules! lua_convert_float {
impl<'lua> FromLua<'lua> for $x { impl<'lua> FromLua<'lua> for $x {
fn from_lua(value: Value<'lua>, lua: &'lua Lua) -> Result<Self> { fn from_lua(value: Value<'lua>, lua: &'lua Lua) -> Result<Self> {
Ok(lua.coerce_number(value)? as $x) let ty = value.type_name();
lua.coerce_number(value)
.ok_or_else(|| Error::FromLuaConversionError {
from: ty,
to: stringify!($x),
message: Some("expected number or string coercible to number".to_string()),
}).and_then(|n| {
cast(n).ok_or_else(|| Error::FromLuaConversionError {
from: ty,
to: stringify!($x),
message: Some("number out of range".to_string()),
})
})
} }
} }
}; };

View File

@ -356,84 +356,73 @@ impl Lua {
r r
} }
/// Coerces a Lua value to a string. /// Attempts to coerce a Lua value into a String in a manner consistent with Lua's internal
/// behavior.
/// ///
/// The value must be a string (in which case this is a no-op) or a number. /// To succeed, the value must be a string (in which case this is a no-op), an integer, or a
pub fn coerce_string<'lua>(&'lua self, v: Value<'lua>) -> Result<String<'lua>> { /// number.
pub fn coerce_string<'lua>(&'lua self, v: Value<'lua>) -> Option<String<'lua>> {
match v { match v {
Value::String(s) => Ok(s), Value::String(s) => Some(s),
v => unsafe { v => unsafe {
let _sg = StackGuard::new(self.state); let _sg = StackGuard::new(self.state);
assert_stack(self.state, 4); assert_stack(self.state, 4);
let ty = v.type_name();
self.push_value(v); self.push_value(v);
let s = let s = gc_guard(self.state, || ffi::lua_tostring(self.state, -1));
protect_lua_closure(self.state, 1, 1, |state| ffi::lua_tostring(state, -1))?;
if s.is_null() { if s.is_null() {
Err(Error::FromLuaConversionError { None
from: ty,
to: "String",
message: Some("expected string or number".to_string()),
})
} else { } else {
Ok(String(self.pop_ref())) Some(String(self.pop_ref()))
} }
}, },
} }
} }
/// Coerces a Lua value to an integer. /// Attempts to coerce a Lua value into an integer in a manner consistent with Lua's internal
/// behavior.
/// ///
/// The value must be an integer, or a floating point number, or a string that can be converted /// To succeed, the value must be an integer, a floating point number that has an exact
/// to an integer. Refer to the Lua manual for details. /// representation as an integer, or a string that can be converted to an integer. Refer to the
pub fn coerce_integer(&self, v: Value) -> Result<Integer> { /// Lua manual for details.
pub fn coerce_integer(&self, v: Value) -> Option<Integer> {
match v { match v {
Value::Integer(i) => Ok(i), Value::Integer(i) => Some(i),
v => unsafe { v => unsafe {
let _sg = StackGuard::new(self.state); let _sg = StackGuard::new(self.state);
assert_stack(self.state, 2); assert_stack(self.state, 2);
let ty = v.type_name();
self.push_value(v); self.push_value(v);
let mut isint = 0; let mut isint = 0;
let i = ffi::lua_tointegerx(self.state, -1, &mut isint); let i = ffi::lua_tointegerx(self.state, -1, &mut isint);
if isint == 0 { if isint == 0 {
Err(Error::FromLuaConversionError { None
from: ty,
to: "integer",
message: None,
})
} else { } else {
Ok(i) Some(i)
} }
}, },
} }
} }
/// Coerce a Lua value to a number. /// Attempts to coerce a Lua value into a Number in a manner consistent with Lua's internal
/// behavior.
/// ///
/// The value must be a number or a string that can be converted to a number. Refer to the Lua /// To succeed, the value must be a number or a string that can be converted to a number. Refer
/// manual for details. /// to the Lua manual for details.
pub fn coerce_number(&self, v: Value) -> Result<Number> { pub fn coerce_number(&self, v: Value) -> Option<Number> {
match v { match v {
Value::Number(n) => Ok(n), Value::Number(n) => Some(n),
v => unsafe { v => unsafe {
let _sg = StackGuard::new(self.state); let _sg = StackGuard::new(self.state);
assert_stack(self.state, 2); assert_stack(self.state, 2);
let ty = v.type_name();
self.push_value(v); self.push_value(v);
let mut isnum = 0; let mut isnum = 0;
let n = ffi::lua_tonumberx(self.state, -1, &mut isnum); let n = ffi::lua_tonumberx(self.state, -1, &mut isnum);
if isnum == 0 { if isnum == 0 {
Err(Error::FromLuaConversionError { None
from: ty,
to: "number",
message: Some("number or string coercible to number".to_string()),
})
} else { } else {
Ok(n) Some(n)
} }
}, },
} }

View File

@ -4,7 +4,7 @@ extern crate rlua;
use std::iter::FromIterator; use std::iter::FromIterator;
use std::panic::catch_unwind; use std::panic::catch_unwind;
use std::sync::Arc; use std::sync::Arc;
use std::{error, fmt}; use std::{error, f32, f64, fmt};
use failure::err_msg; use failure::err_msg;
use rlua::{ use rlua::{
@ -354,29 +354,52 @@ fn test_result_conversions() {
#[test] #[test]
fn test_num_conversion() { fn test_num_conversion() {
let lua = Lua::new(); let lua = Lua::new();
let globals = lua.globals();
globals.set("n", "1.0").unwrap(); assert_eq!(
assert_eq!(globals.get::<_, i64>("n").unwrap(), 1); lua.coerce_integer(Value::String(lua.create_string("1").unwrap())),
assert_eq!(globals.get::<_, f64>("n").unwrap(), 1.0); Some(1)
assert_eq!(globals.get::<_, String>("n").unwrap(), "1.0"); );
assert_eq!(
lua.coerce_integer(Value::String(lua.create_string("1.0").unwrap())),
Some(1)
);
assert_eq!(
lua.coerce_integer(Value::String(lua.create_string("1.5").unwrap())),
None
);
globals.set("n", "1.5").unwrap(); assert_eq!(
assert!(globals.get::<_, i64>("n").is_err()); lua.coerce_number(Value::String(lua.create_string("1").unwrap())),
assert_eq!(globals.get::<_, f64>("n").unwrap(), 1.5); Some(1.0)
assert_eq!(globals.get::<_, String>("n").unwrap(), "1.5"); );
assert_eq!(
lua.coerce_number(Value::String(lua.create_string("1.0").unwrap())),
Some(1.0)
);
assert_eq!(
lua.coerce_number(Value::String(lua.create_string("1.5").unwrap())),
Some(1.5)
);
globals.set("n", 1.5).unwrap(); assert_eq!(lua.eval::<i64>("1.0", None).unwrap(), 1);
assert!(globals.get::<_, i64>("n").is_err()); assert_eq!(lua.eval::<f64>("1.0", None).unwrap(), 1.0);
assert_eq!(globals.get::<_, f64>("n").unwrap(), 1.5); assert_eq!(lua.eval::<String>("1.0", None).unwrap(), "1.0");
assert_eq!(globals.get::<_, String>("n").unwrap(), "1.5");
lua.exec::<()>("a = math.huge", None).unwrap(); assert_eq!(lua.eval::<i64>("1.5", None).unwrap(), 1);
assert!(globals.get::<_, i64>("n").is_err()); assert_eq!(lua.eval::<f64>("1.5", None).unwrap(), 1.5);
assert_eq!(lua.eval::<String>("1.5", None).unwrap(), "1.5");
assert!(lua.eval::<u64>("-1", None).is_err()); assert!(lua.eval::<u64>("-1", None).is_err());
assert!(lua.eval::<i64>("-1", None).is_ok()); assert_eq!(lua.eval::<i64>("-1", None).unwrap(), -1);
assert!(lua.pack(1u128 << 64).is_err());
assert!(lua.unpack::<u64>(lua.pack(1u128 << 64).unwrap()).is_err());
assert!(lua.eval::<i64>("math.huge", None).is_err());
assert_eq!(
lua.unpack::<f64>(lua.pack(f32::MAX).unwrap()).unwrap(),
f32::MAX as f64
);
assert!(lua.unpack::<f32>(lua.pack(f64::MAX).unwrap()).is_err());
} }
#[test] #[test]