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

private macros break intra-doc links #81633

Closed
digama0 opened this issue Feb 1, 2021 · 10 comments
Closed

private macros break intra-doc links #81633

digama0 opened this issue Feb 1, 2021 · 10 comments
Labels
A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) C-bug Category: This is a bug. P-low Low priority T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@digama0
Copy link
Contributor

digama0 commented Feb 1, 2021

mod bar {
    macro_rules! str {() => {}}
}

pub mod foo {
    use std::path::{Path, PathBuf};

    pub struct Foo(PathBuf);
    impl std::ops::Deref for Foo {
        type Target = Path;
        fn deref(&self) -> &Path { &self.0 }
    }
}

In the documentation of Foo::to_str() inherited from the deref impl, the documentation contains a link to [`&str`](str) that fails due to the presence of a str! macro in another module, even though that macro isn't accessible. (Even if it was, it shouldn't be affecting the resolution of an intra-doc link in the standard library.)

Meta

rustc --version --verbose:

rustc 1.51.0-nightly (0e63af5da 2021-01-31)
binary: rustc
commit-hash: 0e63af5da3400ace48a0345117980473fd21ad73
commit-date: 2021-01-31
host: x86_64-unknown-linux-gnu
release: 1.51.0-nightly
LLVM version: 11.0.1
@digama0 digama0 added the C-bug Category: This is a bug. label Feb 1, 2021
@jonas-schievink jonas-schievink added 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. labels Feb 1, 2021
@camelid camelid added I-prioritize Issue: Indicates that prioritization has been requested for this issue. A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) labels Feb 1, 2021
@apiraino apiraino added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Feb 5, 2021
@apiraino
Copy link
Contributor

apiraino commented Feb 5, 2021

Assigning P-high as discussed as part of the Prioritization Working Group procedure and removing I-prioritize

@GuillaumeGomez GuillaumeGomez self-assigned this Feb 5, 2021
@GuillaumeGomez GuillaumeGomez added P-low Low priority and removed P-high High priority labels Feb 5, 2021
@GuillaumeGomez
Copy link
Member

After a deeper look, this seems like a low priority considering how rare it is. I'll still continue to check what's going on.

@GuillaumeGomez GuillaumeGomez removed their assignment Feb 16, 2021
@jyn514
Copy link
Member

jyn514 commented Aug 5, 2021

I can't investigate this currently due to #87783 🤦

@jyn514
Copy link
Member

jyn514 commented Nov 30, 2021

I continue to be stymied in investigating this 🤦 #91400

@jyn514
Copy link
Member

jyn514 commented Dec 1, 2021

Ok, I managed to mostly reproduce this without using types from the standard library - my code has different bugs but there are still bugs.

pub mod foo {
    pub struct Bar(Baz);
    impl std::ops::Deref for Bar {
        type Target = Baz;
        fn deref(&self) -> &Baz { &self.0 }
    }

    pub struct Baz {}
    impl Baz {
        /// [str]
        pub fn foo(&self) {
            str!()
        }
    }
}

mod bar {
    macro_rules! str {() => {}}
}

That has the following output when run with rustdoc:

warning: `str` is both a builtin type and a macro
  --> src/lib.rs:47:14
   |
47 |         /// [str]
   |              ^^^ ambiguous link
   |
   = note: `#[warn(rustdoc::broken_intra_doc_links)]` on by default
help: to link to the builtin type, prefix with `prim@`
   |
47 |         /// [prim@str]
   |              +++++
help: to link to the macro, add an exclamation mark
   |
47 |         /// [str!]
   |                 +

and with rustc:

error: cannot find macro `str` in this scope
  --> src/lib.rs:49:13
   |
49 |             str!()
   |             ^^^
   |
   = help: have you added the `#[macro_use]` on the module/import?

warning: unused macro definition: `str`
  --> src/lib.rs:55:18
   |
55 |     macro_rules! str {() => {}}
   |                  ^^^
   |
   = note: `#[warn(unused_macros)]` on by default

@jyn514
Copy link
Member

jyn514 commented Dec 1, 2021

Oh, and Deref isn't related, you get the same issues without it:

mod bar {
    macro_rules! str {() => {}}
}

pub mod foo {
    pub struct Baz {}
    impl Baz {
        /// [str]
        pub fn foo(&self) {
            // str!()
        }
    }
}

@jyn514
Copy link
Member

jyn514 commented Dec 1, 2021

This is the code that's incorrectly resolving str!:

if let Some(&res) = resolver.all_macros().get(&Symbol::intern(path_str)) {
return Ok(res.try_into().unwrap());
}

That was first added in #47046, specifically 00ce770. There are two other macro lookups in that function, resolve_macro_path and resolve_str_path_error(MacroNS). resolve_macro_path was added in 7ac48d7 in the same PR. resolve_str_path_error was added in 4eda3f7 / #73183.

@Manishearth do you think we need all three of these? I'm tempted to remove everything but resolve_str_path_error.

@jyn514
Copy link
Member

jyn514 commented Dec 1, 2021

I'm tempted to remove everything but resolve_str_path_error.

Tried this and I'm not seeing any test failures :)

@petrochenkov
Copy link
Contributor

petrochenkov commented Apr 28, 2022

Fixed in #96521.

bors added a commit to rust-lang-ci/rust that referenced this issue May 1, 2022
…illaumeGomez

rustdoc: Resolve doc links referring to `macro_rules` items

cc rust-lang#81633

UPD: the fallback to considering *all* `macro_rules` in the crate for unresolved names is not removed in this PR, it will be removed separately and will be run through crater.
@petrochenkov
Copy link
Contributor

This was fixed (with a test) in #96676, closing.

workingjubilee pushed a commit to tcdi/postgrestd that referenced this issue Sep 15, 2022
rustdoc: Resolve doc links referring to `macro_rules` items

cc rust-lang/rust#81633

UPD: the fallback to considering *all* `macro_rules` in the crate for unresolved names is not removed in this PR, it will be removed separately and will be run through crater.
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 A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) C-bug Category: This is a bug. P-low Low priority 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.

8 participants