Skip to content

Show enum discriminant if a compatible repr is used #116600

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 12, 2023
Merged

Show enum discriminant if a compatible repr is used #116600

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d6056ec
Show enum discriminant if a compatible repr is used
GuillaumeGomez Oct 10, 2023
bd59fc6
Add tests for enum discriminant value display with `repr`
GuillaumeGomez Oct 10, 2023
00c3de9
Improve code documentation a bit
GuillaumeGomez Oct 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 24 additions & 10 deletions src/librustdoc/html/render/print_item.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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();
Comment on lines +1253 to +1254
Copy link
Member

@fmease fmease Oct 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish we didn't need to do this and could just store the DefId of the inner item inside TypeAliasInnerType and obtain it directly via adt.did() in clean_ty_alias_inner_type but that would increase the size of the type ... so your code is fine.

Copy link
Member Author

@GuillaumeGomez GuillaumeGomez Oct 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately yes. We could maybe add a helper method later on if we need to do it once again though.


wrap_item(w, |w| {
let variants_len = variants.len();
let variants_count = variants_iter().count();
Expand All @@ -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| {
Expand Down Expand Up @@ -1435,16 +1439,21 @@ 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<VariantIdx, clean::Item>) -> 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 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,
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 &&
Expand All @@ -1455,7 +1464,11 @@ fn should_show_enum_discriminant(variants: &IndexVec<VariantIdx, clean::Item>) -
return false;
}
}
has_variants_with_value
if has_variants_with_value {
return true;
}
let repr = cx.tcx().adt_def(enum_def_id).repr();
repr.c() || repr.int.is_some()
}

fn display_c_like_variant(
Expand Down Expand Up @@ -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(" ");
Expand Down Expand Up @@ -1552,6 +1565,7 @@ fn item_variants(
cx: &mut Context<'_>,
it: &clean::Item,
variants: &IndexVec<VariantIdx, clean::Item>,
enum_def_id: DefId,
) {
let tcx = cx.tcx();
write!(
Expand All @@ -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;
Expand Down Expand Up @@ -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());
Expand Down
24 changes: 24 additions & 0 deletions tests/rustdoc/auxiliary/enum-variant.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
74 changes: 74 additions & 0 deletions tests/rustdoc/enum-variant-value.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;