-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Improve error when an .rlib can't be parsed #88368
Conversation
(rust-highfive has picked a reviewer for you, use r? to override) |
I would also be fine with making this a warning instead (a proper warning, using |
This comment has been minimized.
This comment has been minimized.
It's been a while since I've looked at this, but I'm a little concerned about changing this to an error. There are some assumptions about invalid libraries, where they should be legitimately ignored. For example, there are race conditions in the pipelining system whereby I don't have much time to dig into this right now, but I'll try if petrochenkov doesn't get to it first. |
I think metadata errors from this PR can use the same approach - stashed into new |
To copy my comment from #88351 (comment):
I originally intended the warn!() to be a user visible warning when I introduced it in #53470, but @alexcrichton had concerns that it would show up under normal usage when one rustc is loading metadata while another is writing an rlib. |
I implemented this. |
warn!
logging when an .rlib can't be parsed to a hard error
Err, I think I accidentally reused an existing error code. I'll add a new one. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
254729c
to
26c7039
Compare
This usually describes either an error in the compiler itself or some sort of IO error. Either way, we should report it to the user rather than just saying "crate not found". This only gives an error if the crate couldn't be loaded at all - if the compiler finds another .rlib or .rmeta file which was valid, it will continue to compile the crate. Example output: ``` error[E0785]: found invalid metadata files for crate `foo` --> bar.rs:3:24 | 3 | println!("{}", foo::FOO_11_49[0]); | ^^^ | = warning: failed to parse rlib '/home/joshua/test-rustdoc/libfoo.rlib': Invalid archive extended name offset ```
26c7039
to
257ac1b
Compare
|
@bors r=petrochenkov rollup=iffy |
📌 Commit 257ac1b has been approved by |
⌛ Testing commit 257ac1b with merge 0432b5360a5eef87e52ca8fc679acbf8d5da176e... |
💔 Test failed - checks-actions |
The job Click to see the possible cause of the failure (guessed by this bot)
|
Spurious? |
☀️ Test successful - checks-actions |
Finished benchmarking commit (5fa94f3): comparison url. Summary: This change led to moderate relevant mixed results 🤷 in compiler performance.
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression |
I suspect the regression is just because the new |
This usually describes either an error in the compiler itself or some
sort of IO error. Either way, we should report it to the user rather
than just saying "crate not found".
This only gives an error if the crate couldn't be loaded at all - if the
compiler finds another .rlib or .rmeta file which was valid, it will
continue to compile the crate.
Example output:
cc @ehuss