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

Inconsistently documented constraints for IntoIterator::Item #77763

Closed
cuviper opened this issue Oct 9, 2020 · 8 comments · Fixed by #102439
Closed

Inconsistently documented constraints for IntoIterator::Item #77763

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

Comments

@cuviper
Copy link
Member

cuviper commented Oct 9, 2020

The documentation for IntoIterator is inconsistent in how it shows the Item constraints.

The actual trait declaration looks like this:

pub trait IntoIterator {
    type Item;
    type IntoIter: Iterator<Item = Self::Item>;
    fn into_iter(self) -> Self::IntoIter;
}

The nightly core documentation matches this declaration as of 1.49.0-nightly (3525087 2020-10-08). This is good.

However, the nightly std documentation shows no Item constraints at all:

pub trait IntoIterator {
    type Item;
    type IntoIter: Iterator;
    fn into_iter(self) -> Self::IntoIter;
}

The std documentation for 1.48.0-beta.2 is even weirder, using an equality constraint that's not supported (#20041), and this goes all the way back to 1.0.0.

pub trait IntoIterator
where
    <Self::IntoIter as Iterator>::Item == Self::Item,
{
    type Item;
    type IntoIter: Iterator;
    fn into_iter(self) -> Self::IntoIter;
}
@cuviper cuviper added the C-bug Category: This is a bug. label Oct 9, 2020
@jyn514 jyn514 added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-trait-system Area: Trait system labels Oct 9, 2020
@jyn514
Copy link
Member

jyn514 commented Oct 9, 2020

This is the relevant code for traits:

"{}{}{}trait {}{}{}",

This is the relevant code for trait bounds:
crate fn print_generic_bounds(bounds: &[clean::GenericBound]) -> impl fmt::Display + '_ {

After that it goes into Ty::print() and I'm not sure where it goes.
write!(f, "{}", self.trait_.print())

@jyn514
Copy link
Member

jyn514 commented Oct 11, 2020

Well. I think I found the issue.

// FIXME: `param_names` are not rendered, and this seems bad?
drop(param_names);

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

jyn514 commented Oct 11, 2020

Hmm, changing that didn't help - it's always None, at least when documenting IntoIterator.

@jyn514
Copy link
Member

jyn514 commented Oct 11, 2020

The bounds passed to print_generic_bounds are incorrect across crates. Here they are for the original crate:

2:rustcDEBUG rustdoc::html::format printing bounds [TraitBound(PolyTrait { trait_: ResolvedPath { path: Path { global: false, res: Def(Trait, DefId(2:5285)), segments: [PathSegment { name: "Iterator", args: AngleBracketed { args: [], bindings: [TypeBinding { name: "Item", kind: Equality { ty: QPath { name: "Item", self_type: Generic("Self"), trait_: ResolvedPath { path: Path { global: false, res: Def(Trait, DefId(0:3)), segments: [] }, param_names: None, did: DefId(0:3), is_generic: false } } } }] } }] }, param_names: None, did: DefId(2:5285), is_generic: false }, generic_params: [] }, None)]

and for the re-export:

2:rustcDEBUG rustdoc::html::format printing bounds [TraitBound(PolyTrait { trait_: ResolvedPath { path: Path { global: false, res: Err, segments: [PathSegment { name: "Iterator", args: AngleBracketed { args: [], bindings: [] } }] }, param_names: None, did: DefId(2:5285), is_generic: false }, generic_params: [] }, None)]

In particular note that bindings is completely empty.

@jyn514
Copy link
Member

jyn514 commented Oct 11, 2020

This looks like the culprit:

ty::Foreign(did) => {
inline::record_extern_fqn(cx, did, TypeKind::Foreign);
let path = external_path(
cx,
cx.tcx.item_name(did),
None,
false,
vec![],
InternalSubsts::empty(),
);
ResolvedPath { path, param_names: None, did, is_generic: false }
}

vec![] is the bindings.

@jyn514
Copy link
Member

jyn514 commented Oct 11, 2020

This needs larger refactors to rustdoc than I'm comfortable making. bindings is Vec<TypeBinding>, but the only way to construct those is through

impl Clean<TypeBinding> for hir::TypeBinding<'_> {

which works on the hir, not DefIds, and the HIR is not preserved across crates.

Maybe @GuillaumeGomez is interested in refactoring this? ;)

@fmease
Copy link
Member

fmease commented Sep 27, 2022

Update: The current nightly output is still incorrect:

pub trait IntoIterator {
    type Item;
    type IntoIter: Iterator
    where
        <Self::IntoIter as Iterator>::Item == Self::Item;

    fn into_iter(self) -> Self::IntoIter;
}

I am already working on a fix and I have already assigned myself to the duplicates of this issue (#84579 and #102142).
@rustbot claim

@GuillaumeGomez
Copy link
Member

Awesome, thanks!

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 A-trait-system Area: Trait system 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