rune: Do not trace return value (causes panic on Result:Err) #748
Conversation
|
Thanks! Wouldn't fixing the |
|
Hm, fair point, I work primarily on 0.13.x, not main. So that variant is more well tested. However I don't think it would currently crash on main. But if you somehow manage to return a moved value it would. Not sure if that is possible. |
Instead attempt to print *something*
|
Do you want to remove the tracing change now? It shouldn't crash without that any more, but I admit to not having tested that yet. Even if it doesn't crash, how does every return attempting to format the value affect the performance? |
It should only affect performance if tracing is enabled. Otherwise it should be negligible. Tracing can also be statically compiled away with feature flags if we want to maximize performance. |
| @@ -2137,7 +2137,15 @@ impl fmt::Debug for Value { | |||
| let mut o = Formatter::new(); | |||
|
|
|||
| if self.string_debug(&mut o).is_err() { | |||
There was a problem hiding this comment.
Might be useful to include this error as well?
This is the fix for #747 on the main branch (removal of instrumenting the return value). I left the instrument itself in, though this could be removed instead if it isn't useful to keep that around.