From 2bbb203050b281d0f83d457fca60129d0003ede6 Mon Sep 17 00:00:00 2001 From: Michael Pfaff Date: Thu, 6 Jul 2023 13:32:11 -0400 Subject: [PATCH] Revert "Make Debug interface more user friendly" This reverts commit b3b8d794460752b812375f881a6e01fb73db4012. --- src/function.rs | 51 +++++++++++++++++++++++------------------------ src/hook.rs | 48 ++++++++++++++++---------------------------- src/util/mod.rs | 21 +++---------------- tests/function.rs | 34 +++++++++++++++---------------- tests/hooks.rs | 16 +++++++++++---- tests/tests.rs | 4 ++-- 6 files changed, 76 insertions(+), 98 deletions(-) diff --git a/src/function.rs b/src/function.rs index 59a66ac..39f9c13 100644 --- a/src/function.rs +++ b/src/function.rs @@ -10,8 +10,7 @@ use crate::memory::MemoryState; use crate::table::Table; use crate::types::{Callback, LuaRef, MaybeSend}; use crate::util::{ - assert_stack, check_stack, error_traceback, linenumber_to_usize, pop_error, ptr_to_lossy_str, - ptr_to_str, StackGuard, + assert_stack, check_stack, error_traceback, pop_error, ptr_to_cstr_bytes, StackGuard, }; use crate::value::{FromLuaMulti, IntoLua, IntoLuaMulti}; @@ -53,22 +52,24 @@ impl OwnedFunction { /// [`Lua Debug Interface`]: https://www.lua.org/manual/5.4/manual.html#4.7 #[derive(Clone, Debug)] pub struct FunctionInfo { - /// A (reasonable) name of the function (`None` if the name cannot be found). + /// A (reasonable) name of the function. pub name: Option, - /// Explains the `name` field (can be `global`/`local`/`method`/`field`/`upvalue`/etc). + /// Explains the `name` field ("global", "local", "method", "field", "upvalue", or ""). /// /// Always `None` for Luau. - pub name_what: Option<&'static str>, - /// A string `Lua` if the function is a Lua function, `C` if it is a C function, `main` if it is the main part of a chunk. - pub what: &'static str, - /// Source of the chunk that created the function. - pub source: Option, - /// A "printable" version of `source`, to be used in error messages. - pub short_src: Option, + pub name_what: Option, + /// A string "Lua" if the function is a Lua function, "C" if it is a C function, "main" if it is the main part of a chunk. + pub what: Option, + /// The source of the chunk that created the function. + pub source: Option>, + /// A "printable" version of source, to be used in error messages. + pub short_src: Option>, /// The line number where the definition of the function starts. - pub line_defined: Option, - /// The line number where the definition of the function ends (not set by Luau). - pub last_line_defined: Option, + pub line_defined: i32, + /// The line number where the definition of the function ends. + /// + /// Always `-1` for Luau. + pub last_line_defined: i32, } /// Luau function coverage snapshot. @@ -400,25 +401,23 @@ impl<'lua> Function<'lua> { mlua_assert!(res != 0, "lua_getinfo failed with `>Sn`"); FunctionInfo { - name: ptr_to_lossy_str(ar.name).map(|s| s.into_owned()), + name: ptr_to_cstr_bytes(ar.name).map(|s| String::from_utf8_lossy(s).into_owned()), #[cfg(not(feature = "luau"))] - name_what: match ptr_to_str(ar.namewhat) { - Some("") => None, - val => val, - }, + name_what: ptr_to_cstr_bytes(ar.namewhat) + .map(|s| String::from_utf8_lossy(s).into_owned()), #[cfg(feature = "luau")] name_what: None, - what: ptr_to_str(ar.what).unwrap_or("main"), - source: ptr_to_lossy_str(ar.source).map(|s| s.into_owned()), + what: ptr_to_cstr_bytes(ar.what).map(|s| String::from_utf8_lossy(s).into_owned()), + source: ptr_to_cstr_bytes(ar.source).map(|s| s.to_vec()), #[cfg(not(feature = "luau"))] - short_src: ptr_to_lossy_str(ar.short_src.as_ptr()).map(|s| s.into_owned()), + short_src: ptr_to_cstr_bytes(ar.short_src.as_ptr()).map(|s| s.to_vec()), #[cfg(feature = "luau")] - short_src: ptr_to_lossy_str(ar.short_src).map(|s| s.into_owned()), - line_defined: linenumber_to_usize(ar.linedefined), + short_src: ptr_to_cstr_bytes(ar.short_src).map(|s| s.to_vec()), + line_defined: ar.linedefined, #[cfg(not(feature = "luau"))] - last_line_defined: linenumber_to_usize(ar.lastlinedefined), + last_line_defined: ar.lastlinedefined, #[cfg(feature = "luau")] - last_line_defined: None, + last_line_defined: -1, } } } diff --git a/src/hook.rs b/src/hook.rs index 443facd..14871b7 100644 --- a/src/hook.rs +++ b/src/hook.rs @@ -1,4 +1,3 @@ -use std::borrow::Cow; use std::cell::UnsafeCell; #[cfg(not(feature = "luau"))] use std::ops::{BitOr, BitOrAssign}; @@ -7,7 +6,7 @@ use std::os::raw::c_int; use ffi::lua_Debug; use crate::lua::Lua; -use crate::util::{linenumber_to_usize, ptr_to_lossy_str, ptr_to_str}; +use crate::util::ptr_to_cstr_bytes; /// Contains information about currently executing Lua code. /// @@ -79,12 +78,9 @@ impl<'lua> Debug<'lua> { ); DebugNames { - name: ptr_to_lossy_str((*self.ar.get()).name), + name: ptr_to_cstr_bytes((*self.ar.get()).name), #[cfg(not(feature = "luau"))] - name_what: match ptr_to_str((*self.ar.get()).namewhat) { - Some("") => None, - val => val, - }, + name_what: ptr_to_cstr_bytes((*self.ar.get()).namewhat), #[cfg(feature = "luau")] name_what: None, } @@ -106,17 +102,15 @@ impl<'lua> Debug<'lua> { ); DebugSource { - source: ptr_to_lossy_str((*self.ar.get()).source), + source: ptr_to_cstr_bytes((*self.ar.get()).source), #[cfg(not(feature = "luau"))] - short_src: ptr_to_lossy_str((*self.ar.get()).short_src.as_ptr()), + short_src: ptr_to_cstr_bytes((*self.ar.get()).short_src.as_ptr()), #[cfg(feature = "luau")] - short_src: ptr_to_lossy_str((*self.ar.get()).short_src), - line_defined: linenumber_to_usize((*self.ar.get()).linedefined), + short_src: ptr_to_cstr_bytes((*self.ar.get()).short_src), + line_defined: (*self.ar.get()).linedefined, #[cfg(not(feature = "luau"))] - last_line_defined: linenumber_to_usize((*self.ar.get()).lastlinedefined), - #[cfg(feature = "luau")] - last_line_defined: None, - what: ptr_to_str((*self.ar.get()).what).unwrap_or("main"), + last_line_defined: (*self.ar.get()).lastlinedefined, + what: ptr_to_cstr_bytes((*self.ar.get()).what), } } } @@ -216,26 +210,18 @@ pub enum DebugEvent { #[derive(Clone, Debug)] pub struct DebugNames<'a> { - /// A (reasonable) name of the function (`None` if the name cannot be found). - pub name: Option>, - /// Explains the `name` field (can be `global`/`local`/`method`/`field`/`upvalue`/etc). - /// - /// Always `None` for Luau. - pub name_what: Option<&'static str>, + pub name: Option<&'a [u8]>, + pub name_what: Option<&'a [u8]>, } #[derive(Clone, Debug)] pub struct DebugSource<'a> { - /// Source of the chunk that created the function. - pub source: Option>, - /// A "printable" version of `source`, to be used in error messages. - pub short_src: Option>, - /// The line number where the definition of the function starts. - pub line_defined: Option, - /// The line number where the definition of the function ends (not set by Luau). - pub last_line_defined: Option, - /// A string `Lua` if the function is a Lua function, `C` if it is a C function, `main` if it is the main part of a chunk. - pub what: &'static str, + pub source: Option<&'a [u8]>, + pub short_src: Option<&'a [u8]>, + pub line_defined: i32, + #[cfg(not(feature = "luau"))] + pub last_line_defined: i32, + pub what: Option<&'a [u8]>, } #[derive(Copy, Clone, Debug)] diff --git a/src/util/mod.rs b/src/util/mod.rs index e806cc7..14a8e97 100644 --- a/src/util/mod.rs +++ b/src/util/mod.rs @@ -1,12 +1,11 @@ use std::any::{Any, TypeId}; -use std::borrow::Cow; use std::ffi::CStr; use std::fmt::Write; use std::mem::MaybeUninit; use std::os::raw::{c_char, c_int, c_void}; use std::panic::{catch_unwind, resume_unwind, AssertUnwindSafe}; use std::sync::Arc; -use std::{mem, ptr, slice, str}; +use std::{mem, ptr, slice}; use once_cell::sync::Lazy; use rustc_hash::FxHashMap; @@ -1066,25 +1065,11 @@ pub(crate) unsafe fn get_destructed_userdata_metatable(state: *mut ffi::lua_Stat ffi::lua_rawgetp(state, ffi::LUA_REGISTRYINDEX, key); } -pub(crate) unsafe fn ptr_to_str<'a>(input: *const c_char) -> Option<&'a str> { +pub(crate) unsafe fn ptr_to_cstr_bytes<'a>(input: *const c_char) -> Option<&'a [u8]> { if input.is_null() { return None; } - str::from_utf8(CStr::from_ptr(input).to_bytes()).ok() -} - -pub(crate) unsafe fn ptr_to_lossy_str<'a>(input: *const c_char) -> Option> { - if input.is_null() { - return None; - } - Some(String::from_utf8_lossy(CStr::from_ptr(input).to_bytes())) -} - -pub(crate) fn linenumber_to_usize(n: c_int) -> Option { - match n { - n if n < 0 => None, - n => Some(n as usize), - } + Some(CStr::from_ptr(input).to_bytes()) } static DESTRUCTED_USERDATA_METATABLE: u8 = 0; diff --git a/tests/function.rs b/tests/function.rs index 83325a6..234227e 100644 --- a/tests/function.rs +++ b/tests/function.rs @@ -196,37 +196,37 @@ fn test_function_info() -> Result<()> { let function1_info = function1.info(); #[cfg(feature = "luau")] assert_eq!(function1_info.name.as_deref(), Some("function1")); - assert_eq!(function1_info.source.as_deref(), Some("source1")); - assert_eq!(function1_info.line_defined, Some(2)); + assert_eq!(function1_info.source.as_deref(), Some(b"source1".as_ref())); + assert_eq!(function1_info.line_defined, 2); #[cfg(not(feature = "luau"))] - assert_eq!(function1_info.last_line_defined, Some(4)); + assert_eq!(function1_info.last_line_defined, 4); #[cfg(feature = "luau")] - assert_eq!(function1_info.last_line_defined, None); - assert_eq!(function1_info.what, "Lua"); + assert_eq!(function1_info.last_line_defined, -1); + assert_eq!(function1_info.what.as_deref(), Some("Lua")); let function2_info = function2.info(); assert_eq!(function2_info.name, None); - assert_eq!(function2_info.source.as_deref(), Some("source1")); - assert_eq!(function2_info.line_defined, Some(3)); + assert_eq!(function2_info.source.as_deref(), Some(b"source1".as_ref())); + assert_eq!(function2_info.line_defined, 3); #[cfg(not(feature = "luau"))] - assert_eq!(function2_info.last_line_defined, Some(3)); + assert_eq!(function2_info.last_line_defined, 3); #[cfg(feature = "luau")] - assert_eq!(function2_info.last_line_defined, None); - assert_eq!(function2_info.what, "Lua"); + assert_eq!(function2_info.last_line_defined, -1); + assert_eq!(function2_info.what.as_deref(), Some("Lua")); let function3_info = function3.info(); assert_eq!(function3_info.name, None); - assert_eq!(function3_info.source.as_deref(), Some("=[C]")); - assert_eq!(function3_info.line_defined, None); - assert_eq!(function3_info.last_line_defined, None); - assert_eq!(function3_info.what, "C"); + assert_eq!(function3_info.source.as_deref(), Some(b"=[C]".as_ref())); + assert_eq!(function3_info.line_defined, -1); + assert_eq!(function3_info.last_line_defined, -1); + assert_eq!(function3_info.what.as_deref(), Some("C")); let print_info = globals.get::<_, Function>("print")?.info(); #[cfg(feature = "luau")] assert_eq!(print_info.name.as_deref(), Some("print")); - assert_eq!(print_info.source.as_deref(), Some("=[C]")); - assert_eq!(print_info.what, "C"); - assert_eq!(print_info.line_defined, None); + assert_eq!(print_info.source.as_deref(), Some(b"=[C]".as_ref())); + assert_eq!(print_info.what.as_deref(), Some("C")); + assert_eq!(print_info.line_defined, -1); Ok(()) } diff --git a/tests/hooks.rs b/tests/hooks.rs index 0f02f22..f9a7023 100644 --- a/tests/hooks.rs +++ b/tests/hooks.rs @@ -2,6 +2,7 @@ use std::cell::RefCell; use std::ops::Deref; +use std::str; use std::sync::atomic::{AtomicI64, Ordering}; use std::sync::{Arc, Mutex}; @@ -60,8 +61,9 @@ fn test_function_calls() -> Result<()> { assert_eq!(debug.event(), DebugEvent::Call); let names = debug.names(); let source = debug.source(); - let name = names.name.map(|s| s.into_owned()); - hook_output.lock().unwrap().push((name, source.what)); + let name = names.name.map(|s| str::from_utf8(s).unwrap().to_owned()); + let what = source.what.map(|s| str::from_utf8(s).unwrap().to_owned()); + hook_output.lock().unwrap().push((name, what)); Ok(()) })?; @@ -78,12 +80,18 @@ fn test_function_calls() -> Result<()> { if cfg!(feature = "luajit") && lua.load("jit.version_num").eval::()? >= 20100 { assert_eq!( *output, - vec![(None, "main"), (Some("len".to_string()), "Lua")] + vec![ + (None, Some("main".to_string())), + (Some("len".to_string()), Some("Lua".to_string())) + ] ); } else { assert_eq!( *output, - vec![(None, "main"), (Some("len".to_string()), "C")] + vec![ + (None, Some("main".to_string())), + (Some("len".to_string()), Some("C".to_string())) + ] ); } diff --git a/tests/tests.rs b/tests/tests.rs index a60d7b8..4add2a3 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -1226,8 +1226,8 @@ fn test_inspect_stack() -> Result<()> { let logline = lua.create_function(|lua, msg: StdString| { let debug = lua.inspect_stack(1).unwrap(); // caller - let source = debug.source().short_src; - let source = source.as_deref().unwrap_or("?"); + let source = debug.source().short_src.map(core::str::from_utf8); + let source = source.transpose().unwrap().unwrap_or("?"); let line = debug.curr_line(); Ok(format!("{}:{} {}", source, line, msg)) })?;