Add run_query_iter()

[bakaq]

This PR addresses one of the biggest limitations of the Rust embedding API introduced in #1880, which was the lack of an iterator API on answers.

I used the "generator" code of @jjtolton from #2465 as a basis for the iterator QueryState, which is returned by the new function run_query_iter(). This iterator can express determinism, in the sense that it can differentiate an extra false caused by backtracking from having no more answers:

let mut machine = Machine::new_lib();

let results1 = machine
    .run_query_iter("true.".into())
    .collect::<Result<Vec<_>,_>>()
    .unwrap();

let results2 = machine
    .run_query_iter("true;false.".into())
    .collect::<Result<Vec<_>,_>>()
    .unwrap();

assert_eq!(results1, vec![QueryResolutionLine::True]);
assert_eq!(results2, vec![QueryResolutionLine::True, QueryResolutionLine::False]);

The old run_query() is implemented in terms of run_query_iter() (just collecting the iterator), with (as far as I tested) the same behavior and functionality¹.

The only question I have is if calling trust_me() is sufficient to clean the state of the Machine, even if the whole iteration is not consumed, but it seems to pass all tests so I guess it's fine.

@lucksus, I would appreciate if you tested this on your use cases to see if it works, because you are probably the person that most uses this API.

Footnotes

1. In fact, I solved a "bug" that the older run_query() had, in which it didn't backtrack on queries without variables, like ?- true;false.. You could consider that a feature, because it acted like a fusing || (if you know the first argument of a disjunction is true you don't have to check the other), but that also means that the alternative choice points weren't running which seems wrong (they could have side effects after all). That also means that now instead of true;false. resulting in the equivalent of [true] it returns [true, false], which is a change of behavior, but I think this is much more expressive. ↩

[bakaq]

I forgot that Miri is now enabled in CI. My new tests create a whole Machine, which we know from previous ventures doesn't work well.

[bakaq]

Ok, after some bit of testing and thought, I think that the Drop implementation with trust_me() is fine for most uses. It's equivalent to what the old run_query() did when the iterator is fully consumed (so no new bugs there), and it also seems to work fine when the iterator is not fully consumed as far as I tested. If this is ever a problem in the future we know where to look. run_query() still returns just one true in the case of ?- true;false. because of how it collects the iterator, so there is no actual breaking change there.

I think this is ready for review @mthom.

[mthom]

Looks good to me. I pinged @lucksus to have a look at this and #2460.

[lucksus]

This looks great! Thanks for doing this refactoring into an iterator <3
I will have to work through some dependency conflicts in order to compile and test this branch in our use-case. With Juniper, Deno and Holochain with have some heavy weight dependencies that share other dependecies with Scryer (and amongst them). Always a bit of a hassle to find a set of versions that work. Scryer updated reqwest but we are forced to use an older version of Deno with an older reqwest dep. I will work through these issues tomorrow and report back.

But: if the tests in lib_machine pass, including the "integration test" (which is a log of our ad4m integration tests and how those drive the included scryer lib_machine), it should all be fine on our end too.

[bakaq]

The integration tests do pass on this PR. It's completely backwards compatible as far as I know. #2475, however, is definitely a breaking change. I would appreciate if you also took a look, because it significantly improves the accuracy and representation of answers.