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

Backport 46112 fix to beta #46905

Merged
merged 3 commits into from
Dec 23, 2017
Merged

Conversation

pnkfelix
Copy link
Member

@pnkfelix pnkfelix commented Dec 21, 2017

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
Collaborator

r? @michaelwoerister

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

@rust-highfive
Copy link
Collaborator

warning Warning warning

  • 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!

@pnkfelix pnkfelix added the P-high High priority label Dec 21, 2017
@michaelwoerister
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Dec 21, 2017

📌 Commit 5f85764 has been approved by michaelwoerister

@kennytm kennytm 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
Member

kennytm commented Dec 23, 2017

@bors p=2

Beta backport.

@bors
Copy link
Contributor

bors commented Dec 23, 2017

⌛ 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
Copy link
Contributor

bors commented Dec 23, 2017

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

@bors bors merged commit 5f85764 into rust-lang:beta Dec 23, 2017
@apiraino apiraino removed the P-high High priority label Feb 8, 2022
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants