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

cleanup: Unify find_use_placement approaches. #94941

Closed
pnkfelix opened this issue Mar 14, 2022 · 2 comments · Fixed by #95194
Closed

cleanup: Unify find_use_placement approaches. #94941

pnkfelix opened this issue Mar 14, 2022 · 2 comments · Fixed by #95194
Assignees
@kckeiks
Labels
A-diagnostics Area: Messages for errors, warnings, and lints E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@pnkfelix
Copy link
Member

pnkfelix commented Mar 14, 2022 •
edited by rustbot
Loading

Spawned off of: #94584 (comment)

The code for find_use_placement was at one point roughly the same as the algorithm used prior to PR #94584.

PR #94584 changed things to use a more robust algorithm. But it didn't handle the find_use_placement code.

The two could be brought into sync, as documented in a FIXME
@pnkfelix pnkfelix added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 14, 2022
@pnkfelix pnkfelix changed the title Unify find_use_placement approaches. cleanup: Unify find_use_placement approaches. Mar 14, 2022
@pnkfelix pnkfelix mentioned this issue Mar 14, 2022
Merged
@pnkfelix pnkfelix added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels Mar 14, 2022
@pnkfelix
Copy link
Member Author

(I think I should be able to mentor this.)

@kckeiks
Copy link
Contributor

kckeiks commented Mar 14, 2022

@rustbot claim

@kckeiks kckeiks mentioned this issue Mar 22, 2022
Merged
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Apr 14, 2022
@Dylan-DPC
Rollup merge of rust-lang#95194 - kckeiks:update-algo-in-find-use-pla…
e1ace84 
…cement, r=pnkfelix

remove find_use_placement

A more robust solution to finding where to place use suggestions was added in rust-lang#94584.
The algorithm uses the AST to find the span for the suggestion so we pass this span
down to the HIR during lowering and use it instead of calling `find_use_placement`

Fixes rust-lang#94941
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Apr 15, 2022
@Dylan-DPC
Rollup merge of rust-lang#95194 - kckeiks:update-algo-in-find-use-pla…
5f525be 
…cement, r=pnkfelix

remove find_use_placement

A more robust solution to finding where to place use suggestions was added in rust-lang#94584.
The algorithm uses the AST to find the span for the suggestion so we pass this span
down to the HIR during lowering and use it instead of calling `find_use_placement`

Fixes rust-lang#94941
RalfJung added a commit to RalfJung/rust that referenced this issue Apr 15, 2022
@RalfJung
Rollup merge of rust-lang#95194 - kckeiks:update-algo-in-find-use-pla…
38d81e2 
…cement, r=pnkfelix

remove find_use_placement

A more robust solution to finding where to place use suggestions was added in rust-lang#94584.
The algorithm uses the AST to find the span for the suggestion so we pass this span
down to the HIR during lowering and use it instead of calling `find_use_placement`

Fixes rust-lang#94941
RalfJung added a commit to RalfJung/rust that referenced this issue Apr 15, 2022
@RalfJung
Rollup merge of rust-lang#95194 - kckeiks:update-algo-in-find-use-pla…
0ee9aae 
…cement, r=pnkfelix

remove find_use_placement

A more robust solution to finding where to place use suggestions was added in rust-lang#94584.
The algorithm uses the AST to find the span for the suggestion so we pass this span
down to the HIR during lowering and use it instead of calling `find_use_placement`

Fixes rust-lang#94941
@bors bors closed this as completed in bdbf099 Apr 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kckeiks kckeiks

Labels
A-diagnostics Area: Messages for errors, warnings, and lints E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

remove find_use_placement kckeiks/rust
2 participants
@pnkfelix @kckeiks