Allow Translation of Nev Rule Ids back to Rules and their Line Numbers#790
Conversation
monsterkrampe
added
wasm
|
"If we cannot resolve this now then please create a technical debt issue for that." |
|
Ah, ok. Maybe putting a link to the issue in the TODO comment then would be nice :) |
| nemo-physical = { path = "../nemo-physical", default-features = false } | ||
| log = { workspace = true } | ||
| nom = "7.1.1" | ||
| line-index = "0.1.1" |
There was a problem hiding this comment.
We already use that in nemo-language-server, so it's now time to move this out into the top-level Cargo.toml and inherit the version both here and there.
|
This PR also introduces RuleIdTranslation, which basically translates rule indices from the original (before transformations) to the normalized program. Hence, we assume now that no program transformation changes the structure of derivation trees (which is true now). Also, it properly sets the origins for the global, incermental import, and sparql merge transformations (which is also covered by unit tests now). And finally, some small changes to the API and how the python and wasm code builds programs and ExecutionEngines. |
| let norm_to_orig = normalized | ||
| .rules() | ||
| .iter() | ||
| .map(|rule| { | ||
| let origin_id = tracing_resolve_origin_id(handle, rule.id()); | ||
| *orig_index_by_id.get(&origin_id).expect( | ||
| "a normalized rule has no corresponding original rule; a \ | ||
| transformation created a rule without recording its origin", | ||
| ) | ||
| }) | ||
| .collect::<Vec<usize>>(); | ||
|
|
||
| // Invert the mapping. As the transformations are one-to-one on rules, no | ||
| // two normalized rules map to the same original rule, so every insert is | ||
| // into a fresh slot (checked via the resulting length below). | ||
| let mut orig_to_norm = HashMap::with_capacity(norm_to_orig.len()); | ||
| for (norm_index, &orig_index) in norm_to_orig.iter().enumerate() { | ||
| orig_to_norm.insert(orig_index, norm_index); | ||
| } |
There was a problem hiding this comment.
Why not build both mapping simultaneously?
let mut norm_to_orig = HashMap::with_capacity(normalized.rules().count());
let mut orig_to_norm = HashMap::with_capacity(original.rules().count());
for rule in normalized.rules() {
let origin_id = tracing_resolve_origin_id(handle, rule.id());
let origin_index = orig_index_by_id.get(&origin_id).expect("each normalized rule has a corresponding original rule");
let prev = norm_to_orig.insert(rule.id(), origin_index);
debug_assert!(prev.is_none(), "each normalized rule maps to exactly one original rule");
let prev = orig_to_norm.insert(origin_index, rule.id());
debug_assert!(prev.is_none(), "each original rules maps to exactly one normalized rule");
}
mmarx
left a comment
mmarx
left a comment
There was a problem hiding this comment.
Yeah, much better. I'd just prefer assert_matches!(…) over assert!(matches!(…)), since that gives much better error messages.
| .unwrap(); | ||
| let rule = transformed.rules().next().unwrap(); | ||
|
|
||
| assert!(matches!(rule.origin(), Origin::Global(id) if id == original)); |
There was a problem hiding this comment.
Better use assert_matches! instead of assert!(matches!(…)), that'll give much better error messages in case of failure:
use std::assert_matches;
assert_matches!(rule.origin(), Origin::Global(id) if id == original)| let transformed = handle.transform(TransformationIncremental::new()).unwrap(); | ||
| let rule = transformed.rules().next().unwrap(); | ||
|
|
||
| assert!(matches!(rule.origin(), Origin::Incremental(id) if id == original)); |
| let transformed = inlined.transform(TransformationMergeSparql).unwrap(); | ||
| let rule = transformed.rules().next().unwrap(); | ||
|
|
||
| assert!(matches!(rule.origin(), Origin::MergeSparql(id) if id == predecessor)); |
| .unwrap(); | ||
| let rule = transformed.rules().next().unwrap(); | ||
|
|
||
| assert!(matches!(rule.origin(), Origin::Normalization(id) if id == original)); |
3 participants
This PR adds a function to
nemo-wasmthat allows to translate a rule id from Nev into the corresponding line number in the underlying Nemo program. For this to work, the PR also touches the handling of rule ids in thenemocrate.@aannleax I'm not sure if you still wanted to clean things up here. For example, there is still a TODO in
rule_translation.rs. If we cannot resolve this now then please create a technical debt issue for that. Once you are done, please mark the PR as ready :)