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

Remove FakeDefId::expect_local() #85093

Merged
merged 2 commits into from
May 9, 2021

Conversation

camelid
Copy link
Member

@camelid camelid commented May 8, 2021

This function returned a fake DefIndex, with no indication that it was
fake, when it was provided with a FakeDefId::Fake. Every use of the
function uses the returned DefIndex in a call to
tcx.local_def_id_to_hir_id(), which I'm pretty sure would panic if it
were given a fake DefIndex.

I removed the function and replaced all calls to it with a call to
expect_real() followed by DefId::expect_local() (that's a function
on the real DefId).

@camelid camelid added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. C-cleanup Category: PRs that clean code up or issues documenting cleanup. labels May 8, 2021
@rust-highfive
Copy link
Collaborator

Some changes occurred in intra-doc-links.

cc @jyn514

@rust-highfive
Copy link
Collaborator

r? @jyn514

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 8, 2021
@camelid
Copy link
Member Author

camelid commented May 8, 2021

r? @GuillaumeGomez (since Joshua's on break)

}

#[inline]
#[track_caller]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I did not remove #[inline]. It's just a quirk of the diff that it looks that way.

@rust-log-analyzer

This comment has been minimized.

This function returned a fake `DefIndex`, with no indication that it was
fake, when it was provided with a `FakeDefId::Fake`. Every use of the
function uses the returned `DefIndex` in a call to
`tcx.local_def_id_to_hir_id()`, which I'm pretty sure would panic if it
were given a fake `DefIndex`.

I removed the function and replaced all calls to it with a call to
`expect_real()` followed by `DefId::expect_local()` (that's a function
on the *real* `DefId`).
Now, in the case that the function is not inlined, the panic location
will be the caller's location, which is more helpful since the panic is
not `expect_real()`'s fault.
@GuillaumeGomez
Copy link
Member

Thanks!

@bors: r+

@bors
Copy link
Contributor

bors commented May 9, 2021

📌 Commit 4b7c8b0 has been approved by GuillaumeGomez

@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 May 9, 2021
@bors
Copy link
Contributor

bors commented May 9, 2021

⌛ Testing commit 4b7c8b0 with merge 19dae7b...

@bors
Copy link
Contributor

bors commented May 9, 2021

☀️ Test successful - checks-actions
Approved by: GuillaumeGomez
Pushing 19dae7b to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 9, 2021
@bors bors merged commit 19dae7b into rust-lang:master May 9, 2021
@rustbot rustbot added this to the 1.54.0 milestone May 9, 2021
@camelid camelid deleted the remove-fake-expect_local branch May 9, 2021 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. merged-by-bors This PR was explicitly merged by bors. 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.

7 participants