Auto merge of #59382 - davidtwco:rfc-2008-refactoring, r=petrochenkov

Separate `DefId`s for variants and their constructors

Part of #44109. Split off from #59376. See [Zulip topic](https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/rfc-2008/near/132663140) for previous discussion.

r? @petrochenkov

6 files changed, 65 insertions, 81 deletions

diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs
index 2cbd2dfeb25..bcc8fdf8cd4 100644
--- a/src/libsyntax/ast.rs
+++ b/src/libsyntax/ast.rs
@@ -1956,8 +1956,13 @@ pub struct EnumDef {
 
 #[derive(Clone, RustcEncodable, RustcDecodable, Debug)]
 pub struct Variant_ {
+    /// Name of the variant.
     pub ident: Ident,
+    /// Attributes of the variant.
     pub attrs: Vec<Attribute>,
+    /// Id of the variant (not the constructor, see `VariantData::ctor_id()`).
+    pub id: NodeId,
+    /// Fields and constructor id of the variant.
     pub data: VariantData,
     /// Explicit discriminant, e.g., `Foo = 1`.
     pub disr_expr: Option<AnonConst>,
@@ -2117,23 +2122,13 @@ pub struct StructField {
     pub attrs: Vec<Attribute>,
 }
 
-/// Fields and Ids of enum variants and structs
-///
-/// For enum variants: `NodeId` represents both an Id of the variant itself (relevant for all
-/// variant kinds) and an Id of the variant's constructor (not relevant for `Struct`-variants).
-/// One shared Id can be successfully used for these two purposes.
-/// Id of the whole enum lives in `Item`.
-///
-/// For structs: `NodeId` represents an Id of the structure's constructor, so it is not actually
-/// used for `Struct`-structs (but still presents). Structures don't have an analogue of "Id of
-/// the variant itself" from enum variants.
-/// Id of the whole struct lives in `Item`.
+/// Fields and constructor ids of enum variants and structs.
 #[derive(Clone, RustcEncodable, RustcDecodable, Debug)]
 pub enum VariantData {
     /// Struct variant.
     ///
     /// E.g., `Bar { .. }` as in `enum Foo { Bar { .. } }`.
-    Struct(Vec<StructField>, NodeId, bool),
+    Struct(Vec<StructField>, bool),
     /// Tuple variant.
     ///
     /// E.g., `Bar(..)` as in `enum Foo { Bar(..) }`.
@@ -2145,36 +2140,19 @@ pub enum VariantData {
 }
 
 impl VariantData {
+    /// Return the fields of this variant.
     pub fn fields(&self) -> &[StructField] {
         match *self {
             VariantData::Struct(ref fields, ..) | VariantData::Tuple(ref fields, _) => fields,
             _ => &[],
         }
     }
-    pub fn id(&self) -> NodeId {
+
+    /// Return the `NodeId` of this variant's constructor, if it has one.
+    pub fn ctor_id(&self) -> Option<NodeId> {
         match *self {
-            VariantData::Struct(_, id, _) | VariantData::Tuple(_, id) | VariantData::Unit(id) => id,
-        }
-    }
-    pub fn is_struct(&self) -> bool {
-        if let VariantData::Struct(..) = *self {
-            true
-        } else {
-            false
-        }
-    }
-    pub fn is_tuple(&self) -> bool {
-        if let VariantData::Tuple(..) = *self {
-            true
-        } else {
-            false
-        }
-    }
-    pub fn is_unit(&self) -> bool {
-        if let VariantData::Unit(..) = *self {
-            true
-        } else {
-            false
+            VariantData::Struct(..) => None,
+            VariantData::Tuple(_, id) | VariantData::Unit(id) => Some(id),
         }
     }
 }
diff --git a/src/libsyntax/config.rs b/src/libsyntax/config.rs
index 7159c949513..18173628a26 100644
--- a/src/libsyntax/config.rs
+++ b/src/libsyntax/config.rs
@@ -225,10 +225,9 @@ impl<'a> StripUnconfigured<'a> {
 
     fn configure_variant_data(&mut self, vdata: &mut ast::VariantData) {
         match vdata {
-            ast::VariantData::Struct(fields, _id, _) |
-            ast::VariantData::Tuple(fields, _id) =>
+            ast::VariantData::Struct(fields, ..) | ast::VariantData::Tuple(fields, _) =>
                 fields.flat_map_in_place(|field| self.configure(field)),
-            ast::VariantData::Unit(_id) => {}
+            ast::VariantData::Unit(_) => {}
         }
     }
 
diff --git a/src/libsyntax/ext/build.rs b/src/libsyntax/ext/build.rs
index e95f0589449..614967bdeb4 100644
--- a/src/libsyntax/ext/build.rs
+++ b/src/libsyntax/ext/build.rs
@@ -1062,6 +1062,7 @@ impl<'a> AstBuilder for ExtCtxt<'a> {
         respan(span,
                ast::Variant_ {
                    ident,
+                   id: ast::DUMMY_NODE_ID,
                    attrs: Vec::new(),
                    data: vdata,
                    disr_expr: None,
diff --git a/src/libsyntax/mut_visit.rs b/src/libsyntax/mut_visit.rs
index 5bb1d8a4b94..784d0049ac5 100644
--- a/src/libsyntax/mut_visit.rs
+++ b/src/libsyntax/mut_visit.rs
@@ -450,9 +450,10 @@ pub fn noop_visit_foreign_mod<T: MutVisitor>(foreign_mod: &mut ForeignMod, vis:
 }
 
 pub fn noop_visit_variant<T: MutVisitor>(variant: &mut Variant, vis: &mut T) {
-    let Spanned { node: Variant_ { ident, attrs, data, disr_expr }, span } = variant;
+    let Spanned { node: Variant_ { ident, attrs, id, data, disr_expr }, span } = variant;
     vis.visit_ident(ident);
     visit_attrs(attrs, vis);
+    vis.visit_id(id);
     vis.visit_variant_data(data);
     visit_opt(disr_expr, |disr_expr| vis.visit_anon_const(disr_expr));
     vis.visit_span(span);
@@ -765,11 +766,11 @@ pub fn noop_visit_where_predicate<T: MutVisitor>(pred: &mut WherePredicate, vis:
 
 pub fn noop_visit_variant_data<T: MutVisitor>(vdata: &mut VariantData, vis: &mut T) {
     match vdata {
-        VariantData::Struct(fields, id, _) |
+        VariantData::Struct(fields, ..) => visit_vec(fields, |field| vis.visit_struct_field(field)),
         VariantData::Tuple(fields, id) => {
             visit_vec(fields, |field| vis.visit_struct_field(field));
             vis.visit_id(id);
-        }
+        },
         VariantData::Unit(id) => vis.visit_id(id),
     }
 }
diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs
index 99260314054..d7a2170342d 100644
--- a/src/libsyntax/parse/parser.rs
+++ b/src/libsyntax/parse/parser.rs
@@ -6844,7 +6844,7 @@ impl<'a> Parser<'a> {
             } else {
                 // If we see: `struct Foo<T> where T: Copy { ... }`
                 let (fields, recovered) = self.parse_record_struct_body()?;
-                VariantData::Struct(fields, ast::DUMMY_NODE_ID, recovered)
+                VariantData::Struct(fields, recovered)
             }
         // No `where` so: `struct Foo<T>;`
         } else if self.eat(&token::Semi) {
@@ -6852,7 +6852,7 @@ impl<'a> Parser<'a> {
         // Record-style struct definition
         } else if self.token == token::OpenDelim(token::Brace) {
             let (fields, recovered) = self.parse_record_struct_body()?;
-            VariantData::Struct(fields, ast::DUMMY_NODE_ID, recovered)
+            VariantData::Struct(fields, recovered)
         // Tuple-style struct definition with optional where-clause.
         } else if self.token == token::OpenDelim(token::Paren) {
             let body = VariantData::Tuple(self.parse_tuple_struct_body()?, ast::DUMMY_NODE_ID);
@@ -6881,10 +6881,10 @@ impl<'a> Parser<'a> {
         let vdata = if self.token.is_keyword(keywords::Where) {
             generics.where_clause = self.parse_where_clause()?;
             let (fields, recovered) = self.parse_record_struct_body()?;
-            VariantData::Struct(fields, ast::DUMMY_NODE_ID, recovered)
+            VariantData::Struct(fields, recovered)
         } else if self.token == token::OpenDelim(token::Brace) {
             let (fields, recovered) = self.parse_record_struct_body()?;
-            VariantData::Struct(fields, ast::DUMMY_NODE_ID, recovered)
+            VariantData::Struct(fields, recovered)
         } else {
             let token_str = self.this_token_descr();
             let mut err = self.fatal(&format!(
@@ -7708,7 +7708,7 @@ impl<'a> Parser<'a> {
                 // Parse a struct variant.
                 all_nullary = false;
                 let (fields, recovered) = self.parse_record_struct_body()?;
-                struct_def = VariantData::Struct(fields, ast::DUMMY_NODE_ID, recovered);
+                struct_def = VariantData::Struct(fields, recovered);
             } else if self.check(&token::OpenDelim(token::Paren)) {
                 all_nullary = false;
                 struct_def = VariantData::Tuple(
@@ -7730,6 +7730,7 @@ impl<'a> Parser<'a> {
 
             let vr = ast::Variant_ {
                 ident,
+                id: ast::DUMMY_NODE_ID,
                 attrs: variant_attrs,
                 data: struct_def,
                 disr_expr,
diff --git a/src/libsyntax/print/pprust.rs b/src/libsyntax/print/pprust.rs
index 07df14ddc72..f5a9aded845 100644
--- a/src/libsyntax/print/pprust.rs
+++ b/src/libsyntax/print/pprust.rs
@@ -1550,44 +1550,47 @@ impl<'a> State<'a> {
                         print_finalizer: bool) -> io::Result<()> {
         self.print_ident(ident)?;
         self.print_generic_params(&generics.params)?;
-        if !struct_def.is_struct() {
-            if struct_def.is_tuple() {
-                self.popen()?;
-                self.commasep(
-                    Inconsistent, struct_def.fields(),
-                    |s, field| {
-                        s.maybe_print_comment(field.span.lo())?;
-                        s.print_outer_attributes(&field.attrs)?;
-                        s.print_visibility(&field.vis)?;
-                        s.print_type(&field.ty)
-                    }
-                )?;
-                self.pclose()?;
-            }
-            self.print_where_clause(&generics.where_clause)?;
-            if print_finalizer {
-                self.s.word(";")?;
+        match struct_def {
+            ast::VariantData::Tuple(..) | ast::VariantData::Unit(..) => {
+                if let ast::VariantData::Tuple(..) = struct_def {
+                    self.popen()?;
+                    self.commasep(
+                        Inconsistent, struct_def.fields(),
+                        |s, field| {
+                            s.maybe_print_comment(field.span.lo())?;
+                            s.print_outer_attributes(&field.attrs)?;
+                            s.print_visibility(&field.vis)?;
+                            s.print_type(&field.ty)
+                        }
+                    )?;
+                    self.pclose()?;
+                }
+                self.print_where_clause(&generics.where_clause)?;
+                if print_finalizer {
+                    self.s.word(";")?;
+                }
+                self.end()?;
+                self.end() // close the outer-box
             }
-            self.end()?;
-            self.end() // close the outer-box
-        } else {
-            self.print_where_clause(&generics.where_clause)?;
-            self.nbsp()?;
-            self.bopen()?;
-            self.hardbreak_if_not_bol()?;
-
-            for field in struct_def.fields() {
+            ast::VariantData::Struct(..) => {
+                self.print_where_clause(&generics.where_clause)?;
+                self.nbsp()?;
+                self.bopen()?;
                 self.hardbreak_if_not_bol()?;
-                self.maybe_print_comment(field.span.lo())?;
-                self.print_outer_attributes(&field.attrs)?;
-                self.print_visibility(&field.vis)?;
-                self.print_ident(field.ident.unwrap())?;
-                self.word_nbsp(":")?;
-                self.print_type(&field.ty)?;
-                self.s.word(",")?;
-            }
 
-            self.bclose(span)
+                for field in struct_def.fields() {
+                    self.hardbreak_if_not_bol()?;
+                    self.maybe_print_comment(field.span.lo())?;
+                    self.print_outer_attributes(&field.attrs)?;
+                    self.print_visibility(&field.vis)?;
+                    self.print_ident(field.ident.unwrap())?;
+                    self.word_nbsp(":")?;
+                    self.print_type(&field.ty)?;
+                    self.s.word(",")?;
+                }
+
+                self.bclose(span)
+            }
         }
     }
 
@@ -3266,6 +3269,7 @@ mod tests {
             let var = source_map::respan(syntax_pos::DUMMY_SP, ast::Variant_ {
                 ident,
                 attrs: Vec::new(),
+                id: ast::DUMMY_NODE_ID,
                 // making this up as I go.... ?
                 data: ast::VariantData::Unit(ast::DUMMY_NODE_ID),
                 disr_expr: None,