about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2023-10-09 13:18:47 +0000
committerbors <bors@rust-lang.org>2023-10-09 13:18:47 +0000
commitbe581d9f82a1bcc547c685d7e1b411c246b0bb00 (patch)
tree2a8e943c3edc2ee25d2eac993c6578638a086810
parent7ed044c075fc0e7ad2574d3144ca00ba14608d8f (diff)
parent1210aac1c00d9d3435ba801e0dee2d4ac502a738 (diff)
downloadrust-be581d9f82a1bcc547c685d7e1b411c246b0bb00.tar.gz
rust-be581d9f82a1bcc547c685d7e1b411c246b0bb00.zip
Auto merge of #116142 - GuillaumeGomez:enum-variant-display, r=fmease
[rustdoc] Show enum discrimant if it is a C-like variant

Fixes https://github.com/rust-lang/rust/issues/101337.

We currently display values for associated constant items in traits:

![image](https://github.com/rust-lang/rust/assets/3050060/03e566ec-c670-47b4-8ca2-b982baa7a0f4)

And we also display constant values like [here](file:///home/imperio/rust/rust/build/x86_64-unknown-linux-gnu/doc/std/f32/consts/constant.E.html).

I think that for coherency, we should display values of C-like enum variants.

With this change, it looks like this:

![image](https://github.com/rust-lang/rust/assets/3050060/b53fbbe0-bdb1-4289-8537-f2dd4988e9ac)

As for the display of the constant value itself, I used what we already have to keep coherency.

We display the C-like variants value in the following scenario:
 1. It is a C-like variant with a value set => all the time
 2. It is a C-like variant without a value set: All other variants are C-like variants and at least one them has its value set.

Here is the result in code:

```rust
// Ax and Bx value will be displayed.
enum A {
    Ax = 12,
    Bx,
}

// Ax and Bx value will not be displayed
enum B {
    Ax,
    Bx,
}

// Bx value will not be displayed
enum C {
    Ax(u32),
    Bx,
}

// Bx value will not be displayed, Cx value will be displayed.
#[repr(u32)]
enum D {
    Ax(u32),
    Bx,
    Cx = 12,
}
```

r? `@notriddle`
-rw-r--r--src/librustdoc/clean/types.rs7
-rw-r--r--src/librustdoc/clean/utils.rs28
-rw-r--r--src/librustdoc/html/render/print_item.rs106
-rw-r--r--src/librustdoc/json/conversions.rs2
-rw-r--r--tests/rustdoc/auxiliary/enum-variant.rs24
-rw-r--r--tests/rustdoc/enum-variant-value.rs117
6 files changed, 253 insertions, 31 deletions
diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs
index b891dc59cb1..48ce0a89449 100644
--- a/src/librustdoc/clean/types.rs
+++ b/src/librustdoc/clean/types.rs
@@ -2084,9 +2084,8 @@ impl Discriminant {
     pub(crate) fn expr(&self, tcx: TyCtxt<'_>) -> Option<String> {
         self.expr.map(|body| rendered_const(tcx, body))
     }
-    /// Will always be a machine readable number, without underscores or suffixes.
-    pub(crate) fn value(&self, tcx: TyCtxt<'_>) -> String {
-        print_evaluated_const(tcx, self.value, false).unwrap()
+    pub(crate) fn value(&self, tcx: TyCtxt<'_>, with_underscores: bool) -> String {
+        print_evaluated_const(tcx, self.value, with_underscores, false).unwrap()
     }
 }
 
@@ -2331,7 +2330,7 @@ impl ConstantKind {
         match *self {
             ConstantKind::TyConst { .. } | ConstantKind::Anonymous { .. } => None,
             ConstantKind::Extern { def_id } | ConstantKind::Local { def_id, .. } => {
-                print_evaluated_const(tcx, def_id, true)
+                print_evaluated_const(tcx, def_id, true, true)
             }
         }
     }
diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs
index 8388f722a7f..01078504b71 100644
--- a/src/librustdoc/clean/utils.rs
+++ b/src/librustdoc/clean/utils.rs
@@ -275,7 +275,8 @@ pub(crate) fn print_const(cx: &DocContext<'_>, n: ty::Const<'_>) -> String {
 pub(crate) fn print_evaluated_const(
     tcx: TyCtxt<'_>,
     def_id: DefId,
-    underscores_and_type: bool,
+    with_underscores: bool,
+    with_type: bool,
 ) -> Option<String> {
     tcx.const_eval_poly(def_id).ok().and_then(|val| {
         let ty = tcx.type_of(def_id).instantiate_identity();
@@ -284,7 +285,7 @@ pub(crate) fn print_evaluated_const(
             (mir::ConstValue::Scalar(_), &ty::Adt(_, _)) => None,
             (mir::ConstValue::Scalar(_), _) => {
                 let const_ = mir::Const::from_value(val, ty);
-                Some(print_const_with_custom_print_scalar(tcx, const_, underscores_and_type))
+                Some(print_const_with_custom_print_scalar(tcx, const_, with_underscores, with_type))
             }
             _ => None,
         }
@@ -320,32 +321,37 @@ fn format_integer_with_underscore_sep(num: &str) -> String {
 fn print_const_with_custom_print_scalar<'tcx>(
     tcx: TyCtxt<'tcx>,
     ct: mir::Const<'tcx>,
-    underscores_and_type: bool,
+    with_underscores: bool,
+    with_type: bool,
 ) -> String {
     // Use a slightly different format for integer types which always shows the actual value.
     // For all other types, fallback to the original `pretty_print_const`.
     match (ct, ct.ty().kind()) {
         (mir::Const::Val(mir::ConstValue::Scalar(int), _), ty::Uint(ui)) => {
-            if underscores_and_type {
-                format!("{}{}", format_integer_with_underscore_sep(&int.to_string()), ui.name_str())
+            let mut output = if with_underscores {
+                format_integer_with_underscore_sep(&int.to_string())
             } else {
                 int.to_string()
+            };
+            if with_type {
+                output += ui.name_str();
             }
+            output
         }
         (mir::Const::Val(mir::ConstValue::Scalar(int), _), ty::Int(i)) => {
             let ty = ct.ty();
             let size = tcx.layout_of(ty::ParamEnv::empty().and(ty)).unwrap().size;
             let data = int.assert_bits(size);
             let sign_extended_data = size.sign_extend(data) as i128;
-            if underscores_and_type {
-                format!(
-                    "{}{}",
-                    format_integer_with_underscore_sep(&sign_extended_data.to_string()),
-                    i.name_str()
-                )
+            let mut output = if with_underscores {
+                format_integer_with_underscore_sep(&sign_extended_data.to_string())
             } else {
                 sign_extended_data.to_string()
+            };
+            if with_type {
+                output += i.name_str();
             }
+            output
         }
         _ => ct.to_string(),
     }
diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs
index c6751c9585e..467493cb0b3 100644
--- a/src/librustdoc/html/render/print_item.rs
+++ b/src/librustdoc/html/render/print_item.rs
@@ -5,10 +5,12 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_hir as hir;
 use rustc_hir::def::CtorKind;
 use rustc_hir::def_id::DefId;
+use rustc_index::IndexVec;
 use rustc_middle::middle::stability;
 use rustc_middle::ty::{self, TyCtxt};
 use rustc_span::hygiene::MacroKind;
 use rustc_span::symbol::{kw, sym, Symbol};
+use rustc_target::abi::VariantIdx;
 use std::cell::{RefCell, RefMut};
 use std::cmp::Ordering;
 use std::fmt;
@@ -1257,13 +1259,14 @@ fn item_type_alias(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, t: &c
                         w,
                         cx,
                         Some(&t.generics),
-                        variants_iter(),
+                        &variants,
                         variants_count,
                         has_stripped_entries,
                         *is_non_exhaustive,
+                        it.def_id().unwrap(),
                     )
                 });
-                item_variants(w, cx, it, variants_iter());
+                item_variants(w, cx, it, &variants);
             }
             clean::TypeAliasInnerType::Union { fields } => {
                 wrap_item(w, |w| {
@@ -1416,36 +1419,81 @@ fn item_enum(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, e: &clean::
             it.name.unwrap(),
             e.generics.print(cx),
         );
+
         render_enum_fields(
             w,
             cx,
             Some(&e.generics),
-            e.variants(),
+            &e.variants,
             count_variants,
             e.has_stripped_entries(),
             it.is_non_exhaustive(),
+            it.def_id().unwrap(),
         );
     });
 
     write!(w, "{}", document(cx, it, None, HeadingOffset::H2));
 
     if count_variants != 0 {
-        item_variants(w, cx, it, e.variants());
+        item_variants(w, cx, it, &e.variants);
     }
     let def_id = it.item_id.expect_def_id();
     write!(w, "{}", render_assoc_items(cx, it, def_id, AssocItemRender::All));
     write!(w, "{}", document_type_layout(cx, def_id));
 }
 
-fn render_enum_fields<'a>(
+/// It'll return true if all variants are C-like variants and if at least one of them has a value
+/// set.
+fn should_show_enum_discriminant(variants: &IndexVec<VariantIdx, clean::Item>) -> bool {
+    let mut has_variants_with_value = false;
+    for variant in variants {
+        if let clean::VariantItem(ref var) = *variant.kind &&
+            matches!(var.kind, clean::VariantKind::CLike)
+        {
+            has_variants_with_value |= var.discriminant.is_some();
+        } else {
+            return false;
+        }
+    }
+    has_variants_with_value
+}
+
+fn display_c_like_variant(
+    w: &mut Buffer,
+    cx: &mut Context<'_>,
+    item: &clean::Item,
+    variant: &clean::Variant,
+    index: VariantIdx,
+    should_show_enum_discriminant: bool,
+    enum_def_id: DefId,
+) {
+    let name = item.name.unwrap();
+    if let Some(ref value) = variant.discriminant {
+        write!(w, "{} = {}", name.as_str(), value.value(cx.tcx(), true));
+    } else if should_show_enum_discriminant {
+        let adt_def = cx.tcx().adt_def(enum_def_id);
+        let discr = adt_def.discriminant_for_variant(cx.tcx(), index);
+        if discr.ty.is_signed() {
+            write!(w, "{} = {}", name.as_str(), discr.val as i128);
+        } else {
+            write!(w, "{} = {}", name.as_str(), discr.val);
+        }
+    } else {
+        w.write_str(name.as_str());
+    }
+}
+
+fn render_enum_fields(
     mut w: &mut Buffer,
     cx: &mut Context<'_>,
     g: Option<&clean::Generics>,
-    variants: impl Iterator<Item = &'a clean::Item>,
+    variants: &IndexVec<VariantIdx, clean::Item>,
     count_variants: usize,
     has_stripped_entries: bool,
     is_non_exhaustive: bool,
+    enum_def_id: DefId,
 ) {
+    let should_show_enum_discriminant = should_show_enum_discriminant(variants);
     if !g.is_some_and(|g| print_where_clause_and_check(w, g, cx)) {
         // If there wasn't a `where` clause, we add a whitespace.
         w.write_str(" ");
@@ -1461,15 +1509,24 @@ fn render_enum_fields<'a>(
             toggle_open(&mut w, format_args!("{count_variants} variants"));
         }
         const TAB: &str = "    ";
-        for v in variants {
+        for (index, v) in variants.iter_enumerated() {
+            if v.is_stripped() {
+                continue;
+            }
             w.write_str(TAB);
-            let name = v.name.unwrap();
             match *v.kind {
-                // FIXME(#101337): Show discriminant
                 clean::VariantItem(ref var) => match var.kind {
-                    clean::VariantKind::CLike => w.write_str(name.as_str()),
+                    clean::VariantKind::CLike => display_c_like_variant(
+                        w,
+                        cx,
+                        v,
+                        var,
+                        index,
+                        should_show_enum_discriminant,
+                        enum_def_id,
+                    ),
                     clean::VariantKind::Tuple(ref s) => {
-                        write!(w, "{name}({})", print_tuple_struct_fields(cx, s),);
+                        write!(w, "{}({})", v.name.unwrap(), print_tuple_struct_fields(cx, s));
                     }
                     clean::VariantKind::Struct(ref s) => {
                         render_struct(w, v, None, None, &s.fields, TAB, false, cx);
@@ -1490,11 +1547,11 @@ fn render_enum_fields<'a>(
     }
 }
 
-fn item_variants<'a>(
+fn item_variants(
     w: &mut Buffer,
     cx: &mut Context<'_>,
     it: &clean::Item,
-    variants: impl Iterator<Item = &'a clean::Item>,
+    variants: &IndexVec<VariantIdx, clean::Item>,
 ) {
     let tcx = cx.tcx();
     write!(
@@ -1507,7 +1564,11 @@ fn item_variants<'a>(
         document_non_exhaustive_header(it),
         document_non_exhaustive(it)
     );
-    for variant in variants {
+    let should_show_enum_discriminant = should_show_enum_discriminant(variants);
+    for (index, variant) in variants.iter_enumerated() {
+        if variant.is_stripped() {
+            continue;
+        }
         let id = cx.derive_id(format!("{}.{}", ItemType::Variant, variant.name.unwrap()));
         write!(
             w,
@@ -1522,7 +1583,22 @@ fn item_variants<'a>(
             it.const_stable_since(tcx),
             " rightside",
         );
-        write!(w, "<h3 class=\"code-header\">{name}", name = variant.name.unwrap());
+        w.write_str("<h3 class=\"code-header\">");
+        if let clean::VariantItem(ref var) = *variant.kind &&
+            let clean::VariantKind::CLike = var.kind
+        {
+            display_c_like_variant(
+                w,
+                cx,
+                variant,
+                var,
+                index,
+                should_show_enum_discriminant,
+                it.def_id().unwrap(),
+            );
+        } else {
+            w.write_str(variant.name.unwrap().as_str());
+        }
 
         let clean::VariantItem(variant_data) = &*variant.kind else { unreachable!() };
 
diff --git a/src/librustdoc/json/conversions.rs b/src/librustdoc/json/conversions.rs
index 472cde51be5..e7f782bb6a6 100644
--- a/src/librustdoc/json/conversions.rs
+++ b/src/librustdoc/json/conversions.rs
@@ -744,7 +744,7 @@ impl FromWithTcx<clean::Discriminant> for Discriminant {
             // `rustc_middle` types, not `rustc_hir`, but because JSON never inlines
             // the expr is always some.
             expr: disr.expr(tcx).unwrap(),
-            value: disr.value(tcx),
+            value: disr.value(tcx, false),
         }
     }
 }
diff --git a/tests/rustdoc/auxiliary/enum-variant.rs b/tests/rustdoc/auxiliary/enum-variant.rs
new file mode 100644
index 00000000000..90c71b86329
--- /dev/null
+++ b/tests/rustdoc/auxiliary/enum-variant.rs
@@ -0,0 +1,24 @@
+#![crate_name = "bar"]
+
+pub enum E {
+    A = 12,
+    B,
+    C = 1245,
+}
+
+pub enum F {
+    A,
+    B,
+}
+
+#[repr(u32)]
+pub enum G {
+    A = 12,
+    B,
+    C(u32),
+}
+
+pub enum H {
+    A,
+    C(u32),
+}
diff --git a/tests/rustdoc/enum-variant-value.rs b/tests/rustdoc/enum-variant-value.rs
new file mode 100644
index 00000000000..c306736bfe9
--- /dev/null
+++ b/tests/rustdoc/enum-variant-value.rs
@@ -0,0 +1,117 @@
+// This test ensures that the variant value is displayed with underscores but without
+// a type name at the end.
+
+// aux-build:enum-variant.rs
+
+#![crate_name = "foo"]
+
+extern crate bar;
+
+// In this case, since all variants are C-like variants and at least one of them
+// has its value set, we display values for all of them.
+
+// @has 'foo/enum.A.html'
+// @has - '//*[@class="rust item-decl"]/code' 'A = 12,'
+// @has - '//*[@class="rust item-decl"]/code' 'B = 13,'
+// @has - '//*[@class="rust item-decl"]/code' 'C = 1_245,'
+// @matches - '//*[@id="variant.A"]/h3' '^A = 12$'
+// @matches - '//*[@id="variant.B"]/h3' '^B = 13$'
+// @matches - '//*[@id="variant.C"]/h3' '^C = 1_245$'
+pub enum A {
+    A = 12,
+    B,
+    C = 1245,
+}
+
+// In this case, all variants are C-like variants but none of them has its value set.
+// Therefore we don't display values.
+
+// @has 'foo/enum.B.html'
+// @has - '//*[@class="rust item-decl"]/code' 'A,'
+// @has - '//*[@class="rust item-decl"]/code' 'B,'
+// @matches - '//*[@id="variant.A"]/h3' '^A$'
+// @matches - '//*[@id="variant.B"]/h3' '^B$'
+pub enum B {
+    A,
+    B,
+}
+
+// In this case, not all variants are C-like variants so we don't display values.
+
+// @has 'foo/enum.C.html'
+// @has - '//*[@class="rust item-decl"]/code' 'A = 12,'
+// @has - '//*[@class="rust item-decl"]/code' 'B,'
+// @has - '//*[@class="rust item-decl"]/code' 'C(u32),'
+// @matches - '//*[@id="variant.A"]/h3' '^A = 12$'
+// @matches - '//*[@id="variant.B"]/h3' '^B$'
+// @has - '//*[@id="variant.C"]/h3' 'C(u32)'
+#[repr(u32)]
+pub enum C {
+    A = 12,
+    B,
+    C(u32),
+}
+
+// In this case, not all variants are C-like variants and no C-like variant has its
+// value set, so we don't display values.
+
+// @has 'foo/enum.D.html'
+// @has - '//*[@class="rust item-decl"]/code' 'A,'
+// @has - '//*[@class="rust item-decl"]/code' 'C(u32),'
+// @matches - '//*[@id="variant.A"]/h3' '^A$'
+// @has - '//*[@id="variant.C"]/h3' 'C(u32)'
+pub enum D {
+    A,
+    C(u32),
+}
+
+// @has 'foo/enum.E.html'
+// @has - '//*[@class="rust item-decl"]/code' 'A = 12,'
+// @has - '//*[@class="rust item-decl"]/code' 'B = 13,'
+// @has - '//*[@class="rust item-decl"]/code' 'C = 1_245,'
+// @matches - '//*[@id="variant.A"]/h3' '^A = 12$'
+// @matches - '//*[@id="variant.B"]/h3' '^B = 13$'
+// @matches - '//*[@id="variant.C"]/h3' '^C = 1_245$'
+pub use bar::E;
+
+// @has 'foo/enum.F.html'
+// @has - '//*[@class="rust item-decl"]/code' 'A,'
+// @has - '//*[@class="rust item-decl"]/code' 'B,'
+// @matches - '//*[@id="variant.A"]/h3' '^A$'
+// @matches - '//*[@id="variant.B"]/h3' '^B$'
+pub use bar::F;
+
+// @has 'foo/enum.G.html'
+// @has - '//*[@class="rust item-decl"]/code' 'A = 12,'
+// @has - '//*[@class="rust item-decl"]/code' 'B,'
+// @has - '//*[@class="rust item-decl"]/code' 'C(u32),'
+// @matches - '//*[@id="variant.A"]/h3' '^A = 12$'
+// @matches - '//*[@id="variant.B"]/h3' '^B$'
+// @has - '//*[@id="variant.C"]/h3' 'C(u32)'
+pub use bar::G;
+
+// @has 'foo/enum.H.html'
+// @has - '//*[@class="rust item-decl"]/code' 'A,'
+// @has - '//*[@class="rust item-decl"]/code' 'C(u32),'
+// @matches - '//*[@id="variant.A"]/h3' '^A$'
+// @has - '//*[@id="variant.C"]/h3' 'C(u32)'
+pub use bar::H;
+
+// Testing more complex cases.
+pub const X: isize = 2;
+// @has 'foo/enum.I.html'
+// @has - '//*[@class="rust item-decl"]/code' 'A = 2,'
+// @has - '//*[@class="rust item-decl"]/code' 'B = 4,'
+// @has - '//*[@class="rust item-decl"]/code' 'C = 9,'
+// @has - '//*[@class="rust item-decl"]/code' 'D = -1,'
+// @matches - '//*[@id="variant.A"]/h3' '^A = 2$'
+// @matches - '//*[@id="variant.B"]/h3' '^B = 4$'
+// @matches - '//*[@id="variant.C"]/h3' '^C = 9$'
+// @matches - '//*[@id="variant.D"]/h3' '^D = -1$'
+#[repr(isize)]
+pub enum I {
+    A = X,
+    B = X * 2,
+    C = Self::B as isize + X + 3,
+    D = -1,
+}