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 renders Vec's new allocator type parameter even when set to the default #80379

Closed
sfackler opened this issue Dec 26, 2020 · 9 comments · Fixed by #112463
Closed

Rustdoc renders Vec's new allocator type parameter even when set to the default #80379

sfackler opened this issue Dec 26, 2020 · 9 comments · Fixed by #112463
Assignees
Labels
A-cross-crate-reexports Area: Documentation that has been re-exported from a different crate C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@sfackler
Copy link
Member

sfackler commented Dec 26, 2020

For example, the docs for this type use Vec<String, Global> rather than just Vec<String>: https://docs.rs/tokio-postgres/0.7.0/tokio_postgres/types/enum.Kind.html. The definition of the type does not specify the allocator parameter if that matters.

The same is not true for e.g. HashMap's hasher type parameter, so I'm not sure what's different in this case.

@sfackler sfackler added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels Dec 26, 2020
@jyn514
Copy link
Member

jyn514 commented Dec 26, 2020

The difference is that this is a cross-crate re-export. MCVE:

// crate1
use std::collections::*;

pub enum Types {
  Map(HashMap<usize, usize>),
  List(Vec<usize>),
  Pointer(Box<usize>),
}

// crate2
pub use crate1::Types;

image

@jyn514 jyn514 added the A-cross-crate-reexports Area: Documentation that has been re-exported from a different crate label Dec 26, 2020
@jyn514
Copy link
Member

jyn514 commented Dec 26, 2020

This is a bug at least back to 1.22.0.

@jyn514
Copy link
Member

jyn514 commented Dec 26, 2020

In the inner crate, the params never get passes to render in the first place:

DEBUG rustdoc::html::format fmt_type(t=ResolvedPath { path: Path { global: false, res: Def(Struct, DefId(4:4837 ~ alloc[4799]::vec::Vec)), segments: [PathSegment { name: "Vec", args: AngleBracketed { args: [Type(Primitive(Usize))], bindings: [] } }] }, param_names: None, did: DefId(4:4837 ~ alloc[4799]::vec::Vec), is_generic: false }

but in the outer one they do show up:

DEBUG rustdoc::html::format fmt_type(t=ResolvedPath { path: Path { global: false, res: Err, segments: [PathSegment { name: "Vec", args: AngleBracketed { args: [Type(Primitive(Usize)), Type(ResolvedPath { path: Path { global: false, res: Err, segments: [PathSegment { name: "Global", args: AngleBracketed { args: [], bindings: [] } }] }, param_names: None, did: DefId(5:5465 ~ alloc[4799]::alloc::Global), is_generic: false })], bindings: [] } }] }, param_names: None, did: DefId(5:4837 ~ alloc[4799]::vec::Vec), is_generic: false }

I expect this has to do with some difference between HIR and metadata loading.

@jyn514
Copy link
Member

jyn514 commented Dec 26, 2020

I thought this might be related to inlining, but using doc(inline) within the same crate works fine:
image

@jyn514
Copy link
Member

jyn514 commented Dec 26, 2020

The issue is that metadata types, unlike HIR types, go through external_path:

let path = external_path(cx, cx.tcx.item_name(did), None, false, vec![], substs);

and external_path doesn't pay any attention to the type defaults:
let args: Vec<_> = substs

So rustdoc needs to know which of the generic arguments were added by the user and which are just defaults.

bors added a commit to rust-lang-ci/rust that referenced this issue Jan 4, 2021
Cleanup rustdoc handling of associated types

This is best reviewed a commit at a time. No particular reason for these changes, they just stood out as I was reviewing rust-lang#80653 and thinking about rust-lang#80379. The new test case worked before, it just wasn't tested.

r? `@GuillaumeGomez`
@camelid
Copy link
Member

camelid commented Oct 10, 2021

I came across another instance of this recently (#89712), but with a twist. The type that was re-exported also contained a type alias, which was expanded in the re-exported version. Is that the same bug or slightly different?

@fmease
Copy link
Member

fmease commented Jan 19, 2023

@rustbot claim

@jyn514
Copy link
Member

jyn514 commented Jan 19, 2023

I came across another instance of this recently (#89712), but with a twist. The type that was re-exported also contained a type alias, which was expanded in the re-exported version. Is that the same bug or slightly different?

I suspect they're both caused by using metadata loading instead of HIR, if that's what you're asking, but the fix for the two will likely look different.

bors added a commit to rust-lang-ci/rust that referenced this issue Jun 10, 2023
…r-obj-lt-bnds, r=notriddle,cgillot,GuillaumeGomez

rustdoc: re-elide cross-crate default trait-object lifetime bounds

Hide trait-object lifetime bounds (re-exported from an external crate) if they coincide with [their default](https://doc.rust-lang.org/reference/lifetime-elision.html#default-trait-object-lifetimes).
Partially addresses rust-lang#44306. Follow-up to rust-lang#103885. [Zulip discussion](https://rust-lang.zulipchat.com/#narrow/stream/266220-rustdoc/topic/clean_middle_ty.3A.20I.20need.20to.20add.20a.20parameter/near/307143097).

Most notably, if `std` exported something from `core` containing a type like `Box<dyn Fn()>`, then it would now be rendered as `Box<dyn Fn(), Global>` instead of `Box<dyn Fn() + 'static, Global>` (hiding `+ 'static` as it is the default in this case). Showing `Global` here is a separate issue, rust-lang#80379, which is on my agenda.

Note that I am not really fond of the fact that I had to add a parameter to such a widely used function (30+ call sites) to address such a niche bug.

CC `@GuillaumeGomez`
Requesting a review from a compiler contributor or team member as recommended on Zulip.
r? compiler

---

`@rustbot` label T-compiler T-rustdoc A-cross-crate-reexports
bors added a commit to rust-lang-ci/rust that referenced this issue Sep 22, 2023
…-args, r=<try>

rustdoc: elide cross-crate default generic arguments

Early draft. Requesting perf run. Thx :) CC `@GuillaumeGomez`

Elide cross-crate generic arguments if they coincide with their default.
TL;DR: Most notably, no more `Box<…, Global>` in `std`'s docs, just `Box<…>` from now on.
Fixes rust-lang#80379.

Also helps with rust-lang#44306. Follow-up to rust-lang#103885,rust-lang#107637.

`@rustbot` label T-rustdoc A-cross-crate-reexports S-experimental
r? `@ghost`
bors added a commit to rust-lang-ci/rust that referenced this issue Sep 30, 2023
…-args, r=<try>

rustdoc: elide cross-crate default generic arguments

Elide cross-crate generic arguments if they coincide with their default.
TL;DR: Most notably, no more `Box<…, Global>` in `std`'s docs, just `Box<…>` from now on.
Fixes rust-lang#80379.

Also helps with rust-lang#44306. Follow-up to rust-lang#103885, rust-lang#107637.

r? `@ghost`
bors added a commit to rust-lang-ci/rust that referenced this issue Oct 30, 2023
…-args, r=<try>

rustdoc: elide cross-crate default generic arguments

Elide cross-crate generic arguments if they coincide with their default.
TL;DR: Most notably, no more `Box<…, Global>` in `std`'s docs, just `Box<…>` from now on.
Fixes rust-lang#80379.

Also helps with rust-lang#44306. Follow-up to rust-lang#103885, rust-lang#107637.

r? `@ghost`
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Oct 30, 2023
…en-args, r=GuillaumeGomez

rustdoc: elide cross-crate default generic arguments

Elide cross-crate generic arguments if they coincide with their default.
TL;DR: Most notably, no more `Box<…, Global>` in `std`'s docs, just `Box<…>` from now on.
Fixes rust-lang#80379.

Also helps with rust-lang#44306. Follow-up to rust-lang#103885, rust-lang#107637.

r? `@ghost`
@bors bors closed this as completed in b9dce53 Oct 30, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Oct 30, 2023
Rollup merge of rust-lang#112463 - fmease:rustdoc-elide-x-crate-def-gen-args, r=GuillaumeGomez

rustdoc: elide cross-crate default generic arguments

Elide cross-crate generic arguments if they coincide with their default.
TL;DR: Most notably, no more `Box<…, Global>` in `std`'s docs, just `Box<…>` from now on.
Fixes rust-lang#80379.

Also helps with rust-lang#44306. Follow-up to rust-lang#103885, rust-lang#107637.

r? ``@ghost``
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cross-crate-reexports Area: Documentation that has been re-exported from a different crate C-bug Category: This is a bug. 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.

5 participants