Moving to JAQ 2.0 #524
Moving to JAQ 2.0 #524
Conversation
jsuereth
commented
Dec 14, 2024
•
jsuereth
commented
- Update Filter to only remember strings, as we can't safely precompile with new lifetime shenanigans.
- Create new jq file to hide all details of calling JQ
- Update filter/engine to start passing serde-JSON structures by default, instead of JAQ internals.
- Create error-message generation.
- Update tests.
- Update Filter to only remember strings, as we can't safely precompile with new lifetime shenanigans. - Create new jq file to hide all details of calling JQ - Update filter/engine to start passing serde-JSON structures by default, instead of JAQ internals. - Update tests. Note: error messages are horrendous with this update.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| })?; | ||
|
|
||
| let (names, values) = prepare_jq_context(params); | ||
| let funs = jaq_std::funs().chain(jaq_json::funs()); |
There was a problem hiding this comment.
nit: I’m not entirely sure, but you could try something like this to make the intent more explicit.
fn adjust_lifetime<'a, T: 'a>(iter: impl Iterator<Item = T>) -> impl Iterator<Item = T> + 'a {
iter
}
let funs = adjust_lifetime(jaq_std::funs().chain(jaq_json::funs()));
...
.with_funs(funs)
.compile(modules)
...
There was a problem hiding this comment.
unfortunately, that function doesn't work (iter may not live for lifetime of 'a). What we need is to trick the compiler to infer a narrower lifetime when seeing both 'a and 'static to return 'a, and we can only do that when they are in contravariant (or return-value) position. I haven't figured out how to do that yet, but I can look that up for future cleanup. I figure it's not important enough to block this PR.
There was a problem hiding this comment.
So i think I have my arrows backwards (using contravariant to mean covariant), but otherwise (while I didn't get a chance to fully read this), the answer probably lies here: https://doc.rust-lang.org/nomicon/subtyping.html
From what I can tell the variance rules are only used to change lifetimes within rust, the trick I'm using with lambda is the only way I could think of.
| ctx.insert_natives(jaq_core::core()); | ||
| ctx.insert_defs(jaq_std::std()); | ||
| ctx.insert_defs(defs); |
There was a problem hiding this comment.
nit: It's probably ok but are we still supporting the same JQ core functions and definitions?
There was a problem hiding this comment.
I believe we are. I'm pulling in the jaq_std defa and json defa
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #524 +/- ##
=======================================
+ Coverage 74.1% 74.5% +0.4%
=======================================
Files 50 51 +1
Lines 3946 3960 +14
=======================================
+ Hits 2926 2954 +28
+ Misses 1020 1006 -14 ☔ View full report in Codecov by Sentry. |
