Use is_unstable_const_fn instead of is_min_const_fn in rustdoc where appropriate

[slightlyoutofphase]

This closes #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.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @GuillaumeGomez (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[jyn514]

r? @jyn514

Could you please add a test for these changes in src/test/rustdoc? The code you had in #76501 will work fine. See https://rustc-dev-guide.rust-lang.org/rustdoc-internals.html#dotting-is-and-crossing-ts for instructions - the main idea is to write an XPATH query that verifies the right HTML is present. Run rg @has src/test/rustdoc for examples.

[slightlyoutofphase]

Will do. It looks like I still didn't quite get all the existing tests to pass though, without bringing the actual problem back... I'll have to see if there's a better function to use.

[jyn514]

Those test cases just look wrong?

 15: @has check failed
	`XPATH PATTERN` did not match
	// @has 'foo/fn.foo2.html' '//pre' 'pub fn foo2() -> u32'
24: @has check failed
	`XPATH PATTERN` did not match
	// @has 'foo/fn.foo2_gated.html' '//pre' 'pub unsafe fn foo2_gated() -> u32'
33: @has check failed
	`XPATH PATTERN` did not match
	// @has 'foo/fn.bar_not_gated.html' '//pre' 'pub unsafe fn bar_not_gated() -> u32'

// @has 'foo/fn.foo2.html' '//pre' 'pub fn foo2() -> u32'
#[unstable(feature = "humans", issue = "none")]
pub const fn foo2() -> u32 { 42 }


// @has 'foo/fn.foo2_gated.html' '//pre' 'pub unsafe fn foo2_gated() -> u32'
#[unstable(feature = "foo2", issue = "none")]
pub const unsafe fn foo2_gated() -> u32 { 42 }

// @has 'foo/fn.bar_not_gated.html' '//pre' 'pub unsafe fn bar_not_gated() -> u32' 
pub const unsafe fn bar_not_gated() -> u32 { 42 }

All of those should havea const - the functions themselves are unstable, the const is not.
cc @GuillaumeGomez - does that seem right? if so I think the test cases should be changed.

[slightlyoutofphase]

That's kind of what I was thinking as well. Particularly the one that is literally called bar_not_gated with no local unstable flag, you'd expect to show the const.

[slightlyoutofphase]

I'll hold off on uploading the test file with the code from #76501 until we hear back, as there's no point in triggering another build when it's still going to fail const-display.rs.

Edit:

How does this look as far as the XPATH stuff for it, though:

#![feature(const_fn)]

// @has 'issue_76501/fn.bloop.html' '//pre' 'pub const fn bloop() -> i32'
/// A useless function that always returns 1.
pub const fn bloop() -> i32 {
    1
}

/// A struct.
pub struct Struct {}

impl Struct {
    // @has 'issue_76501/struct.Struct.html' '//*[@class="method"]' 'pub const fn bloop() -> i32'
    /// A useless function that always returns 1.
    pub const fn bloop() -> i32 {
        1
    }
}

[GuillaumeGomez]

cc @GuillaumeGomez - does that seem right? if so I think the test cases should be changed.

We should check why the tests were written this way maybe, but it seems wrong.

[jyn514]

Looks like this was introduced in a6943d9 as part of #56845. There wasn't any mention there of treating #[unstable] the same as #[rustc_const_unstable] so @slightlyoutofphase I think it's fine to update the tests with the correct behavior.

Your new test looks great :)

[slightlyoutofphase]

Ok, since everything passed, I went ahead and committed the function name changes in the issue-76501.rs test file that @jyn514 asked about.

Not sure if you want to let the whole build run again just for that, as it's definitely going to pass again?

[jyn514]

Sounds good to me, thanks for the PR!

@bors r+

[bors]

📌 Commit 8a1288b has been approved by jyn514

[slightlyoutofphase]

No problem! Thanks for the guidance on it.

[bors]

⌛ Testing commit 8a1288b with merge a055c5a...

[bors]

☀️ Test successful - checks-actions, checks-azure
Approved by: jyn514
Pushing a055c5a to master...