Open
Conversation
330a92c to
c9a922d
Compare
There was a problem hiding this comment.
Pull request overview
Refactors the Rust binary_view API surface to remove the “viral” BinaryViewExt trait and rework custom binary view registration/initialization, while updating the Minidump view plugin and downstream crates/tests/examples to the new APIs.
Changes:
- Move/reshape custom binary view registration and traits into
rust/src/binary_view.rs, and remove the oldcustom_binary_viewmodule. - Rewrite the Minidump view to use the new custom view traits and improve module/section handling.
- Update Rust tests, examples, architectures, and plugins to stop using
BinaryViewExtand adopt the new APIs.
Reviewed changes
Copilot reviewed 97 out of 102 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| view/minidump/src/view.rs | Rewritten Minidump view using new CustomBinaryView API; adds PE parsing for sections/symbols. |
| view/minidump/src/lib.rs | Registers the Minidump view via register_binary_view_type. |
| view/minidump/src/command.rs | Deleted old command implementation. |
| view/minidump/README.md | Simplified documentation for the Minidump view. |
| view/minidump/Cargo.toml | Bumps minidump and adds object dependency. |
| view/minidump/.gitignore | Deleted plugin-local ignore file. |
| rust/tests/types.rs | Removes BinaryViewExt import usage. |
| rust/tests/type_library.rs | Removes BinaryViewExt import usage. |
| rust/tests/type_container.rs | Removes BinaryViewExt import usage. |
| rust/tests/section.rs | Removes BinaryViewExt import usage. |
| rust/tests/repository.rs | Updates RepositoryManager usage to new static-style API. |
| rust/tests/render_layer.rs | Removes BinaryViewExt import usage. |
| rust/tests/medium_level_il.rs | Removes BinaryViewExt import usage. |
| rust/tests/low_level_il.rs | Removes BinaryViewExt import usage. |
| rust/tests/language_representation.rs | Removes BinaryViewExt import usage. |
| rust/tests/high_level_il.rs | Removes BinaryViewExt import usage. |
| rust/tests/function.rs | Removes BinaryViewExt import usage. |
| rust/tests/debug_info.rs | Removes BinaryViewExt import usage. |
| rust/tests/data_renderer.rs | Removes BinaryViewExt import usage. |
| rust/tests/data_notification.rs | Removes BinaryViewExt import usage. |
| rust/tests/component.rs | Removes BinaryViewExt import usage. |
| rust/tests/collaboration.rs | Removes BinaryViewExt import usage. |
| rust/tests/binary_writer.rs | Removes BinaryViewExt import usage. |
| rust/tests/binary_view.rs | Adds custom view unit test and updates imports to new custom-view APIs. |
| rust/tests/binary_reader.rs | Removes BinaryViewExt import usage. |
| rust/tests/base_detection.rs | Removes BinaryViewExt import usage. |
| rust/src/workflow.rs | Removes BinaryViewExt import usage in docs/support code. |
| rust/src/types/structure.rs | Updates doc links/imports away from BinaryViewExt. |
| rust/src/types/library.rs | Updates doc links/imports away from BinaryViewExt. |
| rust/src/types.rs | Updates imports/docs away from BinaryViewExt. |
| rust/src/segment.rs | Adds documentation for SegmentBuilder::parent_backing. |
| rust/src/section.rs | Updates docs away from BinaryViewExt. |
| rust/src/platform.rs | Updates docs away from BinaryViewExt. |
| rust/src/lib.rs | Removes custom_binary_view module export. |
| rust/src/function.rs | Updates docs/imports away from BinaryViewExt. |
| rust/src/file_metadata.rs | Switches BinaryViewType import to the new location and improves view_types docs. |
| rust/src/ffi.rs | Removes BinaryViewExt import from ffi_span! macro. |
| rust/src/debuginfo.rs | Updates docs away from BinaryViewExt. |
| rust/src/custom_binary_view.rs | Deleted old custom view implementation module. |
| rust/src/component.rs | Removes BinaryViewExt import usage. |
| rust/src/collaboration/sync.rs | Removes BinaryViewExt import usage and improves doc comment. |
| rust/src/collaboration/snapshot.rs | Removes BinaryViewExt import usage. |
| rust/src/collaboration/project.rs | Removes BinaryViewExt import usage. |
| rust/src/collaboration/file.rs | Removes BinaryViewExt import usage. |
| rust/src/binary_view/writer.rs | Removes BinaryViewExt import usage. |
| rust/src/binary_view.rs | Major refactor: new custom view registration + removal of BinaryViewExt. |
| rust/examples/workflow.rs | Removes BinaryViewExt import usage. |
| rust/examples/simple.rs | Removes BinaryViewExt import usage. |
| rust/examples/medium_level_il.rs | Removes BinaryViewExt import usage. |
| rust/examples/high_level_il.rs | Removes BinaryViewExt import usage. |
| rust/examples/flowgraph.rs | Removes BinaryViewExt import usage. |
| rust/examples/disassemble.rs | Removes BinaryViewExt import usage. |
| rust/examples/decompile.rs | Removes BinaryViewExt import usage. |
| rust/examples/bndb_to_type_library.rs | Removes BinaryViewExt import usage. |
| rust/README.md | Updates example to avoid BinaryViewExt. |
| plugins/workflow_objc/src/metadata/selector.rs | Removes BinaryViewExt import usage. |
| plugins/workflow_objc/src/metadata/global_state.rs | Adapts metadata fetching to new get_metadata return type. |
| plugins/workflow_objc/src/activities/util.rs | Removes BinaryViewExt import usage. |
| plugins/workflow_objc/src/activities/super_init.rs | Removes BinaryViewExt import usage. |
| plugins/workflow_objc/src/activities/remove_memory_management.rs | Removes BinaryViewExt import usage. |
| plugins/workflow_objc/src/activities/objc_msg_send_calls/adjust_call_type.rs | Removes BinaryViewExt import usage. |
| plugins/workflow_objc/src/activities/objc_msg_send_calls.rs | Removes BinaryViewExt import usage. |
| plugins/workflow_objc/src/activities/alloc_init.rs | Removes BinaryViewExt import usage. |
| plugins/warp/tests/matcher.rs | Removes BinaryViewExt import usage. |
| plugins/warp/tests/determinism.rs | Removes BinaryViewExt import usage. |
| plugins/warp/src/processor.rs | Removes BinaryViewExt import usage. |
| plugins/warp/src/plugin/workflow.rs | Removes BinaryViewExt import usage. |
| plugins/warp/src/plugin/load.rs | Removes BinaryViewExt import usage. |
| plugins/warp/src/matcher.rs | Removes BinaryViewExt import usage. |
| plugins/warp/src/lib.rs | Removes BinaryViewExt import usage. |
| plugins/warp/src/convert/types.rs | Removes BinaryViewExt import usage in tests. |
| plugins/warp/src/convert/symbol.rs | Removes BinaryViewExt import usage. |
| plugins/warp/src/cache/guid.rs | Removes BinaryViewExt import usage. |
| plugins/warp/src/cache/function.rs | Removes BinaryViewExt import usage. |
| plugins/warp/src/cache.rs | Removes BinaryViewExt import usage. |
| plugins/warp/benches/guid.rs | Removes BinaryViewExt import usage. |
| plugins/warp/benches/function.rs | Removes BinaryViewExt import usage. |
| plugins/warp/benches/convert.rs | Removes BinaryViewExt import usage. |
| plugins/svd/tests/mapper.rs | Removes BinaryViewExt import usage. |
| plugins/svd/src/mapper.rs | Removes BinaryViewExt import usage. |
| plugins/svd/src/lib.rs | Removes BinaryViewExt import usage. |
| plugins/pdb-ng/src/type_parser.rs | Removes BinaryViewExt import usage. |
| plugins/pdb-ng/src/parser.rs | Removes BinaryViewExt import usage. |
| plugins/pdb-ng/src/lib.rs | Removes BinaryViewExt import usage and updates metadata fetching. |
| plugins/idb_import/src/types.rs | Removes BinaryViewExt import usage. |
| plugins/idb_import/src/mapper.rs | Removes BinaryViewExt import usage. |
| plugins/idb_import/src/commands/load_file.rs | Removes BinaryViewExt import usage. |
| plugins/dwarf/shared/src/lib.rs | Removes BinaryViewExt import usage. |
| plugins/dwarf/dwarfdump/src/lib.rs | Removes BinaryViewExt import usage. |
| plugins/dwarf/dwarf_import/src/lib.rs | Removes BinaryViewExt import usage. |
| plugins/dwarf/dwarf_import/src/helpers.rs | Removes BinaryViewExt import usage. |
| plugins/dwarf/dwarf_import/src/dwarfdebuginfo.rs | Removes BinaryViewExt import usage. |
| plugins/dwarf/dwarf_export/src/lib.rs | Removes BinaryViewExt import usage. |
| plugins/bntl_utils/src/process.rs | Switches BinaryViewType import to new location; removes BinaryViewExt usage. |
| plugins/bntl_utils/src/dump.rs | Removes BinaryViewExt import usage. |
| plugins/bntl_utils/src/command/create.rs | Removes BinaryViewExt import usage. |
| arch/riscv/src/lib.rs | Updates to new BinaryViewType::by_name return type and removes old imports. |
| arch/msp430/src/lib.rs | Updates to new BinaryViewType::by_name return type and removes old imports. |
| Cargo.lock | Updates lockfile for new/updated dependencies (minidump, object, etc.). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Remove the "viral" `BinaryViewExt` trait and its blanket impl - Split up the binary view type from the custom trait impl - Simplify and fix bugs regarding custom binary view initialization - Rewrite Minidump binary view example, parses the PE headers to create proper sections now - Add some extra documentation - Add unit test for custom binary view
c9a922d to
35c38a6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
BinaryViewExttrait and its blanket implThis is in preparation for bundling some rust view types such as the aforementioned minidump one, and potential replacements for other existing view types.
The removal of
BinaryViewExtalso fixes some cases where LSPs will trip up do to its blanket impl and suggest functions from it in completely wrong scenarios, although that itself was not the reasoning behind the change, its more so to do with the fact those functions are invalid in the case of aCustomBinaryViewthat has yet to have its core object constructed.Supersedes #8061