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 some guess_head_span with def_span #98519

Merged

Conversation

TaKO8Ki
Copy link
Member

@TaKO8Ki TaKO8Ki commented Jun 26, 2022

This patch fixes a part of #97417.
r? @cjgillot

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 26, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 26, 2022
@TaKO8Ki TaKO8Ki force-pushed the add-head-span-field-to-item-and-impl-item branch from 51d1fef to 28bdcf4 Compare June 26, 2022 03:43
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@TaKO8Ki TaKO8Ki changed the title Add a head_span field to Item, ImplItem and TraitItem [WIP] Add a head_span field to Item, ImplItem and TraitItem Jun 26, 2022
@TaKO8Ki TaKO8Ki changed the title [WIP] Add a head_span field to Item, ImplItem and TraitItem Add a head_span field to Item, ImplItem and TraitItem Jun 26, 2022
@TaKO8Ki TaKO8Ki force-pushed the add-head-span-field-to-item-and-impl-item branch from 319e296 to 0d70331 Compare June 26, 2022 05:53
@cjgillot
Copy link
Contributor

Thanks @TaKO8Ki.
FYI #93967 attempts to make def_span return something similar to the head_span you computed.
I think that the removal of calls to guess_head_span that you did will still be valid, with a direct call to def_span instead of an additional head_span field. What do you think?

@TaKO8Ki
Copy link
Member Author

TaKO8Ki commented Jun 26, 2022

@cjgillot I agree with you. I will replace head_span() with def_span() after #93967 is merged.

@TaKO8Ki TaKO8Ki force-pushed the add-head-span-field-to-item-and-impl-item branch from 0d70331 to 1a4ebb7 Compare July 3, 2022 03:28
@TaKO8Ki TaKO8Ki changed the title Add a head_span field to Item, ImplItem and TraitItem Replace some guess_head_span with def_span Jul 3, 2022
@cjgillot cjgillot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 3, 2022
@TaKO8Ki TaKO8Ki force-pushed the add-head-span-field-to-item-and-impl-item branch from 1a4ebb7 to 7973d5b Compare July 4, 2022 02:10
@TaKO8Ki TaKO8Ki changed the title Replace some guess_head_span with def_span Replace guess_head_span with def_span Jul 4, 2022
@TaKO8Ki TaKO8Ki changed the title Replace guess_head_span with def_span Replace some guess_head_span with def_span Jul 4, 2022
@TaKO8Ki TaKO8Ki requested a review from cjgillot July 4, 2022 02:32
@rust-log-analyzer

This comment has been minimized.

@TaKO8Ki TaKO8Ki force-pushed the add-head-span-field-to-item-and-impl-item branch from 7973d5b to 2c4bdc2 Compare July 4, 2022 03:04
@TaKO8Ki
Copy link
Member Author

TaKO8Ki commented Jul 4, 2022

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 4, 2022
Copy link
Contributor

@cjgillot cjgillot left a comment

Choose a reason for hiding this comment

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

Thanks @TaKO8Ki. This looks good.
A few nits then r=me.

compiler/rustc_typeck/src/check/method/suggest.rs Outdated Show resolved Hide resolved
compiler/rustc_typeck/src/coherence/mod.rs Outdated Show resolved Hide resolved
@@ -79,8 +79,10 @@ LL | pub enum PubBaz {
error: missing documentation for a variant
--> $DIR/lint-missing-doc.rs:119:5
|
LL | PubBazA {
| ^^^^^^^
LL | / PubBazA {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a regression. It can easily be cured by modifying tcx.hir().opt_span. If you want to do it, this can wait a follow-up PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

I fixed it.

@TaKO8Ki TaKO8Ki force-pushed the add-head-span-field-to-item-and-impl-item branch from 2c4bdc2 to b9d4b13 Compare July 4, 2022 08:25
@TaKO8Ki TaKO8Ki requested a review from cjgillot July 4, 2022 08:30
@rustbot
Copy link
Collaborator

rustbot commented Jul 6, 2022

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@TaKO8Ki
Copy link
Member Author

TaKO8Ki commented Jul 6, 2022

@cjgillot I fixed clippy tests. Could you review again?

@TaKO8Ki
Copy link
Member Author

TaKO8Ki commented Jul 6, 2022

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 6, 2022
@Dylan-DPC
Copy link
Member

@bors r=cjgillot

@bors
Copy link
Contributor

bors commented Jul 6, 2022

📌 Commit bd3bb338b6bcad885d50e14ef02412c24c32bc4a has been approved by cjgillot

@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 Jul 6, 2022
@bors
Copy link
Contributor

bors commented Jul 6, 2022

☔ The latest upstream changes (presumably #98206) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 6, 2022
@TaKO8Ki TaKO8Ki force-pushed the add-head-span-field-to-item-and-impl-item branch from 6bab09c to b730bc9 Compare July 6, 2022 10:14
@TaKO8Ki
Copy link
Member Author

TaKO8Ki commented Jul 6, 2022

@Dylan-DPC Could you approve again?

@cjgillot
Copy link
Contributor

cjgillot commented Jul 6, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Jul 6, 2022

📌 Commit b730bc9 has been approved by cjgillot

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 6, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 6, 2022
…laumeGomez

Rollup of 7 pull requests

Successful merges:

 - rust-lang#96935 (Allow arithmetic and certain bitwise ops on AtomicPtr)
 - rust-lang#98519 (Replace some `guess_head_span` with `def_span`)
 - rust-lang#98911 (rustdoc: filter '_ lifetimes from ty::Generics)
 - rust-lang#98939 (rustdoc: Add more semantic information to impl IDs)
 - rust-lang#98971 (Fix typo in file descriptor docs)
 - rust-lang#98983 (docs: Add overview of `rustc_middle::mir::TerminatorKind`)
 - rust-lang#98984 (Remove erroneous doc comment)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit d712f67 into rust-lang:master Jul 6, 2022
@rustbot rustbot added this to the 1.64.0 milestone Jul 6, 2022
@TaKO8Ki TaKO8Ki deleted the add-head-span-field-to-item-and-impl-item branch July 7, 2022 00:37
@TaKO8Ki TaKO8Ki mentioned this pull request Jul 31, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler 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