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

Re-exported types have confusingly different documentation #44306

Open
dtolnay opened this issue Sep 4, 2017 · 7 comments
Open

Re-exported types have confusingly different documentation #44306

dtolnay opened this issue Sep 4, 2017 · 7 comments
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

@dtolnay
Copy link
Member

dtolnay commented Sep 4, 2017

Re-exported types seem to have more confusing documentation than the original type. I noticed this while reviewing rayon::Configuration which is re-exported from rayon-core.


original/src/lib.rs

pub trait T {}
pub struct S;

impl S {
    pub fn f<F>(_: F, _: Box<T>) -> Self
        where F: Fn()
    {
        unimplemented!()
    }
}

This is documented basically like what I wrote, which is what I would expect:

selection_063


src/lib.rs

extern crate original;
pub use original::S;

This is documented with a couple changes, all of which are technically correct but unexpected.

  • The Box<T> is now Box<T + 'static>.
  • The return type has changed from Self to S.
  • The where clause gives an explicit -> (). fixed as of 1.50
  • The parameter name placeholders are gone (which I prefer, but it is a difference).

selection_062

I am using rustc 1.22.0-nightly (f861b6e 2017-09-01).

@GuillaumeGomez

@GuillaumeGomez GuillaumeGomez added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Sep 4, 2017
@GuillaumeGomez
Copy link
Member

Dark magic! 😆

I'll take a look later.

@chordowl
Copy link
Contributor

chordowl commented Sep 4, 2017

cc #24305

@TimNN TimNN added C-enhancement Category: An issue proposing an enhancement or a PR with one. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. labels Sep 17, 2017
@camelid camelid added the A-cross-crate-reexports Area: Documentation that has been re-exported from a different crate label Oct 12, 2020
@camelid
Copy link
Member

camelid commented Oct 12, 2020

I think this is a cross-crate reexports issue, so labeling as such.

@jyn514
Copy link
Member

jyn514 commented Dec 16, 2020

Triage as of rustdoc 1.50.0-nightly (0f6f2d6 2020-12-06): This bug has gone away:

The where clause gives an explicit -> ().

The rest are still changed from the inner crate.

Inner: image
Outer: image

@jyn514
Copy link
Member

jyn514 commented Dec 16, 2020

I expect this is an inconsistency between how clean treats HIR and rustc_middle::ty somewhere (the difference being that ty works off serialized metadata).

@ehuss ehuss removed the T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. label Jan 18, 2022
@fmease
Copy link
Member

fmease commented Oct 27, 2022

Temporarily assigning myself to this issue since I am working on fixing several issues related to the handling of cross-crate trait object types in rustdoc and other minor cross-crate things. I won't fix everything mentioned in this issue however, hence this is only a temporary assignment until the PR is open.

@rustbot claim
@rustbot label -C-enhancement +C-bug

@rustbot rustbot added C-bug Category: This is a bug. and removed C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Oct 27, 2022
JohnTitor pushed a commit to JohnTitor/rust that referenced this issue Nov 6, 2022
…-reexport-fixes, r=cjgillot,GuillaumeGomez

rustdoc: various cross-crate reexport fixes

Fixes for various smaller cross-crate reexport issues.
The PR is split into several commits for easier review. Will be squashed after approval.

Most notable changes:

* We finally render late-bound lifetimes in the generic parameter list of cross-crate functions & methods.
  Previously, we would display the re-export of `pub fn f<'s>(x: &'s str) {}` as `pub fn f(x: &'s str)`
* We now render unnamed parameters of cross-crate functions and function pointers as underscores
  since that's exactly what we do for local definitions, too. Mentioned as a bug in rust-lang#44306.
* From now on, the rendering of cross-crate trait-object types is more correct:
  * `for<>` parameter lists (for higher-ranked lifetimes) are now shown
  * the return type of `Fn{,Mut,Once}` trait bounds is now displayed

Regarding the last list item, here is a diff for visualization (before vs. after):

```patch
- dyn FnOnce(&'any str) + 'static
+ dyn for<'any> FnOnce(&'any str) -> bool + 'static
```

The redundant `+ 'static` will be removed in a follow-up PR that will hide trait-object lifetime-bounds if they coincide with [their default](https://doc.rust-lang.org/reference/lifetime-elision.html#default-trait-object-lifetimes) (see [Zulip discussion](https://rust-lang.zulipchat.com/#narrow/stream/266220-rustdoc/topic/clean_middle_ty.3A.20I.20need.20to.20add.20a.20parameter/near/307143097)). `FIXME(fmease)`s were added.

`@rustbot` label A-cross-crate-reexports
r? `@GuillaumeGomez`
JohnTitor pushed a commit to JohnTitor/rust that referenced this issue Nov 7, 2022
…-reexport-fixes, r=cjgillot,GuillaumeGomez

rustdoc: various cross-crate reexport fixes

Fixes for various smaller cross-crate reexport issues.
The PR is split into several commits for easier review. Will be squashed after approval.

Most notable changes:

* We finally render late-bound lifetimes in the generic parameter list of cross-crate functions & methods.
  Previously, we would display the re-export of `pub fn f<'s>(x: &'s str) {}` as `pub fn f(x: &'s str)`
* We now render unnamed parameters of cross-crate functions and function pointers as underscores
  since that's exactly what we do for local definitions, too. Mentioned as a bug in rust-lang#44306.
* From now on, the rendering of cross-crate trait-object types is more correct:
  * `for<>` parameter lists (for higher-ranked lifetimes) are now shown
  * the return type of `Fn{,Mut,Once}` trait bounds is now displayed

Regarding the last list item, here is a diff for visualization (before vs. after):

```patch
- dyn FnOnce(&'any str) + 'static
+ dyn for<'any> FnOnce(&'any str) -> bool + 'static
```

The redundant `+ 'static` will be removed in a follow-up PR that will hide trait-object lifetime-bounds if they coincide with [their default](https://doc.rust-lang.org/reference/lifetime-elision.html#default-trait-object-lifetimes) (see [Zulip discussion](https://rust-lang.zulipchat.com/#narrow/stream/266220-rustdoc/topic/clean_middle_ty.3A.20I.20need.20to.20add.20a.20parameter/near/307143097)). `FIXME(fmease)`s were added.

``@rustbot`` label A-cross-crate-reexports
r? ``@GuillaumeGomez``
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`
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``
@fmease
Copy link
Member

fmease commented Oct 30, 2023

Update: Thanks to #103885, #107637 & #112463, the output is finally very close to acceptable (source vs. rendered):

- pub fn f<F>(_: F, _: Box<dyn T>) -> Self
- where F: Fn()
+ pub fn f<F>(_: F, _: Box<dyn T>) -> S
+ where
+     F: Fn(),

The remaining issue concerns Self types in cross-crate scenarios. I'm not gonna work on a PR for that in the foreseeable future but at some point I will probably submit a PR for it. I predict it to be a bit nasty performance-wise since it requires performing type unification on every Ty inside an impl.

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``
@fmease fmease removed their assignment Dec 18, 2023
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

No branches or pull requests

9 participants