Skip to content
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

Jump to bottom

Fix const deref methods display #91291

Merged
merged 4 commits into from
Dec 2, 2021
Merged

Fix const deref methods display #91291

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
588a99b
Fix display for "const" deref methods in rustdoc
GuillaumeGomez Nov 27, 2021
0c1b0f7
Add test for const deref methods display
GuillaumeGomez Nov 27, 2021
b647c90
Improve code by replacing &str with Symbol in render_stability_since_raw
GuillaumeGomez Nov 29, 2021
02782bb
Return ConstStability instead of &ConstStability in Item::const_stabi…
GuillaumeGomez Nov 29, 2021
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
12 changes: 6 additions & 6 deletions src/librustdoc/clean/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,8 +376,8 @@ impl Item {
self.def_id.as_def_id().and_then(|did| tcx.lookup_stability(did))
}

crate fn const_stability<'tcx>(&self, tcx: TyCtxt<'tcx>) -> Option<&'tcx ConstStability> {
self.def_id.as_def_id().and_then(|did| tcx.lookup_const_stability(did))
crate fn const_stability<'tcx>(&self, tcx: TyCtxt<'tcx>) -> Option<ConstStability> {
camelid marked this conversation as resolved.
Show resolved Hide resolved
self.def_id.as_def_id().and_then(|did| tcx.lookup_const_stability(did)).map(|cs| *cs)
}

crate fn deprecation(&self, tcx: TyCtxt<'_>) -> Option<Deprecation> {
Expand Down Expand Up @@ -602,16 +602,16 @@ impl Item {
})
}

crate fn stable_since(&self, tcx: TyCtxt<'_>) -> Option<SymbolStr> {
crate fn stable_since(&self, tcx: TyCtxt<'_>) -> Option<Symbol> {
camelid marked this conversation as resolved.
Show resolved Hide resolved
match self.stability(tcx)?.level {
StabilityLevel::Stable { since, .. } => Some(since.as_str()),
StabilityLevel::Stable { since, .. } => Some(since),
StabilityLevel::Unstable { .. } => None,
}
}

crate fn const_stable_since(&self, tcx: TyCtxt<'_>) -> Option<SymbolStr> {
crate fn const_stable_since(&self, tcx: TyCtxt<'_>) -> Option<Symbol> {
match self.const_stability(tcx)?.level {
StabilityLevel::Stable { since, .. } => Some(since.as_str()),
StabilityLevel::Stable { since, .. } => Some(since),
StabilityLevel::Unstable { .. } => None,
}
}
Expand Down
5 changes: 1 addition & 4 deletions src/librustdoc/html/format.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1349,10 +1349,7 @@ impl PrintWithSpace for hir::Mutability {
}
}

crate fn print_constness_with_space(
c: &hir::Constness,
s: Option<&ConstStability>,
) -> &'static str {
crate fn print_constness_with_space(c: &hir::Constness, s: Option<ConstStability>) -> &'static str {
match (c, s) {
// const stable or when feature(staged_api) is not set
(
Expand Down
49 changes: 33 additions & 16 deletions src/librustdoc/html/render/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -804,17 +804,17 @@ fn assoc_type(

fn render_stability_since_raw(
w: &mut Buffer,
ver: Option<&str>,
const_stability: Option<&ConstStability>,
containing_ver: Option<&str>,
containing_const_ver: Option<&str>,
ver: Option<Symbol>,
const_stability: Option<ConstStability>,
containing_ver: Option<Symbol>,
containing_const_ver: Option<Symbol>,
) {
let ver = ver.filter(|inner| !inner.is_empty());

match (ver, const_stability) {
// stable and const stable
(Some(v), Some(ConstStability { level: StabilityLevel::Stable { since }, .. }))
if Some(since.as_str()).as_deref() != containing_const_ver =>
if Some(since) != containing_const_ver =>
{
write!(
w,
Expand Down Expand Up @@ -861,6 +861,7 @@ fn render_assoc_item(
link: AssocItemLink<'_>,
parent: ItemType,
cx: &Context<'_>,
render_mode: RenderMode,
camelid marked this conversation as resolved.
Show resolved Hide resolved
) {
fn method(
w: &mut Buffer,
Expand All @@ -871,6 +872,7 @@ fn render_assoc_item(
link: AssocItemLink<'_>,
parent: ItemType,
cx: &Context<'_>,
render_mode: RenderMode,
) {
let name = meth.name.as_ref().unwrap();
let href = match link {
Expand All @@ -893,8 +895,14 @@ fn render_assoc_item(
}
};
let vis = meth.visibility.print_with_space(meth.def_id, cx).to_string();
let constness =
print_constness_with_space(&header.constness, meth.const_stability(cx.tcx()));
// FIXME: Once https://github.com/rust-lang/rust/issues/67792 is implemented, we can remove
// this condition.
let constness = match render_mode {
RenderMode::Normal => {
print_constness_with_space(&header.constness, meth.const_stability(cx.tcx()))
}
RenderMode::ForDeref { .. } => "",
};
let asyncness = header.asyncness.print_with_space();
let unsafety = header.unsafety.print_with_space();
let defaultness = print_default_space(meth.is_default());
Expand Down Expand Up @@ -945,10 +953,10 @@ fn render_assoc_item(
match *item.kind {
clean::StrippedItem(..) => {}
clean::TyMethodItem(ref m) => {
method(w, item, m.header, &m.generics, &m.decl, link, parent, cx)
method(w, item, m.header, &m.generics, &m.decl, link, parent, cx, render_mode)
}
clean::MethodItem(ref m, _) => {
method(w, item, m.header, &m.generics, &m.decl, link, parent, cx)
method(w, item, m.header, &m.generics, &m.decl, link, parent, cx, render_mode)
}
clean::AssocConstItem(ref ty, ref default) => assoc_const(
w,
Expand Down Expand Up @@ -1415,7 +1423,7 @@ fn render_impl(
"<div id=\"{}\" class=\"{}{} has-srclink\">",
id, item_type, in_trait_class,
);
render_rightside(w, cx, item, containing_item);
render_rightside(w, cx, item, containing_item, render_mode);
write!(w, "<a href=\"#{}\" class=\"anchor\"></a>", id);
w.write_str("<h4 class=\"code-header\">");
render_assoc_item(
Expand All @@ -1424,6 +1432,7 @@ fn render_impl(
link.anchor(source_id.as_ref().unwrap_or(&id)),
ItemType::Impl,
cx,
render_mode,
);
w.write_str("</h4>");
w.write_str("</div>");
Expand Down Expand Up @@ -1459,7 +1468,7 @@ fn render_impl(
"<div id=\"{}\" class=\"{}{} has-srclink\">",
id, item_type, in_trait_class
);
render_rightside(w, cx, item, containing_item);
render_rightside(w, cx, item, containing_item, render_mode);
write!(w, "<a href=\"#{}\" class=\"anchor\"></a>", id);
w.write_str("<h4 class=\"code-header\">");
assoc_const(
Expand Down Expand Up @@ -1638,16 +1647,24 @@ fn render_rightside(
cx: &Context<'_>,
item: &clean::Item,
containing_item: &clean::Item,
render_mode: RenderMode,
) {
let tcx = cx.tcx();

// FIXME: Once https://github.com/rust-lang/rust/issues/67792 is implemented, we can remove
// this condition.
let (const_stability, const_stable_since) = match render_mode {
RenderMode::Normal => (item.const_stability(tcx), containing_item.const_stable_since(tcx)),
RenderMode::ForDeref { .. } => (None, None),
};

write!(w, "<div class=\"rightside\">");
render_stability_since_raw(
w,
item.stable_since(tcx).as_deref(),
item.const_stability(tcx),
containing_item.stable_since(tcx).as_deref(),
containing_item.const_stable_since(tcx).as_deref(),
item.stable_since(tcx),
const_stability,
containing_item.stable_since(tcx),
const_stable_since,
);

write_srclink(cx, item, w);
Expand Down Expand Up @@ -1683,7 +1700,7 @@ pub(crate) fn render_impl_summary(
format!(" data-aliases=\"{}\"", aliases.join(","))
};
write!(w, "<div id=\"{}\" class=\"impl has-srclink\"{}>", id, aliases);
render_rightside(w, cx, &i.impl_item, containing_item);
render_rightside(w, cx, &i.impl_item, containing_item, RenderMode::Normal);
GuillaumeGomez marked this conversation as resolved.
Show resolved Hide resolved
write!(w, "<a href=\"#{}\" class=\"anchor\"></a>", id);
write!(w, "<h3 class=\"code-header in-band\">");

Expand Down
53 changes: 44 additions & 9 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 @@ -101,7 +101,7 @@ pub(super) fn print_item(
let mut stability_since_raw = Buffer::new();
render_stability_since_raw(
&mut stability_since_raw,
item.stable_since(cx.tcx()).as_deref(),
item.stable_since(cx.tcx()),
item.const_stability(cx.tcx()),
None,
None,
Expand Down Expand Up @@ -556,7 +556,14 @@ fn item_trait(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Tra
);
}
for t in &types {
render_assoc_item(w, t, AssocItemLink::Anchor(None), ItemType::Trait, cx);
render_assoc_item(
w,
t,
AssocItemLink::Anchor(None),
ItemType::Trait,
cx,
RenderMode::Normal,
);
w.write_str(";\n");
}
// If there are too many associated constants, hide everything after them
Expand All @@ -580,7 +587,14 @@ fn item_trait(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Tra
w.write_str("\n");
}
for t in &consts {
render_assoc_item(w, t, AssocItemLink::Anchor(None), ItemType::Trait, cx);
render_assoc_item(
w,
t,
AssocItemLink::Anchor(None),
ItemType::Trait,
cx,
RenderMode::Normal,
);
w.write_str(";\n");
}
if !toggle && should_hide_fields(count_methods) {
Expand All @@ -591,7 +605,14 @@ fn item_trait(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Tra
w.write_str("\n");
}
for (pos, m) in required.iter().enumerate() {
render_assoc_item(w, m, AssocItemLink::Anchor(None), ItemType::Trait, cx);
render_assoc_item(
w,
m,
AssocItemLink::Anchor(None),
ItemType::Trait,
cx,
RenderMode::Normal,
);
w.write_str(";\n");

if pos < required.len() - 1 {
Expand All @@ -602,7 +623,14 @@ fn item_trait(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Tra
w.write_str("\n");
}
for (pos, m) in provided.iter().enumerate() {
render_assoc_item(w, m, AssocItemLink::Anchor(None), ItemType::Trait, cx);
render_assoc_item(
w,
m,
AssocItemLink::Anchor(None),
ItemType::Trait,
cx,
RenderMode::Normal,
);
match *m.kind {
clean::MethodItem(ref inner, _)
if !inner.generics.where_predicates.is_empty() =>
Expand Down Expand Up @@ -655,7 +683,14 @@ fn item_trait(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Tra
write_srclink(cx, m, w);
write!(w, "</div>");
write!(w, "<h4 class=\"code-header\">");
render_assoc_item(w, m, AssocItemLink::Anchor(Some(&id)), ItemType::Impl, cx);
render_assoc_item(
w,
m,
AssocItemLink::Anchor(Some(&id)),
ItemType::Impl,
cx,
RenderMode::Normal,
);
w.write_str("</h4>");
w.write_str("</div>");
if toggled {
Expand Down Expand Up @@ -1427,10 +1462,10 @@ fn render_stability_since(
) {
render_stability_since_raw(
w,
item.stable_since(tcx).as_deref(),
item.stable_since(tcx),
item.const_stability(tcx),
containing_item.stable_since(tcx).as_deref(),
containing_item.const_stable_since(tcx).as_deref(),
containing_item.stable_since(tcx),
containing_item.const_stable_since(tcx),
)
}

Expand Down
38 changes: 38 additions & 0 deletions src/test/rustdoc/deref-const-fn.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// This test ensures that the const methods from Deref aren't shown as const.
// For more information, see https://github.com/rust-lang/rust/issues/90855.

#![crate_name = "foo"]

#![feature(staged_api)]

#![stable(feature = "rust1", since = "1.0.0")]

// @has 'foo/struct.Bar.html'
#[stable(feature = "rust1", since = "1.0.0")]
pub struct Bar;

impl Bar {
// @has - '//*[@id="method.len"]' 'pub const fn len(&self) -> usize'
// @has - '//*[@id="method.len"]//span[@class="since"]' '1.0.0 (const: 1.0.0)'
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_stable(feature = "rust1", since = "1.0.0")]
pub const fn len(&self) -> usize { 0 }
GuillaumeGomez marked this conversation as resolved.
Show resolved Hide resolved
}

#[stable(feature = "rust1", since = "1.0.0")]
pub struct Foo {
value: Bar,
}

// @has 'foo/struct.Foo.html'
// @has - '//*[@id="method.len"]' 'pub fn len(&self) -> usize'
// @!has - '//*[@id="method.len"]//span[@class="since"]' '1.0.0'
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, shouldn't stability be shown? Or I guess it depends on when the Deref impl was added too.

Weirdly, https://doc.rust-lang.org/nightly/std/string/struct.String.html#deref-methods-str shows regular and const stability, but https://doc.rust-lang.org/nightly/std/path/struct.PathBuf.html#deref-methods-Path shows neither. Something fishy's going on with re-exports, since https://doc.rust-lang.org/nightly/alloc/string/struct.String.html#deref-methods-str shows stability for some methods but not all and is missing many methods that the std version has. (I think I opened an issue for part of this at some point.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Since you already opened the issue, I'll mark this discussion as resolved.

Copy link
Member

@camelid camelid Dec 1, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

No, the issue I opened was just about how different methods were shown between std and alloc. But I guess this bug is unrelated to this PR?

Copy link
Member

Choose a reason for hiding this comment

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

I thought 1.0.0 without the const version is hidden. Maybe that's why? If there is a stable const version, then we show 1.0.0 (const: 1.xx.0) or const: unstable.

// @!has - '//*[@id="method.len"]//span[@class="since"]' '(const: 1.0.0)'
#[stable(feature = "rust1", since = "1.0.0")]
impl std::ops::Deref for Foo {
type Target = Bar;

fn deref(&self) -> &Self::Target {
&self.value
}
}