-
Notifications
You must be signed in to change notification settings - Fork 13k
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
minor refactorings around InferCtxt::enter
#49020
minor refactorings around InferCtxt::enter
#49020
Conversation
☔ The latest upstream changes (presumably #47630) made this pull request unmergeable. Please resolve the merge conflicts. |
8432c77
to
84252f6
Compare
@@ -189,7 +189,7 @@ impl<'a, 'tcx> MaybeInProgressTables<'a, 'tcx> { | |||
/// `bar()` will each have their own `FnCtxt`, but they will | |||
/// share the inherited fields. | |||
pub struct Inherited<'a, 'gcx: 'a+'tcx, 'tcx: 'a> { | |||
infcx: InferCtxt<'a, 'gcx, 'tcx>, | |||
infcx: &'a InferCtxt<'a, 'gcx, 'tcx>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the extra indirection?
pub fn enter<F, R>(&'tcx mut self, f: F) -> R | ||
where F: for<'b> FnOnce(InferCtxt<'b, 'gcx, 'tcx>) -> R | ||
{ | ||
pub fn enter<'tcx, R>(&'tcx mut self, f: impl FnOnce(&InferCtxt<'_, 'gcx, 'tcx>) -> R) -> R { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it started here - why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can revert that part if you like -- I was actually working towards a goal with this patch series that didn't work out, so it's not really important either way (I just figured I'd land some of the incidental changes).
That said, I always get annoyed that enter gives me ownership of an InferCtxt
, when almost everything wants a reference. I'd be quite surprised if the indrection around Inherited
cost performance.
r=me with travis failure fixed |
☔ The latest upstream changes (presumably #49264) made this pull request unmergeable. Please resolve the merge conflicts. |
Sigh. I should really rebase this. |
Ping from triage, @nikomatsakis ! We agree you should rebase this ❤️. Might as well respond to the review feedback you got while you are at it! |
Eh. I don't care enough to rebase this. =) |
Just a few small simplifications.
r? @eddyb