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.

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

rustdoc: decouple stability and const-stability #91694

Merged
merged 1 commit into from
Jan 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
91 changes: 55 additions & 36 deletions src/librustdoc/html/render/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -792,58 +792,77 @@ fn assoc_type(
}
}

/// Writes a span containing the versions at which an item became stable and/or const-stable. For
/// example, if the item became stable at 1.0.0, and const-stable at 1.45.0, this function would
/// write a span containing "1.0.0 (const: 1.45.0)".
///
/// Returns `true` if a stability annotation was rendered.
///
/// Stability and const-stability are considered separately. If the item is unstable, no version
/// will be written. If the item is const-unstable, "const: unstable" will be appended to the
/// span, with a link to the tracking issue if present. If an item's stability or const-stability
/// version matches the version of its enclosing item, that version will be omitted.
///
/// Note that it is possible for an unstable function to be const-stable. In that case, the span
/// will include the const-stable version, but no stable version will be emitted, as a natural
/// consequence of the above rules.
fn render_stability_since_raw(
w: &mut Buffer,
ver: Option<Symbol>,
const_stability: Option<ConstStability>,
containing_ver: Option<Symbol>,
containing_const_ver: Option<Symbol>,
) -> bool {
let ver = ver.filter(|inner| !inner.is_empty());
let stable_version = ver.filter(|inner| !inner.is_empty() && Some(*inner) != containing_ver);

match (ver, const_stability) {
// stable and const stable
(Some(v), Some(ConstStability { level: StabilityLevel::Stable { since }, .. }))
let mut title = String::new();
let mut stability = String::new();

if let Some(ver) = stable_version {
stability.push_str(&ver.as_str());
title.push_str(&format!("Stable since Rust version {}", ver));
}

let const_title_and_stability = match const_stability {
Some(ConstStability { level: StabilityLevel::Stable { since }, .. })
if Some(since) != containing_const_ver =>
{
write!(
w,
"<span class=\"since\" title=\"Stable since Rust version {0}, const since {1}\">{0} (const: {1})</span>",
v, since
);
Some((format!("const since {}", since), format!("const: {}", since)))
}
// stable and const unstable
(
Some(v),
Some(ConstStability { level: StabilityLevel::Unstable { issue, .. }, feature, .. }),
) => {
write!(
w,
"<span class=\"since\" title=\"Stable since Rust version {0}, const unstable\">{0} (const: ",
v
);
if let Some(n) = issue {
write!(
w,
"<a href=\"https://github.com/rust-lang/rust/issues/{}\" title=\"Tracking issue for {}\">unstable</a>",
Some(ConstStability { level: StabilityLevel::Unstable { issue, .. }, feature, .. }) => {
let unstable = if let Some(n) = issue {
format!(
r#"<a href="https://github.com/rust-lang/rust/issues/{}" title="Tracking issue for {}">unstable</a>"#,
n, feature
);
)
} else {
write!(w, "unstable");
}
write!(w, ")</span>");
String::from("unstable")
};

Some((String::from("const unstable"), format!("const: {}", unstable)))
}
// stable
(Some(v), _) if ver != containing_ver => {
write!(
w,
"<span class=\"since\" title=\"Stable since Rust version {0}\">{0}</span>",
v
);
_ => None,
};

if let Some((const_title, const_stability)) = const_title_and_stability {
if !title.is_empty() {
title.push_str(&format!(", {}", const_title));
} else {
title.push_str(&const_title);
}

if !stability.is_empty() {
stability.push_str(&format!(" ({})", const_stability));
} else {
stability.push_str(&const_stability);
}
_ => return false,
}
true

if !stability.is_empty() {
write!(w, r#"<span class="since" title="{}">{}</span>"#, title, stability);
}

!stability.is_empty()
}

fn render_assoc_item(
Expand Down
17 changes: 17 additions & 0 deletions src/test/rustdoc/const-display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,20 @@ impl Foo {
#[rustc_const_stable(feature = "rust1", since = "1.2.0")]
pub const fn stable_impl() -> u32 { 42 }
}

#[stable(feature = "rust1", since = "1.0.0")]
pub struct Bar;

impl Bar {
// Do not show non-const stabilities that are the same as the enclosing item.
// @matches 'foo/struct.Bar.html' '//span[@class="since"]' '^const: 1.2.0$'
Copy link
Member

Choose a reason for hiding this comment

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

Is the 1.0.0 still displayed? If not, please put it back like it used to. In any case, please add a test to ensure both are displayed if both are present like it's currently the case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@GuillaumeGomez This is one of the goals of this PR. Currently, stabilities are omitted if the stability of the item matches the stability of the enclosing struct or module, except if they also have a const stability.

This PR updates the logic to always omit the stability if it matches, regardless of whether the item is const-stable or not.

Copy link
Member

Choose a reason for hiding this comment

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

Oh I see. Thanks for the explanations!

#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_stable(feature = "rust1", since = "1.2.0")]
pub const fn stable_impl() -> u32 { 42 }

// Show const-stability even for unstable functions.
// @matches 'foo/struct.Bar.html' '//span[@class="since"]' '^const: 1.3.0$'
#[unstable(feature = "foo2", issue = "none")]
#[rustc_const_stable(feature = "rust1", since = "1.3.0")]
Copy link
Member

@Urgau Urgau Dec 9, 2021

Choose a reason for hiding this comment

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

I think this is invalid, you cannot have a rustc_const_stable with an unstable attribute, but due to a bug in the compiler it's currently accepted despite being obviously wrong (cf. #79551).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I disagree that it's "obviously wrong", allowing unstable functions to be called in a const-stable context is called out as a use-case in the rustc-guide.

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough, I didn't knew that. Thanks for that but I still think having both rustc_const_stable and unstable is "wrong" in a sense.

Copy link
Member

Choose a reason for hiding this comment

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

I agree that unstable + rustc_const_stable doesn't make a lot of sense; stable functions can call unstable functions, so shouldn't rustc_const_stable functions be able to call rustc_const_unstable functions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regardless of whether const-stable unstable functions are disallowed in the future, the implementation of this PR won't be affected: the invalid test case will just be removed.

For now, it's supported, so it makes sense to me to show it in the documentation.

pub const fn const_stable_unstable() -> u32 { 42 }
}
2 changes: 1 addition & 1 deletion src/test/rustdoc/deref-const-fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ 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)'
// @has - '//*[@id="method.len"]//span[@class="since"]' '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 }
Expand Down