Encode VariantIdx so we can decode ADT variants in the right order

[compiler-errors]

As far as I can tell, we don't guarantee anything about the ordering of DefIds and module children...

The code that motivated this PR (#111483) looks something like:

#[derive(Protocol)]
pub enum Data {
    #[protocol(discriminator(0x00))]
    Disconnect(Disconnect),
    EncryptionRequest,
    /* more variants... */
}

The specific macro (protocol) doesn't really matter, but as far as I can tell (from calls to build_reduced_graph), the presence of that #[protocol(..)] helper attribute causes the def-id of the Disconnect enum variant to be collected after its siblings, and it shows up after the other variants in module_children.

When we decode the variants for Data in a child crate (an example test, in this case), this means that the Disconnect variant is moved to the end of the variants list, and all of the other variants now have incorrect relative discriminant data, causing the ICE.

This PR fixes this by sorting manually by variant index after they are decoded. I guess there are alternative ways of fixing this, such as not reusing module_children_non_reexports to encode the order-sensitive ADT variants, or to do some sorting in rustc_resolve... but none of those seemed particularly satisfying either.

I really struggled to create a reproduction here -- it required at least 3 crates, one of which is a proc macro, and then some code to actually compute discriminants in the child crate... Needless to say, I failed to repro this in a test, but I can confirm that it fixes the regression in #111483. Test exists now.

r? @petrochenkov but feel free to reassign. Again, sorry for no test, but I hope the explanation at least suggests why a fix like this is likely necessary. Feedback is welcome.

[petrochenkov]

it required at least 3 crates, one of which is a proc macro

I think you could put #[rustfmt::skip] on a variant to delay its "materialization" into a DefId, then you won't need a proc macro.

[petrochenkov]

r=me, preferably with a test #111494 (comment).

[compiler-errors]

Took a lot of work, but I found a test that actually reproduced the issue. It was surprisingly hard (and really order-dependent) to load the wrong ADT from the crate metadata...

@bors r=petrochenkov

[bors]

📌 Commit ff54c80 has been approved by petrochenkov

It is now in the queue for this repository.

[apiraino]

Beta backport approved as per compiler team on Zulip

@rustbot label +beta-accepted