Skip to content

Replace some guess_head_span with def_span - #98519

Merged
bors merged 4 commits into
rust-lang:masterfrom
TaKO8Ki:add-head-span-field-to-item-and-impl-item
Jul 6, 2022
Merged

Replace some guess_head_span with def_span#98519
bors merged 4 commits into
rust-lang:masterfrom
TaKO8Ki:add-head-span-field-to-item-and-impl-item

Conversation

@TaKO8Ki

@TaKO8KiTaKO8Ki commented Jun 26, 2022

Copy link
Copy Markdown
Member

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

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

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@TaKO8KiTaKO8Ki changed the title Add a head_span field to Item, ImplItem and TraitItem[WIP] Add a head_span field to Item, ImplItem and TraitItemJun 26, 2022
@TaKO8KiTaKO8Ki changed the title [WIP] Add a head_span field to Item, ImplItem and TraitItemAdd a head_span field to Item, ImplItem and TraitItemJun 26, 2022
@TaKO8Ki
TaKO8Kiforce-pushed the add-head-span-field-to-item-and-impl-item branch from 319e296 to 0d70331CompareJune 26, 2022 05:53
@cjgillot

cjgillot commented Jun 26, 2022

Copy link
Copy Markdown
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

TaKO8Ki commented Jun 26, 2022

Copy link
Copy Markdown
MemberAuthor

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

@TaKO8Ki
TaKO8Kiforce-pushed the add-head-span-field-to-item-and-impl-item branch from 0d70331 to 1a4ebb7CompareJuly 3, 2022 03:28
@TaKO8KiTaKO8Ki changed the title Add a head_span field to Item, ImplItem and TraitItemReplace some guess_head_span with def_spanJul 3, 2022
cjgillot
cjgillot reviewed Jul 3, 2022
View reviewed changes
Comment threadcompiler/rustc_hir/src/intravisit.rs Outdated
Comment threadcompiler/rustc_trait_selection/src/traits/specialize/mod.rs Outdated
Comment threadcompiler/rustc_trait_selection/src/traits/specialize/mod.rs Outdated
Comment threadcompiler/rustc_typeck/src/astconv/mod.rs Outdated
Comment threadcompiler/rustc_typeck/src/check/compare_method.rs Outdated
@cjgillotcjgillot 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
TaKO8Kiforce-pushed the add-head-span-field-to-item-and-impl-item branch from 1a4ebb7 to 7973d5bCompareJuly 4, 2022 02:10
@TaKO8KiTaKO8Ki changed the title Replace some guess_head_span with def_spanReplace guess_head_span with def_spanJul 4, 2022
@TaKO8KiTaKO8Ki mentioned this pull request Jul 4, 2022
Merged
@TaKO8KiTaKO8Ki changed the title Replace guess_head_span with def_spanReplace some guess_head_span with def_spanJul 4, 2022
@TaKO8Ki
TaKO8Ki requested a review from cjgillotJuly 4, 2022 02:32
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@TaKO8Ki
TaKO8Kiforce-pushed the add-head-span-field-to-item-and-impl-item branch from 7973d5b to 2c4bdc2CompareJuly 4, 2022 03:04
@TaKO8Ki

TaKO8Ki commented Jul 4, 2022

Copy link
Copy Markdown
MemberAuthor

@rustbot ready

@rustbotrustbot 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
cjgillot
cjgillot approved these changes Jul 4, 2022
View reviewed changes

@cjgillotcjgillot left a comment

Copy link
Copy Markdown
Contributor

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.

Comment threadcompiler/rustc_trait_selection/src/traits/specialize/mod.rs Outdated
Comment threadcompiler/rustc_typeck/src/check/method/suggest.rs Outdated
Comment threadcompiler/rustc_typeck/src/coherence/mod.rs Outdated
Comment threadsrc/test/ui/lint/lint-missing-doc.stderr Outdated

@cjgillotcjgillotJul 4, 2022

Copy link
Copy Markdown
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.

@TaKO8KiTaKO8KiJul 4, 2022

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I fixed it.

@TaKO8Ki
TaKO8Kiforce-pushed the add-head-span-field-to-item-and-impl-item branch from 2c4bdc2 to b9d4b13CompareJuly 4, 2022 08:25
@TaKO8Ki
TaKO8Ki requested a review from cjgillotJuly 4, 2022 08:30
cjgillot
cjgillot reviewed Jul 4, 2022
View reviewed changes
Comment threadcompiler/rustc_middle/src/hir/map/mod.rs Outdated
@rustbot

rustbot commented Jul 6, 2022

Copy link
Copy Markdown
Collaborator

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@TaKO8Ki

TaKO8Ki commented Jul 6, 2022

Copy link
Copy Markdown
MemberAuthor

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

@TaKO8Ki

TaKO8Ki commented Jul 6, 2022

Copy link
Copy Markdown
MemberAuthor

@rustbot ready

@rustbotrustbot 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

Dylan-DPC commented Jul 6, 2022

Copy link
Copy Markdown
Member

@bors r=cjgillot

@bors

bors commented Jul 6, 2022

Copy link
Copy Markdown
Collaborator

📌 Commit bd3bb338b6bcad885d50e14ef02412c24c32bc4a has been approved by cjgillot

@borsbors 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

bors commented Jul 6, 2022

Copy link
Copy Markdown
Collaborator

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

@borsbors 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
TaKO8Kiforce-pushed the add-head-span-field-to-item-and-impl-item branch from 6bab09c to b730bc9CompareJuly 6, 2022 10:14
@TaKO8Ki

TaKO8Ki commented Jul 6, 2022

Copy link
Copy Markdown
MemberAuthor

@Dylan-DPC Could you approve again?

@cjgillot

cjgillot commented Jul 6, 2022

Copy link
Copy Markdown
Contributor

@bors r+

@bors

bors commented Jul 6, 2022

Copy link
Copy Markdown
Collaborator

📌 Commit b730bc9 has been approved by cjgillot

@borsbors 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
@RalfJungRalfJung mentioned this pull request Jul 6, 2022
Merged
@GuillaumeGomezGuillaumeGomez mentioned this pull request Jul 6, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 6, 2022
@bors
Auto merge ofrust-lang#98987- GuillaumeGomez:rollup-bcy32bp, r=Guil…
7665c35
…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:masterJul 6, 2022
@rustbotrustbot 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
@TaKO8KiTaKO8Ki mentioned this pull request Jul 31, 2022
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 2, 2022
@matthiaskrgr
Rollup merge ofrust-lang#99994- TaKO8Ki:remove-guess-head-span, r=f…
17f76a1
…ee1-dead
Remove `guess_head_span`
follow-up to rust-lang#98519
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cjgillotcjgillotAwaiting requested review from cjgillot

Assignees

@cjgillotcjgillot

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.64.0

Development

Successfully merging this pull request may close these issues.

7 participants

@TaKO8Ki@rust-log-analyzer@cjgillot@bors@Dylan-DPC@rustbot@rust-highfive