Work around ICE in diagnostics for local super-universes missing UniverseInfos
#115384
Conversation
Query canonicalization can create local super-universes without causes, creating ICEs when accessed during diagnostics.
rustbot
added
S-waiting-on-review
| // #114907 where this happens via liveness and dropck outlives results. | ||
| // Therefore, we return a default value in case that happens, which should at worst emit a | ||
| // suboptimal error, instead of the ICE. | ||
| self.universe_causes.get(&universe).cloned().unwrap_or_else(|| UniverseInfo::other()) |
There was a problem hiding this comment.
If we're going to do this here we should remove the places where we add UniverseInfo::other to universe causes.
There was a problem hiding this comment.
I've pushed a few commits to remove these. I think that's all of them.
My thoughts exactly. Ensuring all these code paths have access to the correct universe cause map (possibly throughout queries), and that we couldn't regress diagnostics by mistake, should also mean having new APIs always requiring a cause when a universe is created. That seems like quite a complicated refactor for such rare ICEs, especially if we can independently improve these higher-ranked subtyping diagnostics in general, and if/when the new solver changes things in these situations. |
…ical_with_fresh_inference_vars` This was backfilling causes for the new universes that can be created by the InferCtxt. We don't need to do that anymore: `other()` is the default when there is no registered universe cause.
This was pre-filling causes for universes that could already exist in the InferCtxt. We don't need to do that anymore: `other()` is the default when there is no registered universe cause.
This was backfilling causes for new universes that may have been created by an op, when there was no error info to use for improved diagnostics. We don't need to do that anymore: `other()` is the default when there is no registered universe cause.
|
@bors r+ |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (4b71f03): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 631.748s -> 629.802s (-0.31%) |
In issue #114907, canonicalization of liveness dropck-outlives results (IIUC) encounters universes absent from the original query. Some local universes are created for the mapping, but importantly, they won't have associated causes.
These missing
UniverseInfos can be needed during diagnostics, causing theIndexMap: key not foundICE seen in the issue.This PR works around this by returning the suboptimal catch-all cause, to avoid the ICE. It does results in suboptimal diagnostics right now, but it's better than an ICE.
r? @matthewjasper.
Let me know if there's a good easy-ish way to fix this, but I believe that for some of these erroneous cases and diagnostics, that inference/canonicalization/higher-ranked subtyping/etc may not behave exactly the same with the new trait solver? If that's the case then it'd probably be best to wait a bit more to do the correct fix.
Fixes #114907.
cc @aliemjay