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

Fix invalid display of inlined re-export when both local and foreign items are inlined #113785

Merged
merged 3 commits into from
Jul 19, 2023

Conversation

GuillaumeGomez
Copy link
Member

Fixes #105735.

The bug is actually quite interesting: at the clean pass, local inlined items have their use item removed, however foreign items don't have their use item removed because it's in the clean pass that we handle them. So when a use inlines both a local and a foreign item, it will work as expected for the foreign one, but not for the local as its use should not be around anymore.

To prevent this, I created a new inlined_foreigns field into the Module struct to allow to remove the use item early on for foreign items as well. Then we iterate it in the clean pass directly.

r? @notriddle

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jul 17, 2023
@GuillaumeGomez
Copy link
Member Author

Updated!

@notriddle
Copy link
Contributor

@bors r rollup

@bors
Copy link
Contributor

bors commented Jul 18, 2023

📌 Commit 01e30d9c345567c14f68b98716ccd58e2da75dee has been approved by notriddle

It is now in the queue for this repository.

@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 Jul 18, 2023
@bors
Copy link
Contributor

bors commented Jul 18, 2023

☔ The latest upstream changes (presumably #113574) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 18, 2023
@GuillaumeGomez
Copy link
Member Author

@bors r=notriddle,aDotInTheVoid rollup

@bors
Copy link
Contributor

bors commented Jul 18, 2023

📌 Commit afec6d2 has been approved by notriddle,aDotInTheVoid

It is now in the queue for this repository.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 18, 2023
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jul 18, 2023
…-105735-fix, r=notriddle,aDotInTheVoid

Fix invalid display of inlined re-export when both local and foreign items are inlined

Fixes rust-lang#105735.

The bug is actually quite interesting: at the `clean` pass, local inlined items have their `use` item removed, however foreign items don't have their `use` item removed because it's in the `clean` pass that we handle them. So when a `use` inlines both a local and a foreign item, it will work as expected for the foreign one, but not for the local as its `use` should not be around anymore.

To prevent this, I created a new `inlined_foreigns` field into the `Module` struct to allow to remove the `use` item early on for foreign items as well. Then we iterate it in the `clean` pass directly.

r? `@notriddle`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 18, 2023
…-105735-fix, r=notriddle,aDotInTheVoid

Fix invalid display of inlined re-export when both local and foreign items are inlined

Fixes rust-lang#105735.

The bug is actually quite interesting: at the `clean` pass, local inlined items have their `use` item removed, however foreign items don't have their `use` item removed because it's in the `clean` pass that we handle them. So when a `use` inlines both a local and a foreign item, it will work as expected for the foreign one, but not for the local as its `use` should not be around anymore.

To prevent this, I created a new `inlined_foreigns` field into the `Module` struct to allow to remove the `use` item early on for foreign items as well. Then we iterate it in the `clean` pass directly.

r? ``@notriddle``
@bors
Copy link
Contributor

bors commented Jul 18, 2023

⌛ Testing commit afec6d2 with merge 63968fcb484c49cecc6576490ac28dd60e14f40a...

@bors
Copy link
Contributor

bors commented Jul 18, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 18, 2023
@GuillaumeGomez
Copy link
Member Author

Flaky failure:

error: failed to get `elsa` as a dependency of package `rustc_data_structures v0.0.0 (/checkout/compiler/rustc_data_structures)`

@bors retry

@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 Jul 18, 2023
@bors
Copy link
Contributor

bors commented Jul 18, 2023

⌛ Testing commit afec6d2 with merge 5fb88f6441afd6781a1bdb8d2c3b2fd2e9cb878f...

@bors
Copy link
Contributor

bors commented Jul 18, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 18, 2023
@GuillaumeGomez
Copy link
Member Author

CI seems dead for the moment.

@rust-log-analyzer
Copy link
Collaborator

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

Click to see the possible cause of the failure (guessed by this bot)
[TIMING] compile::Sysroot { compiler: Compiler { stage: 0, host: x86_64-unknown-linux-gnu }, force_recompile: false } -- 0.000
[TIMING] builder::Builder::sysroot_libdir::Libdir { compiler: Compiler { stage: 0, host: x86_64-unknown-linux-gnu }, target: x86_64-unknown-linux-gnu } -- 0.000
##[group]Building stage0 library artifacts (x86_64-unknown-linux-gnu)
    Updating crates.io index
error: failed to get `elsa` as a dependency of package `rustc_data_structures v0.0.0 (/checkout/compiler/rustc_data_structures)`
Caused by:
  failed to query replaced source registry `crates-io`

Caused by:
Caused by:
  download of el/sa/elsa failed
Caused by:
Caused by:
  failed to get successful HTTP response from `https://index.crates.io/el/sa/elsa` (18.164.116.95), got 421
  debug headers:
  x-amz-cf-pop: EWR53-P1
  x-cache: Error from cloudfront
  x-amz-cf-pop: JFK50-P6
  x-amz-cf-id: Tss8cQnMKBy6YM9NLAiKwDpP8tqz1NZ67wwRmnJnjLK-y8i6MYM9dg==
  <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd">
  <HTML><HEAD><META HTTP-EQUIV="Content-Type" CONTENT="text/html; charset=iso-8859-1">
  <TITLE>ERROR: The request could not be satisfied</TITLE>
  </HEAD><BODY>

@rust-log-analyzer
Copy link
Collaborator

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

Click to see the possible cause of the failure (guessed by this bot)

@GuillaumeGomez
Copy link
Member Author

@bors retry

@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 Jul 18, 2023
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jul 19, 2023
…-105735-fix, r=notriddle,aDotInTheVoid

Fix invalid display of inlined re-export when both local and foreign items are inlined

Fixes rust-lang#105735.

The bug is actually quite interesting: at the `clean` pass, local inlined items have their `use` item removed, however foreign items don't have their `use` item removed because it's in the `clean` pass that we handle them. So when a `use` inlines both a local and a foreign item, it will work as expected for the foreign one, but not for the local as its `use` should not be around anymore.

To prevent this, I created a new `inlined_foreigns` field into the `Module` struct to allow to remove the `use` item early on for foreign items as well. Then we iterate it in the `clean` pass directly.

r? `@notriddle`
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 19, 2023
Rollup of 7 pull requests

Successful merges:

 - rust-lang#113444 (add tests for alias bound preference)
 - rust-lang#113716 (Add the `no-builtins` attribute to functions when `no_builtins` is applied at the crate level.)
 - rust-lang#113754 (Simplify native_libs query)
 - rust-lang#113765 (Make it clearer that edition functions are `>=`, not `==`)
 - rust-lang#113774 (Improve error message when closing bracket interpreted as formatting fill character)
 - rust-lang#113785 (Fix invalid display of inlined re-export when both local and foreign items are inlined)
 - rust-lang#113803 (Fix inline_const with interpolated block)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit c7c8914 into rust-lang:master Jul 19, 2023
11 of 12 checks passed
@rustbot rustbot added this to the 1.73.0 milestone Jul 19, 2023
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.

Regression in rustdoc of re-export involving namespace overlap
5 participants