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

#![feature(const_fn)] causes rustdoc to drop the const keyword when documenting instances of const fn #76501

Closed
slightlyoutofphase opened this issue Sep 8, 2020 · 13 comments · Fixed by #76623
Labels
A-const-fn Area: const fn foo(..) {..}. Pure functions which can be applied at compile time. C-bug Category: This is a bug. requires-nightly This issue requires a nightly compiler in some way. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@slightlyoutofphase
Copy link
Contributor

This is a follow-up on the now-closed issue I opened earlier, with the requested minimal reproduction in the form of a simple lib.rs.

// If the feature flag below is commented out, the code is documented correctly.
// With it enabled, the two `pub const fn`s are documented as just `pub fn`.
#![feature(const_fn)]

/// A useless function that always returns 1.
pub const fn bloop() -> i32 {
    1
}

/// A struct.
pub struct Struct {}

impl Struct {
    /// A useless function that always returns 1.
    pub const fn bloop() -> i32 {
        1
    }
}
@jyn514 jyn514 added A-const-fn Area: const fn foo(..) {..}. Pure functions which can be applied at compile time. C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Sep 9, 2020
@jyn514
Copy link
Member

jyn514 commented Sep 9, 2020

Looks like this is either

let constness = if is_min_const_fn(cx.tcx, did.to_def_id()) {

or
if m.header.constness == hir::Constness::Const
&& !is_min_const_fn(cx.tcx, local_did.to_def_id())
{
m.header.constness = hir::Constness::NotConst;
}

cc @GuillaumeGomez - do you know what those are doing/why they exist?

@GuillaumeGomez
Copy link
Member

It's because of the feature. As long as it's not stable, they don't want the const keyword to be displayed, even if the function description is describing a const fn. This is a bit strange.

@slightlyoutofphase
Copy link
Contributor Author

Yeah, that makes no sense to me at all. I have a crate that is very much nightly-only containing a rather large number of const fns, and at the moment literally none of them are displayed as such in my documentation because of this, despite the fact that they've never not been const fn and will never not be const fn in the future.

I of course cannot disable the flag either as then the crate won't compile, as much of what I'm doing does not fit into the stabilized min_const_fn set of functionality.

At the very least, surely there must be some way to make it possible to specifically tell rustdoc to "please always document const fns as exactly what they are no matter what" without changing this default behaviour if that's not desired?

@jyn514
Copy link
Member

jyn514 commented Sep 9, 2020

@GuillaumeGomez I think the metric should be 'does this have rustc_const_unstable, not 'is this a min_const_generics function'.

@GuillaumeGomez
Copy link
Member

I wonder if this isn't an artifact of the switch when adding const support... But in any case, it should be a check over rustc_const_unstable, you're absolutely right.

@jyn514
Copy link
Member

jyn514 commented Sep 9, 2020

I wonder if this ever worked ... @slightlyoutofphase do you mind trying this with an old version of rustdoc? Preferably before min_const_generics was implemented: #74877.

cc @lcnr

@slightlyoutofphase
Copy link
Contributor Author

@jyn514 I'll try that on my crate later today.

@slightlyoutofphase
Copy link
Contributor Author

After testing both the example I gave here and my own crate with various nightlies from before min_const_generics was implemented, I can confirm that this problem definitely pre-dates it. There's no difference in how rustdoc behaved.

@jyn514
Copy link
Member

jyn514 commented Sep 10, 2020

(apologies to lcnr for the ping)

@jyn514 jyn514 added the requires-nightly This issue requires a nightly compiler in some way. label Sep 10, 2020
@jyn514 jyn514 changed the title The specific presence of #![feature(const_fn)] somehow causes rustdoc to drop the const keyword while documenting actual instances of const fn #![feature(const_fn)] causes rustdoc to drop the const keyword when documenting instances of const fn Sep 10, 2020
@slightlyoutofphase
Copy link
Contributor Author

If this helps at all, whichever version of rustdoc docs.rs was using on August 1st did document my crate properly, it seems. Only the very latest version, uploaded on September 5th, was documented with the const keyword missing. So it's a fairly recent bug, overall.

@jyn514 jyn514 added the E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc label Sep 10, 2020
@slightlyoutofphase
Copy link
Contributor Author

Ok, so I think I was reading the dates on the PRs a bit wrong when I tested yesterday as far as the timeframe for when min_const_fn was merged.

I just did a whole bunch more one-by-one testing of nightlies from August, and in fact the problem doesn't quite predate min_const_fn which was merged on August 7th.

Specifically, the very last nightly to have a rustdoc that documents const fn correctly is nightly-2020-08-10. Switching directly from that to nightly-2020-08-11 from the next day makes the problem appear.

@jyn514
Copy link
Member

jyn514 commented Sep 11, 2020

@slightlyoutofphase consider using https://github.com/rust-lang/cargo-bisect-rustc/blob/master/TUTORIAL.md instead which will do it automatically

@slightlyoutofphase
Copy link
Contributor Author

slightlyoutofphase commented Sep 11, 2020

Well, I already know it's a commit from between the 10th and 11th of August now, at least. Also, cargo-bisect-rustc doesn't really seem to be designed for scenarios where identifying the problem specifically requires running cargo doc on a crate and then visually inspecting the generated HTML files...

@jyn514 jyn514 removed the E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc label Sep 12, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Sep 12, 2020
Use `is_unstable_const_fn` instead of `is_min_const_fn` in rustdoc where appropriate

This closes rust-lang#76501. Specifically, it allows for nightly users with the `#![feature(const_fn)]` flag enabled to still have their `const fn` declarations documented as such, while retaining the desired behavior that rustdoc *not* document functions that have the `rustc_const_unstable` attribute as `const`.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Sep 12, 2020
Use `is_unstable_const_fn` instead of `is_min_const_fn` in rustdoc where appropriate

This closes rust-lang#76501. Specifically, it allows for nightly users with the `#![feature(const_fn)]` flag enabled to still have their `const fn` declarations documented as such, while retaining the desired behavior that rustdoc *not* document functions that have the `rustc_const_unstable` attribute as `const`.
@bors bors closed this as completed in a055c5a Sep 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-fn Area: const fn foo(..) {..}. Pure functions which can be applied at compile time. C-bug Category: This is a bug. requires-nightly This issue requires a nightly compiler in some way. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants