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

intra-doc links to modules instead of primitives inside standard library #58699

Closed
tspiteri opened this issue Feb 24, 2019 · 8 comments · Fixed by #75318
Closed

intra-doc links to modules instead of primitives inside standard library #58699

tspiteri opened this issue Feb 24, 2019 · 8 comments · Fixed by #75318
Labels
A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@tspiteri
Copy link
Contributor

For example in the doc for str::len:

This length is in bytes, not chars or graphemes.

The char in the doc links to char/index.html instead of primitive.char.html.

Outside of the standard library, char links to the primitive, unless there is a use std::char; statement in which case it links to the module.

@jonas-schievink jonas-schievink added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name labels Feb 24, 2019
@GuillaumeGomez
Copy link
Member

Therefore we should try to link to types before modules I assume? I'll see how it comes out.

@GuillaumeGomez GuillaumeGomez self-assigned this Mar 3, 2019
@tspiteri
Copy link
Contributor Author

tspiteri commented Mar 4, 2019

Yes I think types should go before modules.

After all it's always possible to use [`char`][`std::char`] if you actually want to link to the std::char module using char as the link text.

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Mar 4, 2019

Oh btw, you can force the resolution by adding the type before hand like method@len. I wonder if this is documented somewhere... Just like you can say it's a macro by ! at the end or a function by adding () at the end.

@GuillaumeGomez GuillaumeGomez removed their assignment Mar 4, 2019
@GuillaumeGomez
Copy link
Member

It comes from the compiler directly. We need to re-order how the items are resolved.

cc @rust-lang/compiler

@jyn514
Copy link
Member

jyn514 commented Jul 5, 2020

@GuillaumeGomez an alternative is to resolve in an empty module first and only then resolve in the current scope. This ambiguity is only possible for primitives, any other types will give a multiple definition error: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=16d8de614c51403e2f8576f355ed6f9c. Since primitives are always in scope they will be the only thing in scope in the empty module (assuming there's no super, crate, or self keyword).

jyn514 added a commit to jyn514/rust that referenced this issue Jul 6, 2020
Previously, if there were a module in scope with the same name as the
primitive, that would take precedence. Coupled with
rust-lang#58699, this made it impossible
to link to the primitive when that module was in scope.

This approach could be extended so that `struct@foo` would no longer resolve
to any type, etc. However, it could not be used for glob imports:

```rust
pub mod foo {
  pub struct Bar;
}

pub enum Bar {}
use foo::*;

// This is expected to link to `inner::Bar`, but instead it will link to the enum.
/// Link to [struct@Bar]
pub struct MyDocs;
```

The reason for this is that this change does not affect the resolution
algorithm of rustc_resolve at all. The only reason we could special-case
primitives is because we have a list of all possible primitives ahead of time.
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 6, 2020
Always resolve type@primitive as a primitive, not a module

Previously, if there were a module in scope with the same name as the
primitive, that would take precedence. Coupled with
rust-lang#58699, this made it impossible
to link to the primitive when that module was in scope.

This approach could be extended so that `struct@foo` would no longer resolve
to any type, etc. However, it could not be used for glob imports:

```rust
pub mod foo {
  pub struct Bar;
}

pub enum Bar {}
use foo::*;

// This is expected to link to `inner::Bar`, but instead it will link to the enum.
/// Link to [struct@Bar]
pub struct MyDocs;
```

The reason for this is that this change does not affect the resolution
algorithm of rustc_resolve at all. The only reason we could special-case
primitives is because we have a list of all possible primitives ahead of time.

Closes rust-lang#74063

r? @Manishearth
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 6, 2020
Always resolve type@primitive as a primitive, not a module

Previously, if there were a module in scope with the same name as the
primitive, that would take precedence. Coupled with
rust-lang#58699, this made it impossible
to link to the primitive when that module was in scope.

This approach could be extended so that `struct@foo` would no longer resolve
to any type, etc. However, it could not be used for glob imports:

```rust
pub mod foo {
  pub struct Bar;
}

pub enum Bar {}
use foo::*;

// This is expected to link to `inner::Bar`, but instead it will link to the enum.
/// Link to [struct@Bar]
pub struct MyDocs;
```

The reason for this is that this change does not affect the resolution
algorithm of rustc_resolve at all. The only reason we could special-case
primitives is because we have a list of all possible primitives ahead of time.

Closes rust-lang#74063

r? @Manishearth
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 6, 2020
Always resolve type@primitive as a primitive, not a module

Previously, if there were a module in scope with the same name as the
primitive, that would take precedence. Coupled with
rust-lang#58699, this made it impossible
to link to the primitive when that module was in scope.

This approach could be extended so that `struct@foo` would no longer resolve
to any type, etc. However, it could not be used for glob imports:

```rust
pub mod foo {
  pub struct Bar;
}

pub enum Bar {}
use foo::*;

// This is expected to link to `inner::Bar`, but instead it will link to the enum.
/// Link to [struct@Bar]
pub struct MyDocs;
```

The reason for this is that this change does not affect the resolution
algorithm of rustc_resolve at all. The only reason we could special-case
primitives is because we have a list of all possible primitives ahead of time.

Closes rust-lang#74063

r? @Manishearth
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Jul 6, 2020
Always resolve type@primitive as a primitive, not a module

Previously, if there were a module in scope with the same name as the
primitive, that would take precedence. Coupled with
rust-lang#58699, this made it impossible
to link to the primitive when that module was in scope.

This approach could be extended so that `struct@foo` would no longer resolve
to any type, etc. However, it could not be used for glob imports:

```rust
pub mod foo {
  pub struct Bar;
}

pub enum Bar {}
use foo::*;

// This is expected to link to `inner::Bar`, but instead it will link to the enum.
/// Link to [struct@Bar]
pub struct MyDocs;
```

The reason for this is that this change does not affect the resolution
algorithm of rustc_resolve at all. The only reason we could special-case
primitives is because we have a list of all possible primitives ahead of time.

Closes rust-lang#74063

r? @Manishearth
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 7, 2020
Always resolve type@primitive as a primitive, not a module

Previously, if there were a module in scope with the same name as the
primitive, that would take precedence. Coupled with
rust-lang#58699, this made it impossible
to link to the primitive when that module was in scope.

This approach could be extended so that `struct@foo` would no longer resolve
to any type, etc. However, it could not be used for glob imports:

```rust
pub mod foo {
  pub struct Bar;
}

pub enum Bar {}
use foo::*;

// This is expected to link to `inner::Bar`, but instead it will link to the enum.
/// Link to [struct@Bar]
pub struct MyDocs;
```

The reason for this is that this change does not affect the resolution
algorithm of rustc_resolve at all. The only reason we could special-case
primitives is because we have a list of all possible primitives ahead of time.

Closes rust-lang#74063

r? @Manishearth
@jyn514
Copy link
Member

jyn514 commented Jul 10, 2020

@jyn514 I'm a bit wary of doing this because having the char module in scope is not a common situation, and it will get weirder for people who have a different char module.

I think we should resolve to whatever is in scope, but if type@ is explicitly mentioned we can then force resolve to the primitive.

Originally posted by @Manishearth in #74063 (comment)

@Manishearth is this still your opinion? That would mean closing this issue as wontfix, but I'm not convinced that's the most clear behavior.

@Manishearth
Copy link
Member

@jyn514 Well, I would close this as fixed, but yes. I think this is a pretty rare situation to be in and the current behavior is not incorrect, it's just surprising, but "fixing" it would also make it surprising to another group.

@jyn514
Copy link
Member

jyn514 commented Aug 7, 2020

@Manishearth and I discussed this again and decided it would be less surprising than the current behavior (@Manishearth himself made the same mistake in #74470!) So I plan to implement this sometime soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name 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