-
Notifications
You must be signed in to change notification settings - Fork 13.5k
Debug #48116. #48127
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Debug #48116. #48127
Conversation
@bors r+ p=1 |
📌 Commit 9f015ee has been approved by |
☔ The latest upstream changes (presumably #48113) made this pull request unmergeable. Please resolve the merge conflicts. |
@kennytm Needs rebase. |
Okay okay |
@bors r=eddyb |
📌 Commit 3337d51 has been approved by |
⌛ Testing commit 3337d51 with merge 790e7b2fcfdefa3b583b45d05b20c7de9862d5c4... |
💔 Test failed - status-travis |
Perfect, hit #48116 😂 I think we could take a look at the result before retrying.
|
Okay, looks like outer |
What I'm curious about is what in the world is nondeterministic to cause this? |
Since we've got the reason of failure, I'm transforming this PR for debugging. |
@bors try |
[WIP] Debug #48116. 1. When the invalid condition is hit, write out the relevant variables too 2. In compile-fail/parse-fail tests, check for ICE first, so the invalid error patterns won't mask our ICE output. r? @alexcrichton cc #48116, cc @eddyb
💔 Test failed - status-travis |
1. When the invalid condition is hit, write out the relevant variables too 2. In compile-fail/parse-fail tests, check for ICE first, so the invalid error patterns won't mask our ICE output.
d2ca440
to
7c54f2a
Compare
(Needs to test the magic power of the @bors r+ p=52 |
📌 Commit 4f95269 has been approved by |
⌛ Testing commit 4f95269 with merge 3ca6eed5832800d513ead5f11927ed1a4f7bf2ef... |
💔 Test failed - status-travis |
cancelbot is too aggressive, there should be some way to let cancelbot ignore manual restarts... 😐 |
Try to fix 48116 and 48192 The bug #48116 happens because of a misoptimization of the `import_path_to_string` function, where a `names` slice is empty but the `!names.is_empty()` branch is executed. https://github.com/rust-lang/rust/blob/4d2d3fc5dadf894a8ad709a5860a549f2c0b1032/src/librustc_resolve/resolve_imports.rs#L1015-L1042 Yesterday, @eddyb had locally reproduced the bug, and [came across the `position` function](https://mozilla.logbot.info/rust-infra/20180214#c14296834) where the `assume()` call is found to be suspicious. We have *not* concluded that this `assume()` causes #48116, but given [the reputation of `assume()`](#45501 (comment)), this seems higher relevant. Here we try to see if commenting it out can fix the errors. Later @alexcrichton has bisected and found a potential bug [in the LLVM side](#48116 (comment)). We are currently testing if reverting that LLVM commit is enough to stop the bug. If true, this PR can be reverted (keep the `assume()`) and we could backport the LLVM patch instead. (This PR also includes an earlier commit from #48127 for help debugging ICE happening in compile-fail/parse-fail tests.) The PR also reverts #48059, which seems to cause #48192. r? @alexcrichton cc @eddyb, @arthurprs (#47333)
r? @alexcrichton
cc #48116, cc @eddyb