-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
When a codegen worker has a FatalError, propagate it instead of ICE'ing. #67458
When a codegen worker has a FatalError, propagate it instead of ICE'ing. #67458
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
(I'm a little worried about the test being fragile, in terms of the specifics of its output. I tried making this a ... (which sounds like a bug, if you ask me. We may want to look into changing that, to avoid having hidden ICE's in the compile-fail test suite.) I guess I could/should look into trying to do some custom normalization of the test output to side-step the fragility regarding the details of the error output. |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
You could also use |
No, that is too broad (I explicitly double-checked just now). Specifically, it will miss the ICE itself. (Note that in this particular example, we are always getting the desired diagnostic from the worker thread. The only problem is that the coordinating thread is ICE'ing, because that code was written assuming that an error from the worker must correspond to a panic.) But I think |
9f0210e
to
eda1411
Compare
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
☔ The latest upstream changes (presumably #67540) made this pull request unmergeable. Please resolve the merge conflicts. |
1f33ed9
to
77803af
Compare
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
3ec4463
to
18750e0
Compare
@bors r+ |
📌 Commit 18750e0 has been approved by |
⌛ Testing commit 18750e0 with merge 7d17d79335ee63ad9d7f8010ccbb23e88b88e396... |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
💔 Test failed - checks-azure |
…from-worker, r=matthewjasper When a codegen worker has a FatalError, propagate it instead of ICE'ing. Fix #66530
💔 Test failed - checks-azure |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
sigh, gotta ignore wasm and emscripten for some reason ... |
18750e0
to
c8a09fe
Compare
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
what ...? But I thought I fetched ... hmm. Well, okay, that teaches me to force-push without reviewing more carefully. |
c8a09fe
to
e60a406
Compare
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
it uses normalize-stderr-test because not all targets hit the same OS error number nor message ... ... and ignores tidy since I dont know how to make the normalize line shorter ... and has effectively a no-op for its error-pattern because the targets' error messages are so wildly different (and the error-pattern check occurs *before* stderr normalization.)
e60a406
to
7c5cff7
Compare
I go around claiming I'm going to be careful about force-pushing, but next thing I know, I've blown away the version of the file that actually properly "worked" on Mac OS X and Linux ... Sigh. |
@bors r=matthewjasper |
📌 Commit 7c5cff7 has been approved by |
@bors rollup=never |
⌛ Testing commit 7c5cff7 with merge 890d8c9e775898be5872e7e446dcebaa54b29ced... |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
💔 Test failed - checks-azure |
If this happens again, I'm going to change tack and make this an `// only-linux` test or something along those lines.
@bors r=matthewjasper |
📌 Commit 3193836 has been approved by |
…from-worker, r=matthewjasper When a codegen worker has a FatalError, propagate it instead of ICE'ing. Fix #66530
☀️ Test successful - checks-azure |
Fix #66530