Skip to content

Backport 46112 fix to beta - #46905

Merged
bors merged 3 commits into
rust-lang:betafrom
pnkfelix:backport-46112-fix-to-beta
Dec 23, 2017
Merged

Backport 46112 fix to beta#46905
bors merged 3 commits into
rust-lang:betafrom
pnkfelix:backport-46112-fix-to-beta

Conversation

@pnkfelix

@pnkfelixpnkfelix commented Dec 21, 2017

Copy link
Copy Markdown
Contributor

Its probably easiest to focus on the diff rather than the commit series.

If you prefer I could squash them, but I figured preserving the cherry-picking will make it easier to relate what has happened here to what happens on the master branch.

Note: This is a backport of just#46838. It does not include #46708 (which we may end up wanting to revert).

Fix#46112

Sorting by crate-num should ensure that we favor `std::foo::bar` over
`any_other_crate::foo::bar`.
Interestingly, *this* change had a much larger impact on our internal
test suite than PR rust-lang#46708 (which was my original fix to rust-lang#46112).
----
(This is cherry-pick of aa030dd to beta.)
…re::`
The reason we see `core::` even after visiting the `std` crate first
is the special case code that looks like this:
```rust
Entry::Occupied(mut entry) => {
// If `child` is defined in crate `cnum`, ensure
// that it is mapped to a parent in `cnum`.
if child.krate == cnum && entry.get().krate != cnum {
entry.insert(parent);
}
}
```
This causes items to be associated with the crates they were
originally defined in, even if we had encountered them during the
traversal of an earlier crate.
(Having said that, I am not clear on why this same logic does not
apply when both rust-lang#46708 and rust-lang#46838 have been applied. But at this
point, I am just happy to have a plausible explanation for why we see
`core::foo::bar` in the output for these tests, and want to focus on
getting this fix for rust-lang#46112 backported to beta.)
@rust-highfive

Copy link
Copy Markdown
Contributor

r? @michaelwoerister

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive

Copy link
Copy Markdown
Contributor

warningWarningwarning

  • Pull requests are usually filed against the master branch for this repo, but this one is against beta. Please double check that you specified the right target!

@pnkfelixpnkfelix added the P-high High priority label Dec 21, 2017
@michaelwoerister

Copy link
Copy Markdown
Member

@bors r+

@bors

bors commented Dec 21, 2017

Copy link
Copy Markdown
Collaborator

📌 Commit 5f85764 has been approved by michaelwoerister

@kennytmkennytm added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Dec 21, 2017
@kennytm

Copy link
Copy Markdown
Member

@bors p=2

Beta backport.

@bors

bors commented Dec 23, 2017

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 5f85764 with merge e878c67...

bors added a commit that referenced this pull request Dec 23, 2017
…woerister
Backport 46112 fix to beta
Its probably easiest to focus on the diff rather than the commit series.
If you prefer I could squash them, but I figured preserving the cherry-picking will make it easier to relate what has happened here to what happens on the `master` branch.
Note: This is a backport of *just* #46838. It does not include #46708 (which we may end up wanting to revert).
Fix#46112
@bors

bors commented Dec 23, 2017

Copy link
Copy Markdown
Collaborator

☀️ Test successful - status-appveyor, status-travis
Approved by: michaelwoerister
Pushing e878c67 to beta...

@bors
bors merged commit 5f85764 into rust-lang:betaDec 23, 2017
@apirainoapiraino removed the P-high High priority label Feb 8, 2022
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@pnkfelix@rust-highfive@michaelwoerister@bors@kennytm@apiraino