Rollup merge of #111494 - compiler-errors:variant-order, r=petrochenkov

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

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

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

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

The specific macro ([`protocol`](https://github.com/dylanmckay/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.

Author: Dylan-DPC

compiler/rustc_metadata/src/rmeta/decoder.rs

@@ -856,7 +856,12 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
         ty::EarlyBinder(&*output)
     }
 
-    fn get_variant(self, kind: &DefKind, index: DefIndex, parent_did: DefId) -> ty::VariantDef {
+    fn get_variant(
+        self,
+        kind: DefKind,
+        index: DefIndex,
+        parent_did: DefId,
+    ) -> (VariantIdx, ty::VariantDef) {
         let adt_kind = match kind {
             DefKind::Variant => ty::AdtKind::Enum,
             DefKind::Struct => ty::AdtKind::Struct,
@@ -870,22 +875,25 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
             if adt_kind == ty::AdtKind::Enum { Some(self.local_def_id(index)) } else { None };
         let ctor = data.ctor.map(|(kind, index)| (kind, self.local_def_id(index)));
 
-        ty::VariantDef::new(
-            self.item_name(index),
-            variant_did,
-            ctor,
-            data.discr,
-            self.get_associated_item_or_field_def_ids(index)
-                .map(|did| ty::FieldDef {
-                    did,
-                    name: self.item_name(did.index),
-                    vis: self.get_visibility(did.index),
-                })
-                .collect(),
-            adt_kind,
-            parent_did,
-            false,
-            data.is_non_exhaustive,
+        (
+            data.idx,
+            ty::VariantDef::new(
+                self.item_name(index),
+                variant_did,
+                ctor,
+                data.discr,
+                self.get_associated_item_or_field_def_ids(index)
+                    .map(|did| ty::FieldDef {
+                        did,
+                        name: self.item_name(did.index),
+                        vis: self.get_visibility(did.index),
+                    })
+                    .collect(),
+                adt_kind,
+                parent_did,
+                false,
+                data.is_non_exhaustive,
+            ),
         )
     }
 
@@ -901,7 +909,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
         };
         let repr = self.root.tables.repr_options.get(self, item_id).unwrap().decode(self);
 
-        let variants = if let ty::AdtKind::Enum = adt_kind {
+        let mut variants: Vec<_> = if let ty::AdtKind::Enum = adt_kind {
             self.root
                 .tables
                 .module_children_non_reexports
@@ -912,15 +920,22 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
                     let kind = self.def_kind(index);
                     match kind {
                         DefKind::Ctor(..) => None,
-                        _ => Some(self.get_variant(&kind, index, did)),
+                        _ => Some(self.get_variant(kind, index, did)),
                     }
                 })
                 .collect()
         } else {
-            std::iter::once(self.get_variant(&kind, item_id, did)).collect()
+            std::iter::once(self.get_variant(kind, item_id, did)).collect()
         };
 
-        tcx.mk_adt_def(did, adt_kind, variants, repr)
+        variants.sort_by_key(|(idx, _)| *idx);
+
+        tcx.mk_adt_def(
+            did,
+            adt_kind,
+            variants.into_iter().map(|(_, variant)| variant).collect(),
+            repr,
+        )
     }
 
     fn get_visibility(self, id: DefIndex) -> Visibility<DefId> {

compiler/rustc_metadata/src/rmeta/encoder.rs

@@ -1375,9 +1375,10 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
             // Therefore, the loop over variants will encode its fields as the adt's children.
         }
 
-        for variant in adt_def.variants().iter() {
+        for (idx, variant) in adt_def.variants().iter_enumerated() {
             let data = VariantData {
                 discr: variant.discr,
+                idx,
                 ctor: variant.ctor.map(|(kind, def_id)| (kind, def_id.index)),
                 is_non_exhaustive: variant.is_field_list_non_exhaustive(),
             };

compiler/rustc_metadata/src/rmeta/mod.rs

@@ -31,6 +31,7 @@ use rustc_span::edition::Edition;
 use rustc_span::hygiene::{ExpnIndex, MacroKind};
 use rustc_span::symbol::{Ident, Symbol};
 use rustc_span::{self, ExpnData, ExpnHash, ExpnId, Span};
+use rustc_target::abi::VariantIdx;
 use rustc_target::spec::{PanicStrategy, TargetTriple};
 
 use std::marker::PhantomData;
@@ -430,6 +431,7 @@ define_tables! {
 
 #[derive(TyEncodable, TyDecodable)]
 struct VariantData {
+    idx: VariantIdx,
     discr: ty::VariantDiscr,
     /// If this is unit or tuple-variant/struct, then this is the index of the ctor id.
     ctor: Option<(CtorKind, DefIndex)>,

tests/ui/enum-discriminant/auxiliary/discr-foreign-dep.rs

@@ -0,0 +1,7 @@
+#[derive(Default)]
+pub enum Foo {
+    A(u32),
+    #[default]
+    B,
+    C(u32),
+}

tests/ui/enum-discriminant/discr-foreign.rs

@@ -0,0 +1,11 @@
+// aux-build:discr-foreign-dep.rs
+// build-pass
+
+extern crate discr_foreign_dep;
+
+fn main() {
+    match Default::default() {
+        discr_foreign_dep::Foo::A(_) => {}
+        _ => {}
+    }
+}