diff --git a/c/src/metta.rs b/c/src/metta.rs index ad59ce2ac..f4e88ffbb 100644 --- a/c/src/metta.rs +++ b/c/src/metta.rs @@ -233,6 +233,7 @@ pub extern "C" fn sexpr_parser_free(parser: sexpr_parser_t) { /// atom if parsing is finished, or an error expression atom if a parse error occurred. /// @note The caller must take ownership responsibility for the returned `atom_t`, and ultimately free /// it with `atom_free()` or pass it to another function that takes ownership responsibility +/// @note If this function encounters an error, the error may be accessed with `sexpr_parser_err_str()` /// #[no_mangle] pub extern "C" fn sexpr_parser_parse( @@ -259,12 +260,12 @@ pub extern "C" fn sexpr_parser_parse( /// @return A pointer to the C-string containing the parse error that occurred, or NULL if no /// parse error occurred /// @warning The returned pointer should NOT be freed. It must never be accessed after the -/// sexpr_parser_t has been freed, or any subsequent `sexpr_parser_parse` or +/// sexpr_parser_t has been freed, or any subsequent call to `sexpr_parser_parse` or /// `sexpr_parser_parse_to_syntax_tree` has been made. /// #[no_mangle] pub extern "C" fn sexpr_parser_err_str( - parser: *mut sexpr_parser_t) -> *const c_char { + parser: *const sexpr_parser_t) -> *const c_char { let parser = unsafe{ &*parser }; parser.err_string } @@ -733,21 +734,36 @@ pub extern "C" fn step_get_result(step: step_result_t, pub struct metta_t { /// Internal. Should not be accessed directly metta: *mut RustMettaInterpreter, + err_string: *mut c_char, +} + +impl metta_t { + fn free_err_string(&mut self) { + if !self.err_string.is_null() { + let string = unsafe{ std::ffi::CString::from_raw(self.err_string) }; + drop(string); + self.err_string = core::ptr::null_mut(); + } + } } struct RustMettaInterpreter(Metta); impl From for metta_t { fn from(metta: Metta) -> Self { - Self{ metta: Box::into_raw(Box::new(RustMettaInterpreter(metta))) } + Self{ + metta: Box::into_raw(Box::new(RustMettaInterpreter(metta))), + err_string: core::ptr::null_mut(), + } } } impl metta_t { fn borrow(&self) -> &Metta { - unsafe{ &(&*self.metta).0 } + &unsafe{ &*self.metta }.0 } - fn into_inner(self) -> Metta { + fn into_inner(mut self) -> Metta { + self.free_err_string(); unsafe{ Box::from_raw(self.metta).0 } } } @@ -794,7 +810,10 @@ pub extern "C" fn metta_new_with_space_environment_and_stdlib(space: *mut space_ }; let metta = Metta::new_with_stdlib_loader(|metta| { - let mut metta = metta_t{metta: (metta as *const Metta).cast_mut().cast()}; + let mut metta = metta_t{ + metta: (metta as *const Metta).cast_mut().cast(), + err_string: core::ptr::null_mut(), + }; callback(&mut metta, context); }, Some(dyn_space.clone()), env_builder); metta.into() @@ -846,6 +865,22 @@ pub extern "C" fn metta_free(metta: metta_t) { drop(metta); } +/// @brief Returns the error string associated with the last `metta_run`, `metta_evaluate_atom`, +/// or `metta_load_module` call +/// @ingroup interpreter_group +/// @param[in] metta A pointer to the MeTTa handle +/// @return A pointer to the C-string containing the error that occurred, or NULL if no +/// error occurred +/// @warning The returned pointer should NOT be freed. It must never be accessed after the +/// metta_t has been freed, or any subsequent call to `metta_run`, `metta_evaluate_atom`, or +/// `metta_load_module` has been made. +/// +#[no_mangle] +pub extern "C" fn metta_err_str(metta: *const metta_t) -> *const c_char { + let metta = unsafe{ &*metta }; + metta.err_string +} + /// @brief Compares two `metta_t` handles to test whether the referenced MeTTa runner is the same /// @ingroup interpreter_group /// @param[in] a A pointer to the first Interpreter handle @@ -925,18 +960,75 @@ pub extern "C" fn metta_tokenizer(metta: *mut metta_t) -> tokenizer_t { /// @param[in] parser An S-Expression Parser containing the MeTTa text /// @param[in] callback A function that will be called to provide a vector of atoms produced by the evaluation /// @param[in] context A pointer to a caller-defined structure to facilitate communication with the `callback` function +/// @note If this function encounters an error, the callback will not be called and the error may be accessed with `metta_err_str()` /// @warning Ownership of the provided parser will be taken by this function, so it must not be subsequently accessed /// nor freed. /// #[no_mangle] pub extern "C" fn metta_run(metta: *mut metta_t, parser: sexpr_parser_t, callback: c_atom_vec_callback_t, context: *mut c_void) { - let metta = unsafe{ &*metta }.borrow(); + let metta = unsafe{ &mut *metta }; + metta.free_err_string(); let parser = parser.into_inner(); - let results = metta.run(parser); - // TODO: return erorrs properly after step_get_result() is changed to return errors. - for result in results.expect("Returning errors from C API is not implemented yet") { - return_atoms(&result, callback, context); + let rust_metta = metta.borrow(); + let results = rust_metta.run(parser); + match results { + Ok(results) => { + for result in results { + return_atoms(&result, callback, context); + } + }, + Err(err) => { + let err_cstring = std::ffi::CString::new(err).unwrap(); + metta.err_string = err_cstring.into_raw(); + } + } +} + +/// @brief Runs the MeTTa Interpreter to evaluate an input Atom +/// @ingroup interpreter_group +/// @param[in] metta A pointer to the Interpreter handle +/// @param[in] atom The `atom_t` representing the atom to evaluate +/// @param[in] callback A function that will be called to provide a vector of atoms produced by the evaluation +/// @param[in] context A pointer to a caller-defined structure to facilitate communication with the `callback` function +/// @note If this function encounters an error, the callback will not be called and the error may be accessed with `metta_err_str()` +/// @warning This function takes ownership of the provided `atom_t`, so it must not be subsequently accessed or freed +/// +#[no_mangle] +pub extern "C" fn metta_evaluate_atom(metta: *mut metta_t, atom: atom_t, + callback: c_atom_vec_callback_t, context: *mut c_void) { + let metta = unsafe{ &mut *metta }; + metta.free_err_string(); + let atom = atom.into_inner(); + let rust_metta = metta.borrow(); + let result = rust_metta.evaluate_atom(atom); + match result { + Ok(result) => return_atoms(&result, callback, context), + Err(err) => { + let err_cstring = std::ffi::CString::new(err).unwrap(); + metta.err_string = err_cstring.into_raw(); + } + } +} + +/// @brief Loads a module into a MeTTa interpreter +/// @ingroup interpreter_group +/// @param[in] metta A pointer to the handle specifying the interpreter into which to load the module +/// @param[in] name A C-style string containing the module name +/// @note If this function encounters an error, the error may be accessed with `metta_err_str()` +/// +#[no_mangle] +pub extern "C" fn metta_load_module(metta: *mut metta_t, name: *const c_char) { + let metta = unsafe{ &mut *metta }; + metta.free_err_string(); + let rust_metta = metta.borrow(); + let result = rust_metta.load_module(PathBuf::from(cstr_as_str(name))); + match result { + Ok(()) => {}, + Err(err) => { + let err_cstring = std::ffi::CString::new(err).unwrap(); + metta.err_string = err_cstring.into_raw(); + } } } @@ -951,24 +1043,39 @@ pub extern "C" fn metta_run(metta: *mut metta_t, parser: sexpr_parser_t, pub struct runner_state_t { /// Internal. Should not be accessed directly state: *mut RustRunnerState, + err_string: *mut c_char, } -struct RustRunnerState(RunnerState<'static>); +struct RustRunnerState(RunnerState<'static, 'static>); -impl From> for runner_state_t { - fn from(state: RunnerState<'static>) -> Self { - Self{ state: Box::into_raw(Box::new(RustRunnerState(state))) } +impl runner_state_t { + fn free_err_string(&mut self) { + if !self.err_string.is_null() { + let string = unsafe{ std::ffi::CString::from_raw(self.err_string) }; + drop(string); + self.err_string = core::ptr::null_mut(); + } + } +} + +impl From> for runner_state_t { + fn from(state: RunnerState<'static, 'static>) -> Self { + Self{ + state: Box::into_raw(Box::new(RustRunnerState(state))), + err_string: core::ptr::null_mut(), + } } } impl runner_state_t { - fn into_inner(self) -> RunnerState<'static> { + fn into_inner(mut self) -> RunnerState<'static, 'static> { + self.free_err_string(); unsafe{ Box::from_raw(self.state).0 } } - fn borrow(&self) -> &RunnerState<'static> { + fn borrow(&self) -> &RunnerState<'static, 'static> { &unsafe{ &*(&*self).state }.0 } - fn borrow_mut(&mut self) -> &mut RunnerState<'static> { + fn borrow_mut(&mut self) -> &mut RunnerState<'static, 'static> { &mut unsafe{ &mut *(&*self).state }.0 } } @@ -1017,14 +1124,37 @@ pub extern "C" fn runner_state_free(state: runner_state_t) { drop(state); } +/// @brief Returns the error string associated with the last `runner_state_step` +/// @ingroup interpreter_group +/// @param[in] state A pointer to the runner state +/// @return A pointer to the C-string containing the error that occurred, or NULL if no +/// error occurred +/// @warning The returned pointer should NOT be freed. It must never be accessed after the +/// runner_state_t has been freed, or any subsequent call to `runner_state_step` has been made. +/// +#[no_mangle] +pub extern "C" fn runner_state_err_str(state: *const runner_state_t) -> *const c_char { + let state = unsafe{ &*state }; + state.err_string +} + /// @brief Runs one step of the interpreter /// @ingroup interpreter_group /// @param[in] state A pointer to the in-flight runner state +/// @note If this function encounters an error, the error may be accessed with `runner_state_err_str()` /// #[no_mangle] pub extern "C" fn runner_state_step(state: *mut runner_state_t) { - let state = unsafe{ &mut *state }.borrow_mut(); - state.run_step().unwrap_or_else(|err| panic!("Unhandled MeTTa error: {}", err)); + let state = unsafe{ &mut *state }; + state.free_err_string(); + let rust_state = state.borrow_mut(); + match rust_state.run_step() { + Ok(_) => {}, + Err(err) => { + let err_cstring = std::ffi::CString::new(err).unwrap(); + state.err_string = err_cstring.into_raw(); + } + } } /// @brief Returns whether or not the runner_state_t has completed all outstanding work @@ -1071,37 +1201,6 @@ pub extern "C" fn runner_state_current_results(state: *const runner_state_t, } } -/// @brief Runs the MeTTa Interpreter to evaluate an input Atom -/// @ingroup interpreter_group -/// @param[in] metta A pointer to the Interpreter handle -/// @param[in] atom The `atom_t` representing the atom to evaluate -/// @param[in] callback A function that will be called to provide a vector of atoms produced by the evaluation -/// @param[in] context A pointer to a caller-defined structure to facilitate communication with the `callback` function -/// @warning This function takes ownership of the provided `atom_t`, so it must not be subsequently accessed or freed -/// -#[no_mangle] -pub extern "C" fn metta_evaluate_atom(metta: *mut metta_t, atom: atom_t, - callback: c_atom_vec_callback_t, context: *mut c_void) { - let metta = unsafe{ &*metta }.borrow(); - let atom = atom.into_inner(); - let result = metta.evaluate_atom(atom) - .expect("Returning errors from C API is not implemented yet"); - return_atoms(&result, callback, context); -} - -/// @brief Loads a module into a MeTTa interpreter -/// @ingroup interpreter_group -/// @param[in] metta A pointer to the handle specifying the interpreter into which to load the module -/// @param[in] name A C-style string containing the module name -/// -#[no_mangle] -pub extern "C" fn metta_load_module(metta: *mut metta_t, name: *const c_char) { - let metta = unsafe{ &*metta }.borrow(); - // TODO: return erorrs properly - metta.load_module(PathBuf::from(cstr_as_str(name))) - .expect("Returning errors from C API is not implemented yet"); -} - // =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- // Environment Interface // =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=- diff --git a/c/tests/check_runner.c b/c/tests/check_runner.c index 2a25d7c7e..53214744d 100644 --- a/c/tests/check_runner.c +++ b/c/tests/check_runner.c @@ -54,10 +54,27 @@ START_TEST (test_incremental_runner) } END_TEST +START_TEST (test_runner_errors) +{ + metta_t runner = new_test_metta(); + + sexpr_parser_t parser = sexpr_parser_new("!(+ 1 (+ 2 (+ 3 4))"); + atom_vec_t* results = NULL; + metta_run(&runner, parser, ©_atom_vec, &results); + + //We have a parse error, so the callback should never be called + ck_assert(results == NULL); + ck_assert_str_eq(metta_err_str(&runner), "Unexpected end of expression"); + + metta_free(runner); +} +END_TEST + void init_test(TCase* test_case) { tcase_set_timeout(test_case, 300); //300s = 5min. To test for memory leaks tcase_add_checked_fixture(test_case, setup, teardown); tcase_add_test(test_case, test_incremental_runner); + tcase_add_test(test_case, test_runner_errors); } TEST_MAIN(init_test); diff --git a/lib/src/metta/runner/mod.rs b/lib/src/metta/runner/mod.rs index f11d0e26c..c4da04c9c 100644 --- a/lib/src/metta/runner/mod.rs +++ b/lib/src/metta/runner/mod.rs @@ -57,16 +57,16 @@ enum MettaRunnerMode { TERMINATE, } -pub struct RunnerState<'a> { +pub struct RunnerState<'m, 'i> { mode: MettaRunnerMode, - metta: &'a Metta, - parser: Option>, - atoms: Option<&'a [Atom]>, - interpreter_state: Option>, + metta: &'m Metta, + parser: Option>, + atoms: Option<&'i [Atom]>, + interpreter_state: Option>, results: Vec>, } -impl std::fmt::Debug for RunnerState<'_> { +impl std::fmt::Debug for RunnerState<'_, '_> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("RunnerState") .field("mode", &self.mode) @@ -245,7 +245,7 @@ impl Metta { self.0.settings.borrow().get(key.into()).map(|a| a.to_string()) } - pub fn run<'p, 'a: 'p>(&'a self, parser: SExprParser<'p>) -> Result>, String> { + pub fn run(&self, parser: SExprParser) -> Result>, String> { let state = RunnerState::new_with_parser(self, parser); state.run_to_completion() } @@ -286,8 +286,8 @@ fn wrap_atom_by_metta_interpreter(runner: &Metta, atom: Atom) -> Atom { eval } -impl<'a> RunnerState<'a> { - fn new(metta: &'a Metta) -> Self { +impl<'m, 'i> RunnerState<'m, 'i> { + fn new(metta: &'m Metta) -> Self { Self { metta, mode: MettaRunnerMode::ADD, @@ -298,14 +298,14 @@ impl<'a> RunnerState<'a> { } } /// Returns a new RunnerState, for running code from the [SExprParser] with the specified [Metta] runner - pub fn new_with_parser(metta: &'a Metta, parser: SExprParser<'a>) -> Self { + pub fn new_with_parser(metta: &'m Metta, parser: SExprParser<'i>) -> Self { let mut state = Self::new(metta); state.parser = Some(parser); state } /// Returns a new RunnerState, for running code encoded as a slice of [Atom]s with the specified [Metta] runner - pub fn new_with_atoms(metta: &'a Metta, atoms: &'a[Atom]) -> Self { + pub fn new_with_atoms(metta: &'m Metta, atoms: &'i[Atom]) -> Self { let mut state = Self::new(metta); state.atoms = Some(atoms); state @@ -332,16 +332,12 @@ impl<'a> RunnerState<'a> { } else { //This interpreter is finished, process the results - match interpreter_state.into_result() { - Err(msg) => return Err(msg), - Ok(result) => { - let error = result.iter().any(|atom| atom_is_error(atom)); - self.results.push(result); - if error { - self.mode = MettaRunnerMode::TERMINATE; - return Ok(()); - } - } + let result = interpreter_state.into_result().unwrap(); + let error = result.iter().any(|atom| atom_is_error(atom)); + self.results.push(result); + if error { + self.mode = MettaRunnerMode::TERMINATE; + return Ok(()); } } @@ -349,7 +345,13 @@ impl<'a> RunnerState<'a> { // Get the next atom, and start a new intperpreter let next_atom = if let Some(parser) = self.parser.as_mut() { - parser.parse(&self.metta.0.tokenizer.borrow())? + match parser.parse(&self.metta.0.tokenizer.borrow()) { + Ok(atom) => atom, + Err(err) => { + self.mode = MettaRunnerMode::TERMINATE; + return Err(err); + } + } } else { if let Some(atoms) = self.atoms.as_mut() { if let Some((atom, rest)) = atoms.split_first() { diff --git a/python/hyperon/runner.py b/python/hyperon/runner.py index 664063a26..20e35f8d8 100644 --- a/python/hyperon/runner.py +++ b/python/hyperon/runner.py @@ -29,6 +29,9 @@ def run_step(self): Executes the next step in the interpretation plan, or begins interpretation of the next atom in the stream of MeTTa code. """ hp.runner_state_step(self.cstate) + err_str = hp.runner_state_err_str(self.cstate) + if (err_str is not None): + raise RuntimeError(err_str) def is_complete(self): """ @@ -169,6 +172,7 @@ def import_file(self, fname): program = f.read() f.close() # changing cwd + # TODO: Changing the working dir will not be necessary when the stdlib ops can access the correct runner context. See https://github.com/trueagi-io/hyperon-experimental/issues/410 prev_cwd = os.getcwd() os.chdir(os.sep.join(path[:-1])) result = self.run(program) @@ -180,6 +184,9 @@ def run(self, program, flat=False): """Runs the program""" parser = SExprParser(program) results = hp.metta_run(self.cmetta, parser.cparser) + err_str = hp.metta_err_str(self.cmetta) + if (err_str is not None): + raise RuntimeError(err_str) if flat: return [Atom._from_catom(catom) for result in results for catom in result] else: diff --git a/python/hyperonpy.cpp b/python/hyperonpy.cpp index b8b6c70f1..07d181236 100644 --- a/python/hyperonpy.cpp +++ b/python/hyperonpy.cpp @@ -798,6 +798,10 @@ PYBIND11_MODULE(hyperonpy, m) { return CMetta(metta_new_with_space_environment_and_stdlib(space.ptr(), env_builder.obj, &run_python_loader_callback, NULL)); }, "New MeTTa interpreter instance"); m.def("metta_free", [](CMetta metta) { metta_free(metta.obj); }, "Free MeTTa interpreter"); + m.def("metta_err_str", [](CMetta& metta) { + const char* err_str = metta_err_str(metta.ptr()); + return err_str != NULL ? py::cast(std::string(err_str)) : py::none(); + }, "Returns the error string from the last MeTTa operation or None"); m.def("metta_eq", [](CMetta& a, CMetta& b) { return metta_eq(a.ptr(), b.ptr()); }, "Compares two MeTTa handles"); m.def("metta_search_path_cnt", [](CMetta& metta) { return metta_search_path_cnt(metta.ptr()); }, "Returns the number of module search paths in the runner's environment"); m.def("metta_nth_search_path", [](CMetta& metta, size_t idx) { @@ -816,6 +820,8 @@ PYBIND11_MODULE(hyperonpy, m) { metta_evaluate_atom(metta.ptr(), atom_clone(atom.ptr()), copy_atoms, &atoms); return atoms; }, "Run MeTTa interpreter on an atom"); + //QUESTION: Should we eliminate the `metta_load_module` function from the native APIs, in favor of + // allowing the caller to load modules using the `import` operation in MeTTa code? m.def("metta_load_module", [](CMetta& metta, std::string text) { metta_load_module(metta.ptr(), text.c_str()); }, "Load MeTTa module"); @@ -833,6 +839,10 @@ PYBIND11_MODULE(hyperonpy, m) { }, "Initializes the MeTTa runner state for incremental execution"); m.def("runner_state_step", [](CRunnerState& state) { runner_state_step(state.ptr()); }, "Runs one incremental step of the MeTTa interpreter"); m.def("runner_state_free", [](CRunnerState state) { runner_state_free(state.obj); }, "Frees a Runner State"); + m.def("runner_state_err_str", [](CRunnerState& state) { + const char* err_str = runner_state_err_str(state.ptr()); + return err_str != NULL ? py::cast(std::string(err_str)) : py::none(); + }, "Returns the error string from the last RunnerState operation or None"); m.def("runner_state_is_complete", [](CRunnerState& state) { return runner_state_is_complete(state.ptr()); }, "Returns whether a RunnerState is finished"); m.def("runner_state_current_results", [](CRunnerState& state) { py::list lists_of_atom; diff --git a/python/tests/test_metta.py b/python/tests/test_metta.py index a96d2858e..2294d0ac7 100644 --- a/python/tests/test_metta.py +++ b/python/tests/test_metta.py @@ -57,3 +57,14 @@ def test_gnd_type_error(self): result = runner.run(program) self.assertEqual([[E(S('Error'), ValueAtom('String'), S('BadType'))]], result) + + def test_runner_error(self): + program = ''' + !(+ 2 3 + ''' + runner = MeTTa(env_builder=Environment.test_env()) + try: + runner.run(program) + self.assertTrue(False, "Parse error expected") + except RuntimeError as e: + self.assertEqual(e.args[0], 'Unexpected end of expression')