You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
NLL: diagnostics deviate from source line order for no obvious reason #51167
Why are we inverting the order in which we report these diagnostics?
We may want to consider not doing that. Even if it means e.g. buffering the diangotics and sorting them before emitting them, or some similar hack. (I admit I am loathe to do this since I myself really like when I am able to correlate the RUST_LOG output with diagnostic output. But if necessary, we could add a -Z flag to turn off the buffer+sorting, in order to bring back such correlation. For end users, buffering+sorting is probably a net win.)
An important reason to prioritize this: Fixing this is likely going to make our internal process for comparing the diagnostic output more efficient.
OK, @csmoe did the RPO trick -- it definitely helped, but it doesn't seem to have fixed @pnkfelix's example. Not sure @pnkfelix how many more such examples there are -- however, it occurs to me -- @spastorino is also adding some buffering that we might use to do sorting after the fact, as well.
Activity
nikomatsakis commentedon Jun 29, 2018
Marking as Edition Preview 2 because .. maybe an easy fix.
nikomatsakis commentedon Jul 3, 2018
I think we should just try visiting the basic blocks in reverse-postfix order. The visit occurs here:
rust/src/librustc_mir/dataflow/mod.rs
Lines 334 to 340 in 860d169
and in particular the for loop here goes over the basic blocks in an arbitrary order
rust/src/librustc_mir/dataflow/mod.rs
Line 336 in 860d169
What we want to do is to go over in reverse post-order, which basically maps to execution order as we would normally define it. There is a function
reverse_postorderthat already exists to compute this ordering -- it returns anIterator<Item = BasicBlock>-- and you can see some other random code that uses it here:rust/src/librustc_mir/transform/qualify_consts.rs
Line 130 in 860d169
So we basically want to change that code to iterate over the result of
reverse_postorder.nikomatsakis commentedon Jul 6, 2018
OK, @csmoe did the RPO trick -- it definitely helped, but it doesn't seem to have fixed @pnkfelix's example. Not sure @pnkfelix how many more such examples there are -- however, it occurs to me -- @spastorino is also adding some buffering that we might use to do sorting after the fact, as well.
Rollup merge of rust-lang#52067 - csmoe:issue-51167, r=nikomatsakis
nikomatsakis commentedon Jul 17, 2018
Assigning to @spastorino as this should get fixed as a side-effect of #46908
spastorino commentedon Jul 23, 2018
Assigning to @pnkfelix as he took over the migrate thing. I can help here if needed at some point.
pnkfelix commentedon Jul 24, 2018
@nikomatsakis do you think this should remain on EP2, or should we move it to RC?
(I'm going to take a shot at resolving it today, or at least doing as much as I can via the buffering we added. But still, time is tight for EP2.)
pnkfelix commentedon Jul 24, 2018
moving this to RC as I think it is sufficiently low priority that it should not block EP2 nor NLL's release in EP2.
Rollup merge of rust-lang#52904 - pnkfelix:issue-51167-sort-by-span, …
Rollup merge of rust-lang#52904 - pnkfelix:issue-51167-sort-by-span, …