FnMut debug handler #1696
FnMut debug handler #1696
Conversation
| // Test with | ||
| // cargo integration-test debug_timing -- --nocapture | ||
| #[test] | ||
| fn debug_timing() { |
Perhaps related to the removed comment about accessing referenced data in the debug implementation without cloning it? |
Bring back lifetime
| @@ -447,7 +447,7 @@ pub fn do_debug<A: BackendApi + 'static, S: Storage + 'static, Q: Querier + 'sta | |||
| let message_data = read_region(&data.memory(&mut store), message_ptr, MAX_LENGTH_DEBUG)?; | |||
| let msg = String::from_utf8_lossy(&message_data); | |||
| let gas_remaining = data.get_gas_left(&mut store); | |||
| (*debug_handler)( | |||
| debug_handler.borrow_mut()( | |||
There was a problem hiding this comment.
Can the same be achieved with https://doc.rust-lang.org/std/rc/struct.Rc.html#method.get_mut directly? The Rc<RefCell< feels redundant. Not sure, but would one of the two work?
# either
debug_handler: Option<Rc<DebugHandlerFn>>,
# or
debug_handler: Option<RefCell<DebugHandlerFn>>,
But I might be wrong about this
There was a problem hiding this comment.
After double-checking, I believe we need some kind of pointer around the RefCell, because otherwise the type is unsized (internally it contains a Cell, which directly contains the DebugHandlerFn, which has no fixed size).
RefCell also cannot be cloned (at least without cloning the contained thing).
We also cannot use the Rc::get_mut function, because that returns None if we have multiple Rcs of the debug handler. We always keep one Rc in the context data and clone it in order to use it in this line, so we always have multiple Rcs here. Since this would allow holding an immutable and mutable borrow at the same time.
There was a problem hiding this comment.
We also cannot use the
Rc::get_mutfunction, because that returnsNoneif we have multipleRcs of the debug handler. We always keep oneRcin the context data and clone it in order to use it in this line, so we always have multipleRcs here. Since this would allow holding an immutable and mutable borrow at the same time.
Oh, right, excllent. To rephrase what you are saying, the reference count must be 1 in order to get mutable access. This is what the code of Rc::get_mut checks:
fn is_unique(this: &Self) -> bool {
Rc::weak_count(this) == 0 && Rc::strong_count(this) == 1
}
closes #1673
Not exactly sure why, but I had to remove the lifetime from DebugInfo to get this to work (otherwise I always get lifetime errors when adding the RefCell).