-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Upgrade Chalk to 0.21 #75173
Upgrade Chalk to 0.21 #75173
Conversation
☔ The latest upstream changes (presumably #75276) made this pull request unmergeable. Please resolve the merge conflicts. |
@nikomatsakis gentle ping on this. I'll rebase after review, since there might be comments anyways. |
src/librustc_traits/chalk/db.rs
Outdated
@@ -54,11 +77,7 @@ impl<'tcx> chalk_solve::RustIrDatabase<RustInterner<'tcx>> for RustIrDatabase<'t | |||
// FIXME(chalk): this really isn't right I don't think. The functions |
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.
Is this comment still relevant?
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.
Yes
_ => (), | ||
}; | ||
|
||
t.super_visit_with(self) |
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.
Can be moved to the wildcard match, I think? Same with visit_region
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.
Mmm no, because this returns bool
src/librustc_traits/chalk/mod.rs
Outdated
})); | ||
|
||
let mut params_substitutor = | ||
ParamsSubstitutor::new(tcx, placeholders_collector.next_ty_placehoder); | ||
let obligation = obligation.fold_with(&mut params_substitutor); | ||
let _params: FxHashMap<usize, ParamTy> = params_substitutor.params; |
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 think this can be removed?
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.
It's unused yes, but it's here as a reminder to use it eventually. I left a FIXME
comment.
☔ The latest upstream changes (presumably #75120) made this pull request unmergeable. Please resolve the merge conflicts. |
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.
r=me when rebased!
Rebased. |
@bors r+ |
📌 Commit 7052197d73247b40c9b1413f276ba792d9aeafd3 has been approved by |
⌛ Testing commit 7052197d73247b40c9b1413f276ba792d9aeafd3 with merge 46195f03f1857ace1cbbe9bc877364b430a0c10b... |
💔 Test failed - checks-actions |
Needs a rebase. Kind of expected this seeing the bors queue this morning. Let me do that. |
@bors delegate+ r+ |
✌️ @jackh726 can now approve this pull request |
📌 Commit fafdfaf has been approved by |
Upgrade Chalk to 0.21 Two commits here. First commit actually does the upgrade. Second commit has some changes to make more tests in compare-mode=chalk pass. The `PlaceholdersCollector` and `RegionsSubstitutor` bits are bit a hacky, but only insomuch as `ParamsSubstitutor` is. These won't be needed eventually. r? @nikomatsakis
Welp another rebase needed. |
@bors r=nikomatsakis |
📌 Commit 0aa2153 has been approved by |
☀️ Test successful - checks-actions, checks-azure |
Two commits here. First commit actually does the upgrade. Second commit has some changes to make more tests in compare-mode=chalk pass.
The
PlaceholdersCollector
andRegionsSubstitutor
bits are bit a hacky, but only insomuch asParamsSubstitutor
is. These won't be needed eventually.r? @nikomatsakis