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.

Sign up for GitHub

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

Jump to bottom

shrinking the deprecated method span #85617

Merged
merged 3 commits into from
Jun 3, 2021
Merged

shrinking the deprecated method span #85617

merged 3 commits into from
Jun 3, 2021

Conversation

Rustin170506
Copy link
Member

part of #85403

r? @estebank

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 24, 2021
@Rustin170506
Copy link
Member Author

@estebank Could you please take a look? Thanks!

@estebank
Copy link
Contributor

estebank commented Jun 2, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Jun 2, 2021

📌 Commit 957e2ef has been approved by estebank

@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 Jun 2, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jun 3, 2021
@Dylan-DPC
Rollup merge of rust-lang#85617 - hi-rustin:rustin-patch-fix, r=estebank
ab0b248 
shrinking the deprecated method span

part of rust-lang#85403

r? `@estebank`
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Jun 3, 2021
Closed
@bors
Copy link
Contributor

bors commented Jun 3, 2021

⌛ Testing commit 957e2ef with merge cc77ba4...

@bors
Copy link
Contributor

bors commented Jun 3, 2021

☀️ Test successful - checks-actions
Approved by: estebank
Pushing cc77ba4 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 3, 2021
@bors bors merged commit cc77ba4 into rust-lang:master Jun 3, 2021
@rustbot rustbot added this to the 1.54.0 milestone Jun 3, 2021
@richkadel richkadel mentioned this pull request Jun 4, 2021
Closed
@richkadel
Copy link
Contributor

FYI, this PR causes invalid warnings in some cases, causing build issues for some projects. Please take a look at #86000, which includes an MCVE to see what I'm referring to. Thanks!

@richkadel richkadel mentioned this pull request Jun 4, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 5, 2021
@bors
Auto merge of rust-lang#86001 - richkadel:revert-85617-rustin-patch-f…
5ea1923 
…ix, r=Mark-Simulacrum

Revert "shrinking the deprecated method span"

Reverts rust-lang#85617

Fixes: rust-lang#86000

r? `@Mark-Simulacrum`
@estebank
Copy link
Contributor

estebank commented Jun 5, 2021

@hi-rustin This got reverted and will need to be re-landed after addressing the invalid warning.

@Rustin170506
Copy link
Member Author

@hi-rustin This got reverted and will need to be re-landed after addressing the invalid warning.

Thanks, I will try to fix it.

@Rustin170506
Copy link
Member Author

@hi-rustin This got reverted and will need to be re-landed after addressing the invalid warning.

@estebank Do you have any thoughts on this issue? Maybe we should judge in the visit_path that only the method is processed? But I'm not sure how to determine if a segment is a method or not.

@estebank
Copy link
Contributor

You might be able to "just" look at the path_segment.res to see if it is Some(Res::Def(DefKind::AssocFn | DefKind::AssocTy | DefKind::AssocConst, _).

@Rustin170506 Rustin170506 mentioned this pull request Jun 15, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 12, 2021
@bors
Auto merge of rust-lang#86320 - hi-rustin:rustin-patch-fix-span, r=es…
955b9c0 
…tebank

shrinking the deprecated span

ref: rust-lang#85617 (comment)

part of rust-lang#85403

r? `@estebank`

The reason is that if we use method_span directly, it will cause the in_derive_expansion judgment to fail.
@dtolnay dtolnay mentioned this pull request Aug 31, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@estebank estebank Awaiting requested review from estebank

@camsteffen camsteffen Awaiting requested review from camsteffen

Assignees

@estebank estebank

Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.54.0
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@Rustin170506 @estebank @bors @richkadel @camsteffen @rust-highfive @rustbot