Skip to content

Conversation

surechen
Copy link
Contributor

@surechen surechen commented Sep 6, 2023

fixes #114896

I find we have to get pat_span but not local_decl.source_info.span for suggestion. In let &b = a; pat_span is &b. I think check let &b = a in hir to make sure it is hir::Node::Local(hir::Local {pat: hir::Pat{kind: hir::PatKind::Ref(....... can distinguish it from other situation, but I'm not sure.

If my processing method is not accurate, please guide me to modify it, thank you.

r? @davidtwco

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 6, 2023
debug!("local_decl: {:?}", local_decl);
let pat_span = match *local_decl.local_info() {
LocalInfo::User(BindingForm::Var(mir::VarBindingForm {
binding_mode: ty::BindingMode::BindByValue(Mutability::Not),
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably not in scope for this PR, but we really should put a HirId in here.

Copy link
Contributor Author

@surechen surechen Sep 6, 2023

Choose a reason for hiding this comment

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

Probably not in scope for this PR, but we really should put a HirId in here.

Thanks very much, since this will cause multiple changes when using LocalDecl, I thought I could submit another pr to support this.

@davidtwco
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Sep 11, 2023

📌 Commit a8f3c76 has been approved by davidtwco

It is now in the queue for this repository.

@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 Sep 11, 2023
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Sep 11, 2023
Fix incorrect mutable suggestion information for binding in ref pattern like:  `let &b = a;`

fixes rust-lang#114896

I find we have to get pat_span but not local_decl.source_info.span for suggestion. In `let &b = a;`  pat_span is &b. I think check `let &b = a` in hir to make sure it is hir::Node::Local(hir::Local {pat: hir::Pat{kind: hir::PatKind::Ref(.......   can distinguish it from other situation, but I'm not sure.

If my processing method is not accurate, please guide me to modify it, thank you.

r? `@davidtwco`
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 11, 2023
Rollup of 7 pull requests

Successful merges:

 - rust-lang#115308 (suggest iter_mut() where trying to modify elements from .iter())
 - rust-lang#115595 (Fix incorrect mutable suggestion information for binding in ref pattern like:  `let &b = a;` )
 - rust-lang#115702 (Update mailmap)
 - rust-lang#115708 (fix homogeneous_aggregate not ignoring some ZST)
 - rust-lang#115739 (Call `LateLintPass::check_attribute` from `with_lint_attrs`)
 - rust-lang#115743 (Point out if a local trait has no implementations)
 - rust-lang#115744 (Improve diagnostic for generic params from outer items (E0401))

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Collaborator

bors commented Sep 11, 2023

⌛ Testing commit a8f3c76 with merge 3ebb562...

@bors
Copy link
Collaborator

bors commented Sep 11, 2023

☀️ Test successful - checks-actions
Approved by: davidtwco
Pushing 3ebb562 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 11, 2023
@bors bors merged commit 3ebb562 into rust-lang:master Sep 11, 2023
@rustbot rustbot added this to the 1.74.0 milestone Sep 11, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (3ebb562): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.8% [2.8%, 2.8%] 1
Improvements ✅
(primary)
-1.1% [-1.6%, -0.6%] 2
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -1.1% [-1.6%, -0.6%] 2

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-1.9% [-1.9%, -1.9%] 1
All ❌✅ (primary) - - 0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 629.05s -> 629.503s (0.07%)
Artifact size: 317.65 MiB -> 317.74 MiB (0.03%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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. 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.

consider changing this to be mutable suggested for a binding in a ref pattern
6 participants