From d6056ecb0a4938a594d6cc8ed7eff8046271928f Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 10 Oct 2023 16:18:49 +0200 Subject: [PATCH 1/3] Show enum discriminant if a compatible repr is used --- src/librustdoc/html/render/print_item.rs | 34 +++++++++++++++++------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index 467493cb0b30d..3873a3f35222d 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -7,6 +7,7 @@ use rustc_hir::def::CtorKind; use rustc_hir::def_id::DefId; use rustc_index::IndexVec; use rustc_middle::middle::stability; +use rustc_middle::query::Key; use rustc_middle::ty::{self, TyCtxt}; use rustc_span::hygiene::MacroKind; use rustc_span::symbol::{kw, sym, Symbol}; @@ -1249,6 +1250,9 @@ fn item_type_alias(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, t: &c match inner_type { clean::TypeAliasInnerType::Enum { variants, is_non_exhaustive } => { let variants_iter = || variants.iter().filter(|i| !i.is_stripped()); + let ty = cx.tcx().type_of(it.def_id().unwrap()).instantiate_identity(); + let enum_def_id = ty.ty_adt_id().unwrap(); + wrap_item(w, |w| { let variants_len = variants.len(); let variants_count = variants_iter().count(); @@ -1263,10 +1267,10 @@ fn item_type_alias(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, t: &c variants_count, has_stripped_entries, *is_non_exhaustive, - it.def_id().unwrap(), + enum_def_id, ) }); - item_variants(w, cx, it, &variants); + item_variants(w, cx, it, &variants, enum_def_id); } clean::TypeAliasInnerType::Union { fields } => { wrap_item(w, |w| { @@ -1435,16 +1439,20 @@ fn item_enum(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, e: &clean:: 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, it.def_id().unwrap()); } 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)); } -/// 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) -> bool { +/// It'll return false if any variant is not a C-like variant. Otherwise it'll return true if at +/// least one of them has a value set or if the enum has `#[repr(C)]` or has an integer `repr`. +fn should_show_enum_discriminant( + cx: &Context<'_>, + enum_def_id: DefId, + variants: &IndexVec, +) -> bool { let mut has_variants_with_value = false; for variant in variants { if let clean::VariantItem(ref var) = *variant.kind && @@ -1455,7 +1463,12 @@ fn should_show_enum_discriminant(variants: &IndexVec) - return false; } } - has_variants_with_value + if has_variants_with_value { + return true; + } + let repr = cx.tcx().adt_def(enum_def_id).repr(); + // let repr = cx.tcx().repr_options_of_def(enum_def_id); + repr.c() || repr.int.is_some() } fn display_c_like_variant( @@ -1493,7 +1506,7 @@ fn render_enum_fields( is_non_exhaustive: bool, enum_def_id: DefId, ) { - let should_show_enum_discriminant = should_show_enum_discriminant(variants); + let should_show_enum_discriminant = should_show_enum_discriminant(cx, enum_def_id, 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(" "); @@ -1552,6 +1565,7 @@ fn item_variants( cx: &mut Context<'_>, it: &clean::Item, variants: &IndexVec, + enum_def_id: DefId, ) { let tcx = cx.tcx(); write!( @@ -1564,7 +1578,7 @@ fn item_variants( document_non_exhaustive_header(it), document_non_exhaustive(it) ); - let should_show_enum_discriminant = should_show_enum_discriminant(variants); + let should_show_enum_discriminant = should_show_enum_discriminant(cx, enum_def_id, variants); for (index, variant) in variants.iter_enumerated() { if variant.is_stripped() { continue; @@ -1594,7 +1608,7 @@ fn item_variants( var, index, should_show_enum_discriminant, - it.def_id().unwrap(), + enum_def_id, ); } else { w.write_str(variant.name.unwrap().as_str()); From bd59fc603f57f9cf9f8bae4ddfdf79a883129e76 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 10 Oct 2023 16:55:39 +0200 Subject: [PATCH 2/3] Add tests for enum discriminant value display with `repr` --- tests/rustdoc/auxiliary/enum-variant.rs | 24 ++++++++ tests/rustdoc/enum-variant-value.rs | 74 +++++++++++++++++++++++++ 2 files changed, 98 insertions(+) diff --git a/tests/rustdoc/auxiliary/enum-variant.rs b/tests/rustdoc/auxiliary/enum-variant.rs index 90c71b863290d..a0a7fd894f902 100644 --- a/tests/rustdoc/auxiliary/enum-variant.rs +++ b/tests/rustdoc/auxiliary/enum-variant.rs @@ -22,3 +22,27 @@ pub enum H { A, C(u32), } + +#[repr(C)] +pub enum N { + A, + B, +} + +#[repr(C)] +pub enum O { + A(u32), + B, +} + +#[repr(u32)] +pub enum P { + A, + B, +} + +#[repr(u32)] +pub enum Q { + A(u32), + B, +} diff --git a/tests/rustdoc/enum-variant-value.rs b/tests/rustdoc/enum-variant-value.rs index c306736bfe919..096f8cd4122f8 100644 --- a/tests/rustdoc/enum-variant-value.rs +++ b/tests/rustdoc/enum-variant-value.rs @@ -115,3 +115,77 @@ pub enum I { C = Self::B as isize + X + 3, D = -1, } + +// Testing `repr`. + +// @has 'foo/enum.J.html' +// @has - '//*[@class="rust item-decl"]/code' 'A = 0,' +// @has - '//*[@class="rust item-decl"]/code' 'B = 1,' +// @matches - '//*[@id="variant.A"]/h3' '^A = 0$' +// @matches - '//*[@id="variant.B"]/h3' '^B = 1$' +#[repr(C)] +pub enum J { + A, + B, +} + +// @has 'foo/enum.K.html' +// @has - '//*[@class="rust item-decl"]/code' 'A(u32),' +// @has - '//*[@class="rust item-decl"]/code' 'B,' +// @has - '//*[@id="variant.A"]/h3' 'A(u32)' +// @matches - '//*[@id="variant.B"]/h3' '^B$' +#[repr(C)] +pub enum K { + A(u32), + B, +} + +// @has 'foo/enum.L.html' +// @has - '//*[@class="rust item-decl"]/code' 'A = 0,' +// @has - '//*[@class="rust item-decl"]/code' 'B = 1,' +// @matches - '//*[@id="variant.A"]/h3' '^A = 0$' +// @matches - '//*[@id="variant.B"]/h3' '^B = 1$' +#[repr(u32)] +pub enum L { + A, + B, +} + +// @has 'foo/enum.M.html' +// @has - '//*[@class="rust item-decl"]/code' 'A(u32),' +// @has - '//*[@class="rust item-decl"]/code' 'B,' +// @has - '//*[@id="variant.A"]/h3' 'A(u32)' +// @matches - '//*[@id="variant.B"]/h3' '^B$' +#[repr(u32)] +pub enum M { + A(u32), + B, +} + +// @has 'foo/enum.N.html' +// @has - '//*[@class="rust item-decl"]/code' 'A = 0,' +// @has - '//*[@class="rust item-decl"]/code' 'B = 1,' +// @matches - '//*[@id="variant.A"]/h3' '^A = 0$' +// @matches - '//*[@id="variant.B"]/h3' '^B = 1$' +pub use bar::N; + +// @has 'foo/enum.O.html' +// @has - '//*[@class="rust item-decl"]/code' 'A(u32),' +// @has - '//*[@class="rust item-decl"]/code' 'B,' +// @has - '//*[@id="variant.A"]/h3' 'A(u32)' +// @matches - '//*[@id="variant.B"]/h3' '^B$' +pub use bar::O; + +// @has 'foo/enum.P.html' +// @has - '//*[@class="rust item-decl"]/code' 'A = 0,' +// @has - '//*[@class="rust item-decl"]/code' 'B = 1,' +// @matches - '//*[@id="variant.A"]/h3' '^A = 0$' +// @matches - '//*[@id="variant.B"]/h3' '^B = 1$' +pub use bar::P; + +// @has 'foo/enum.Q.html' +// @has - '//*[@class="rust item-decl"]/code' 'A(u32),' +// @has - '//*[@class="rust item-decl"]/code' 'B,' +// @has - '//*[@id="variant.A"]/h3' 'A(u32)' +// @matches - '//*[@id="variant.B"]/h3' '^B$' +pub use bar::Q; From 00c3de96e689d4b20be218692f88f8afa88ced87 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 11 Oct 2023 23:46:11 +0200 Subject: [PATCH 3/3] Improve code documentation a bit --- src/librustdoc/html/render/print_item.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index 3873a3f35222d..cd31493bf3ff8 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -1447,7 +1447,8 @@ fn item_enum(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, e: &clean:: } /// It'll return false if any variant is not a C-like variant. Otherwise it'll return true if at -/// least one of them has a value set or if the enum has `#[repr(C)]` or has an integer `repr`. +/// least one of them has an explicit discriminant or if the enum has `#[repr(C)]` or an integer +/// `repr`. fn should_show_enum_discriminant( cx: &Context<'_>, enum_def_id: DefId, @@ -1467,7 +1468,6 @@ fn should_show_enum_discriminant( return true; } let repr = cx.tcx().adt_def(enum_def_id).repr(); - // let repr = cx.tcx().repr_options_of_def(enum_def_id); repr.c() || repr.int.is_some() }