Add generic interface for conditionally halting the interpreter #1
Conversation
Signed-off-by: Johannes Löthberg <johannes@kyriasis.com>
kyrias
force-pushed
the
interpreter-limiter
branch
from
f9d0718 to
c4b4815
Compare
|
Though thinking about this a bit more, it would probably be nice if the limiter could return more info to the user. The |
vstroebel
left a comment
vstroebel
left a comment
There was a problem hiding this comment.
Adding more ways to debug the interpreter is a good idea.
IMHO the current way the Limiter is implemented is a little bit too limited 😆
Please see my comments.
| /// The interpreter's configured [`Limiter`][crate::limiters::Limiter] triggered | ||
| LimiterTriggered { | ||
| span: Range<usize>, | ||
| }, |
There was a problem hiding this comment.
Adding a message: String to the variant would allow the Limiter to return more useful information
| mod backends; | ||
| mod errors; | ||
| mod ir; | ||
| pub mod limiters; |
There was a problem hiding this comment.
As of now all modules are implementation details and all types are explicitly exported.
Please remove the pub and add an explicit export to the pub use lines below.
| if let LimiterResult::Halt = self.limiter.execute_op() { | ||
| return Err(RuntimeError::LimiterTriggered { | ||
| span: span.clone(), | ||
| }); | ||
| } |
There was a problem hiding this comment.
Instead of returning LimiterResult it might be better if the limiter returns an Result<(), RuntimeError>.
this would allow the limiter to control the LimiterTriggered error. In addition i think that passing more informations to Limiter::execute_op would be usefull.
Maybe soming like
fn execute_op(&mut self, span: &Range<usize>, op: &Op, heap: &[u8], pointer: usize) -> Result<(), RuntimeError>
2 participants
I was considering that it might sometimes be useful to get the actual op executed as well, but since those structs aren't public and I don't actually have a need for that right now I decided to skip that.