-
Notifications
You must be signed in to change notification settings - Fork 4
Porting / vendoring Chrome C/C++ extension code, add Rust support (sum types etc) and fix https://issues.chromium.org/issues/401522579 #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
TypeError: Failed to construct 'URL': Invalid URL issue
TypeError: Failed to construct 'URL': Invalid URL issuec73f04b to
58079eb
Compare
…i' of commit 2f89fb20f4601f188569f991a98198f83322f2e2 in https://github.com/ChromeDevTools/devtools-frontend.git
…ols-backend' folders
…ode and tests accordingly - Build scripts for stage 1 and 2 of SymbolsBackend - Build scripts for e2e resources - All TypeScript compilation done outside of CMake - Test runner for e2e spec files - Enhance formatting of strings (VSCode debugger expects string to be quoted) - Fix loading of dwo files (was broken when not running in browser worker thread)
`TypeError: Failed to construct URL: Invalid URL` thrown for certain source code paths in the DW_AT_decl_file tags causing all source code resolving to fail, not just the specific source code path.
- Extend TypeSystemClang so we can handle Rust types like variants and zero sized marker types - Simplify unmarshalling of embind objects (replace EmbindObjectPool with getUnmarshalledInterface for easier handling responses from SymbolsBackend) - Add E2E spec file support for Rust
58079eb to
f5fc47e
Compare
|
Wow I'm so excited for this to land. We're building a large-ish wasm application and the lack of debugging has been a real issue. Your effort is highly appreciated @mikaelwaltersson! |
Thanks, unfortunately I suspect this PR might not be merged as it involves transforming the repository from being a thin layer of code and build scripts around the original code in devtools-frontend/extensions/cxx_debugging repository maintained by Google, into a a repository containing all the code for the extension. I can see how that adds a lot of maintenance burden. Ideally I would have been able to just create a new repository and publish my own VSCode extension but way JavaScript debugger extension So in lieu of this PR being merged the only way to use this to either locally build this forked version of the extension as I've been contemplating raising an issue/proposal in vscode-js-debug for decoupling it from the hard dependency on |
|
Thanks for the PR! Sorry I didn't see it, I see to have been unsubscribed from notifications in this repo at some point.
I wonder if you think it'd be reasonable to avoid pulling these into our sourcetree and instead having a build script that copies them in from the devtools repo -- or were there more changes/additions you had to make to these files once they were ported into this extension? |
No worries, I didn't expect it to be looked at right away either especially as the change set is quite large. I had forgot about it myself until I got a notification that there was a comment on the PR. Just fixing the issue with the However to add the Rust types support and fixing the other minor issues mentioned in the PR description requires enough changes to both the C++ code and the TypeScript code for it to be quite hard to create such a build script. Even if you create such a build script that apply patches to the code it would be extremely fragile concerning any changes in the upstream repository. Here is the diff of the changes made to the imported files, even disregarding changes from auto formatting of the code and refactoring it's still quite a lot (the diff is generated with the command I looked into importing the original code using Disregarding the tests that I ported to run in a Last but not least, in my opinion the "build script" approach makes it extremely hard to make any contributions here going forward:
Let me know if you want me to split it into multiple PR's or if the "staged" commit history is enough to go on for the review. |
…-1` values for symbol context without line entries, causing exceptions downstream as `-1` gets interpreted as the Uint32 value `0xFFFFFFFF` in `vscode-js-debug`. Also fixed `test-utils/sync-interface/worker.js` mixed `import` / `require` statements not working with newer versions of nodejs and flaky `test/SymbolsBackend.test.ts`
Two main issues with the current version this PR addresses
TypeError: Failed to construct 'URL': Invalid URL. See https://issues.chromium.org/issues/401522579 for more information.Basic Rust types supported added
String,&str,Vec<T>and array slices are not viewable because of how they are internally represented (Unique<u8>pointer to a buffer and 0-sizedPhantomData<T>marker struct.)Sum types (with the DWARF tag type
DW_TAG_variant_part) will ends up as empty structs as there is no such concept in C/C++.Option<T>andResult<T, E>.)Stringand&str) is added.Vec<T>,VecDeque<T>and&[T]) is added.Rc<T>andWeak<T>) is added.HashMap<K, V>andHashSet<T>) is added.Other minor fixes
JSON.stringifyon them. See attached screenshots for examples..dwofiles is broken in the current version. The code that handles this in the original Chrome C/C++ extension is not expecting to be run under Node.char, this is fixed in the PR.Overview of changes made to facilitate the fixes and added features
@vscode/js-debug.yamlspec files)Screenshots:
String with tab and newline in Chrome C/C++ extension:



String with tab and newline in current VSCode DWARF debugging extension:
String with tab and newline in patched VSCode DWARF debugging extension:
Rust support:

Other remarks
There is quite a lot of files in this PR so I've tried to structure the commit history in a way that makes it easier to follow.
f5a1f23562878a7d46cc719247ca4aa64b5efc17is a verbatim copy of relevant files from the https://github.com/ChromeDevTools/devtools-frontend.git repository.0ee38d9461fd7bc06efdea36fdf4d54f4b5d6721and331eba97a6589a1aeab0a2dfc1aacd48a20a91d1moves / renames the imported files and update some references to theLICENSEfile.120a00400106ff3674d12ba8b2d69356aa7278f2ports / migrates the code and tests and build scripts to be able to run in Node instead of the Chrome DevTools. So this commit is one of the mayor ones the to review along with the last one that adds the Rust type support.7195f4063e4a84fb7700c719424bd0d2da85d859fixes https://issues.chromium.org/issues/40152257939098556b3d8e8705cca353c0bd7022c8e1c7216adds Rust types support.