Skip to content

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Feb 7, 2022

Previously, rustdoc had 3 fallbacks it used:

  1. resolve_macro_path
  2. all_macros
  3. resolve_str_path_error

Ideally, it would only use resolve_str_path_error, to be consistent with other namespaces.
Unfortunately, that doesn't consider macros that aren't defined at module scope;
consider for instance

{
    struct S;

    macro_rules! mac { () => {} }
    // `mac`'s scope starts here

    /// `mac` <- `resolve_str_path_error` won't see this
   struct Z;

    //`mac`'s scope ends here
}

This changes it to only use all_macros and resolve_str_path_error, and gives
resolve_str_path_error precedence over all_macros in case there are two macros with the same
name in the same module.

This is a smaller version of #91427.

r? @petrochenkov

@rustbot rustbot added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Feb 7, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 7, 2022
@jyn514 jyn514 force-pushed the less-macro-special-casing branch 2 times, most recently from 7384636 to 8c7422c Compare February 7, 2022 00:54
Previously, rustdoc had 3 fallbacks it used:
1. `resolve_macro_path`
2. `all_macros`
3. `resolve_str_path_error`

Ideally, it would only use `resolve_str_path_error`, to be consistent with other namespaces.
Unfortunately, that doesn't consider macros that aren't defined at module scope;
consider for instance
```rust
{
    struct S;

    macro_rules! mac { () => {} }
    // `mac`'s scope starts here

    /// `mac` <- `resolve_str_path_error` won't see this
   struct Z;

    //`mac`'s scope ends here
}
```

This changes it to only use `all_macros` and `resolve_str_path_error`, and gives
`resolve_str_path_error` precedence over `all_macros` in case there are two macros with the same
name in the same module.

This also adds a failing test case which will catch trying to remove `all_macros`.
@jyn514 jyn514 force-pushed the less-macro-special-casing branch from 8c7422c to f026550 Compare February 7, 2022 01:29
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
  IMAGE: x86_64-gnu-tools
##[endgroup]
From https://github.com/rust-lang/rust
 * branch              master     -> FETCH_HEAD
Searching for toolstate changes between 7b43cfc9b25ac4a906bd56d32d3111085dd9e6a1 and e6ec01aa7e6e86f0e28e2476aeab1ccf55b909ae
Rustdoc was updated
##[group]Run src/ci/scripts/verify-channel.sh
src/ci/scripts/verify-channel.sh
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
env:
---
.......... (50/59)
.........  (59/59)


/checkout/src/test/rustdoc-gui/mobile.goml An exception occured: Failed to launch the browser process!
Inconsistency detected by ld.so: dl-tls.c: 493: _dl_allocate_tls_init: Assertion `listp->slotinfo[cnt].gen <= GL(dl_tls_generation)' failed!


TROUBLESHOOTING: https://github.com/puppeteer/puppeteer/blob/master/docs/troubleshooting.md
== STACKTRACE ==
Error
Error
    at innerRunTestCode (/node-v14.4.0-linux-x64/lib/node_modules/browser-ui-test/src/index.js:468:16)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)

@jyn514
Copy link
Member Author

jyn514 commented Feb 7, 2022

/checkout/src/test/rustdoc-gui/mobile.goml An exception occured: Failed to launch the browser process!
Inconsistency detected by ld.so: dl-tls.c: 493: _dl_allocate_tls_init: Assertion `listp->slotinfo[cnt].gen <= GL(dl_tls_generation)' failed!

oh fun, cc @GuillaumeGomez

@petrochenkov
Copy link
Contributor

The CI failure looks spurious?
@bors r+

@bors
Copy link
Collaborator

bors commented Feb 7, 2022

📌 Commit f026550 has been approved by petrochenkov

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 7, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 8, 2022
…askrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#86497 (Add {floor,ceil}_char_boundary methods to str)
 - rust-lang#92695 (Add `#[no_coverage]` tests for nested functions)
 - rust-lang#93521 (Fix hover effects in sidebar)
 - rust-lang#93568 (Include all contents of first line of scraped item in Rustdoc)
 - rust-lang#93569 (rustdoc: correct unclosed HTML tags as generics)
 - rust-lang#93672 (update comment wrt const param defaults)
 - rust-lang#93715 (Fix horizontal trim for block doc comments)
 - rust-lang#93721 (rustdoc: Special-case macro lookups less)
 - rust-lang#93728 (Add in ValuePair::Term)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 1f90f4f into rust-lang:master Feb 8, 2022
@rustbot rustbot added this to the 1.60.0 milestone Feb 8, 2022
@jyn514 jyn514 deleted the less-macro-special-casing branch February 8, 2022 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants