Remove importlib frames from traceback#1104
Conversation
|
This is a adaptation of the |
| } | ||
| } | ||
|
|
||
| pub fn remove_importlib_frames(vm: &VirtualMachine, exc: &PyObjectRef) -> PyObjectRef { |
There was a problem hiding this comment.
Are you sure you want to add this? This might be a good opportunity to keep the rustpython interpreter transparent to users, and show them what is actually going on. What is your opinion on this?
There was a problem hiding this comment.
I think that it gives too much information. I will add a TODO that when running on verbose this function will do nothing.
windelbouwman
left a comment
windelbouwman
left a comment
There was a problem hiding this comment.
Except for a minor comment, this looks good to me!
vm/src/import.rs
Outdated
| if let Ok(tb) = vm.get_attribute(exc.clone(), "__traceback__") { | ||
| if objtype::isinstance(&tb, &vm.ctx.list_type()) { | ||
| let mut tb_entries = objsequence::get_elements_list(&tb).to_vec(); | ||
| tb_entries.reverse(); |
There was a problem hiding this comment.
It looks like the list is reversed twice. This might not be required.
vm/src/import.rs
Outdated
| if objtype::isinstance(&tb, &vm.ctx.list_type()) { | ||
| let mut tb_entries = objsequence::get_elements_list(&tb).to_vec(); | ||
| tb_entries.reverse(); | ||
| let mut new_tb = Vec::with_capacity(tb_entries.len()); |
There was a problem hiding this comment.
Since this function is basically a filtering of a list, this might be an excellent opportunity to use the filter_map function, or filter function on rust iterators, and then use collect on the filtered iterator.
Something along this idea:
let new_tb = tb_entries.iter()
.filter(|tb_entry| {
// fetch filename etc..
// funky filtering here, if file_name != frozen etc...
})
.collect();
There was a problem hiding this comment.
This is only a simple filter if the error is from outside the importlib code. That is why we remove only chunks that end in _call_with_frames_removed.
vm/src/import.rs
Outdated
| new_tb.push(tb_entry.clone()) | ||
| } else { | ||
| current_chunk.push(tb_entry.clone()); | ||
| let run_obj_name = objstr::get_value(&location_attrs[0]); |
There was a problem hiding this comment.
This is the same as file_name earlier on.
vm/src/import.rs
Outdated
| let mut new_tb = Vec::with_capacity(tb_entries.len()); | ||
|
|
||
| for tb_entry in tb_entries.iter() { | ||
| let mut current_chunk = vec![]; |
There was a problem hiding this comment.
I think this variable can be removed right?
palaviv
force-pushed
the
remove-importlib-frames
branch
from
c9f01cc to
251e33a
Compare
2 participants
Previously the traceback was:
Now it is: