-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
HirId-ify intravisit #58232
HirId-ify intravisit #58232
Conversation
r? @oli-obk (rust_highfive has picked a reviewer for you, use r? to override) |
r? @Zoxc |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
There's a single stack trace
I think it's caused by the way the @Zoxc: I think |
I'm not sure what the cause of the error here is. I'd try reverting the change to |
☔ The latest upstream changes (presumably #58254) made this pull request unmergeable. Please resolve the merge conflicts. |
bce6cc8
to
962c04b
Compare
I haven't cracked the error yet; it doesn't seem to be caused by changes to I have an idea for a fix based on #56143, though. |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
962c04b
to
693b1f1
Compare
Yep, following issue #56128 was the right way to go; |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
693b1f1
to
a8e1052
Compare
@Zoxc r? |
a8e1052
to
3cdc2c0
Compare
@Zoxc done. Now in order to be able to later remove |
I think you missed my second comment in https://github.com/rust-lang/rust/pull/58232/files#diff-2534024f87f9d909baca60bae845a501R176. |
I think this should do the trick. @kennytm if breaking tools breaks merges now, wouldn't it make sense for tool tests to be included in Travis? |
@ljedrz We'd need extra capacity to run the tool check for every PR. But if the last commit message of the PR contains the words "Update Clippy" then Travis will run the tool tests. |
e39bb00
to
404e643
Compare
The tools build is green; can I get an r+ or rollup? I'd really like to move on now ^^. |
Where does this Clippy commit comes from?
Submodule commit should always point to merge commit. |
@mati865 it's in my clippy branch. Won't it be merged with clippy's master? It's my first time modifying a submodule, I'd be happy to adjust it if necessary. |
@ljedrz That commit must exist inside the rust-clippy repository. You'll need to submit a PR to the rust-clippy repository, which a clippy maintainer could create a branch pointing to that commit (or merge it). |
@kennytm cool, done. |
@bors r=Zoxc |
📌 Commit 404e643 has been approved by |
@bors p=1 due to submodule update so it won't go in a rollup. |
HirId-ify intravisit A big step towards #57578. This affects mostly `hir::{collector, intravisit}` and `rustc::lint`.
☀️ Test successful - checks-travis, status-appveyor |
☀️ Test successful - checks-travis, status-appveyor |
partially HirIdify lints Enables rust-lang/rust#58232 (a part of rust-lang/rust#57578).
partially HirIdify lints Enables rust-lang/rust#58232 (a part of rust-lang/rust#57578).
A big step towards #57578.
This affects mostly
hir::{collector, intravisit}
andrustc::lint
.