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

Replace (Body, DefId) with Body where possible #77552

Merged
merged 5 commits into from
Oct 5, 2020

Conversation

ecstatic-morse
Copy link
Contributor

@ecstatic-morse ecstatic-morse commented Oct 4, 2020

Follow-up to #77430.

I grep-ed for parameter lists in which a Body appeared within a few lines of a DefId, so it's possible that I missed some cases, but this should be pretty complete. Most of these changes were mechanical, but there's a few places where I started calling things "caller" and "callee" when multiple DefIds were in-scope at once. Also, we should probably have a helper function on Body that returns a LocalDefId. I can do that in this PR or in a follow-up.

@rust-highfive
Copy link
Collaborator

r? @estebank

(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 Oct 4, 2020
@ecstatic-morse
Copy link
Contributor Author

r? @lcnr (if you have the time)

@rust-highfive rust-highfive assigned lcnr and unassigned estebank Oct 4, 2020
@@ -758,7 +758,7 @@ impl<'tcx> Validator<'_, 'tcx> {
ty::FnDef(def_id, _) => {
is_const_fn(self.tcx, def_id)
|| is_unstable_const_fn(self.tcx, def_id).is_some()
|| is_lang_panic_fn(self.tcx, self.def_id.to_def_id())
|| is_lang_panic_fn(self.tcx, def_id)
Copy link
Contributor Author

@ecstatic-morse ecstatic-morse Oct 4, 2020

Choose a reason for hiding this comment

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

This line was incorrect. It should be checking the callee, not the caller. Really, it should just be removed entirely, since panicking in a const-context is still unstable and there's no need to promote diverging function calls.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

A `Body` now contains its `MirSource`, which in turn contains the
`DefId` of the item associated with the `Body`.
Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

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

fn do_mir_borrowck still takes a def

Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

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

That's a rather big improvement imo 👍

compiler/rustc_mir/src/borrow_check/nll.rs Outdated Show resolved Hide resolved
@@ -758,7 +758,7 @@ impl<'tcx> Validator<'_, 'tcx> {
ty::FnDef(def_id, _) => {
is_const_fn(self.tcx, def_id)
|| is_unstable_const_fn(self.tcx, def_id).is_some()
|| is_lang_panic_fn(self.tcx, self.def_id.to_def_id())
|| is_lang_panic_fn(self.tcx, def_id)
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@lcnr
Copy link
Contributor

lcnr commented Oct 5, 2020

Thanks, might be prone to merge conflicts

@bors r+ rollup=iffy

@bors
Copy link
Contributor

bors commented Oct 5, 2020

📌 Commit 04a94ab has been approved by lcnr

@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 Oct 5, 2020
@bors
Copy link
Contributor

bors commented Oct 5, 2020

⌛ Testing commit 04a94ab with merge 62bfcfd...

@bors
Copy link
Contributor

bors commented Oct 5, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: lcnr
Pushing 62bfcfd to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 5, 2020
@bors bors merged commit 62bfcfd into rust-lang:master Oct 5, 2020
@rustbot rustbot added this to the 1.49.0 milestone Oct 5, 2020
@ecstatic-morse ecstatic-morse deleted the body-def-id branch October 6, 2020 01:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants