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: generic constant expressions not displayed (and where clauses using said gces) #120994

Open
bend-n opened this issue Feb 12, 2024 · 8 comments
Assignees
Labels
A-const-generics Area: const generics (parameters and arguments) C-bug Category: This is a bug. F-generic_const_exprs `#![feature(generic_const_exprs)]` T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@bend-n
Copy link
Contributor

bend-n commented Feb 12, 2024

This code

#![feature(generic_const_exprs)]
pub const fn assert_zero(x: usize) -> usize {
    if x != 0 {
        panic!("expected zero");
    } else {
        0
    }
}

pub fn chunked<const C: usize, const N: usize, T>(x: [T; N]) -> [[T; C]; N / C]
where
    [(); assert_zero(N % C)]:,
{
    unsafe {
        core::mem::ManuallyDrop::new(x)
            .as_ptr()
            .cast::<[[T; C]; N / C]>()
            .read()
    }
}

Produces this documentation

image

This is what i want

image

Meta

rustdoc 1.78.0-nightly (1a648b3 2024-02-11)

@GuillaumeGomez @fmease

@bend-n bend-n added the C-bug Category: This is a bug. label Feb 12, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Feb 12, 2024
@fmease fmease added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-const-generics Area: const generics (parameters and arguments) F-generic_const_exprs `#![feature(generic_const_exprs)]` and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Feb 13, 2024
@fmease
Copy link
Member

fmease commented Feb 13, 2024

Right, I've anonymized most const exprs in #988141 as a hot fix for #97933 to prevent private/internal const exprs getting leaked, so this is kind of intentional although the heuristic is super conservative (and buggy as it doesn't work in cross-crate scenarios).

However, this doesn't mean that this will never change. I originally (roughtly 2 years ago) set out to improve rustdoc's const story (cc #99688). This has stalled a bit but only because I'm super involved in different rustc projects at the moment and I definitely want to pick this up again.

For context, I plan on writing a rustdoc RFC at some point (target date: mid 2024) for richer const “support” in rustdoc (cc #98929, #99630, #102456).

Since everything (the / and the assert_zero) is public, ideally we would properly display them.
I will gladly mentor & review a PR that splits the current heuristic into (1) const exprs as bound to free & assoc const items (2) const exprs as used as a const arg and make (2) consider more things public. The BinOp / can definitely be considered public (we don't need to worry about impl restrictions yet, #106074) and we can talk about assert_zero (for that we probably need to rewrite the way rustdoc represents const exprs (for local items they are HIR const exprs if I remember correctly which doesn't contain the information if a function is public etc., we would need to use ty::Const here).

Footnotes

  1. That's probably why you've tagged me.

@fmease
Copy link
Member

fmease commented Feb 13, 2024

The empty bound not getting displayed I would indeed consider a bug since there's a semantic difference between fn f<'a, T>() and fn f<'a, T>() where 'a:, T: even outside of generic_const_exprs: It affects “boundness” (early vs late) and well-formedness (the latter snippet contains an extra bound iirc, namely WellFormed(T)). I'm pretty sure we currently filter them out. I will gladly accept any PRs fixing this.

@bend-n
Copy link
Contributor Author

bend-n commented Feb 13, 2024

If you have T: Sealed, it still shows the bound. I dont see why

Since everything (the / and the assert_zero) is public, ideally we would properly display them.

Whats wrong with it displaying the function name without any link?

@bend-n
Copy link
Contributor Author

bend-n commented Feb 13, 2024

Your linked issue seems to be about associated constants, not about types.

@fmease
Copy link
Member

fmease commented Feb 13, 2024

Your linked issue seems to be about associated constants, not about types.

Right and the fix was for all (local) const exprs since back then I tarred all with the same brush. However your issue and your further comments does make me realize that we probably don't need to apply the heuristic to const args 👍 (see below)

@fmease fmease self-assigned this Feb 13, 2024
@fmease
Copy link
Member

fmease commented Feb 13, 2024

Whats wrong with it displaying the function name without any link?

Because it's kind of hard to come up with a good heuristic when to show and when not to show complex const exprs. For example should we display the whole const expr in pub fn f(_: I32<{ let x = 0; x }>)? Of course the likelihood of people writing large constant expressions inside types and function signatures is lower I guess compared to free & assoc const items.

So I've assigned myself to this issue but I can't just remove this heuristic (due to cases like #97933 (comment) (this is in an assoc const but this applies to all const exprs)) only tweak it and make it more liberal if we have a const arg.

And I'm not sure if t-rustdoc is happy about further complicating existing heuristics inside rustdoc.

@bend-n
Copy link
Contributor Author

bend-n commented Feb 13, 2024

So this is largely a design problem, yes?

@fmease
Copy link
Member

fmease commented Feb 13, 2024

Yes, the implementation is straightforward (unless we want to take extra stuff into consideration like the visibility of the functions used inside the const exprs but that's also solvable).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-generics Area: const generics (parameters and arguments) C-bug Category: This is a bug. F-generic_const_exprs `#![feature(generic_const_exprs)]` T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants