4

tweak "make mut" spans when assigning to locals by Ezrashaw · Pull Req...

 1 year ago
source link: https://github.com/rust-lang/rust/pull/110583
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
neoserver,ios ssh client

Conversation

Contributor

Work towards fixing #106857

This PR just cleans up a lot of spans which is helpful before properly fixing the issues. Best reviewed commit-by-commit.

r? @estebank

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

Apr 20, 2023

@@ -6,8 +6,8 @@ LL | *input = self.0;

|

help: consider changing that to be a mutable reference

|

LL | fn example(&self, input: &mut i32); // should suggest here

| ~~~~~~~~

Contributor

Author

I'm going to leave this for a follow-up PR if that's OK with you?

Contributor

As it is causing a clear visual regression I would prefer to investigate further and try to land the whole fix in a single PR (to avoid risking this slipping into beta by accident simply because we forgot to follow up).

Contributor

This is now fixed, right?

Ezrashaw reacted with thumbs up emoji

This comment has been minimized.

} else if binding_exists {

// shrink the span to just after the `&` in `&variable`

let span = span.with_lo(span.lo() + BytePos(1)).shrink_to_lo();

(true, span, "mut ".to_owned())

Contributor

For the test presenting the small regression, can you check that you're actually falling into this branch?

Contributor

Author

Yeah it does. The problem is that there is a call to is_error_in_trait after this function returns which overwrites the returned span (but not the string suggestion) to point to the trait (if local).

Contributor

Author

I've fixed it so that we call is_error_in_trait before all the suggestion logic and integrate the spans from that. See the latest commit.

Contributor

Author

@estebank I'm going to add some more commits and fix that regression. (EDIT: done, ready for review) I suspect there will still be a follow-up PR so this one doesn't get too big.

@rustbot author
@rustbot ready

rustbot

added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.

and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.

labels

Apr 21, 2023

This comment has been minimized.

Contributor

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

LL | fn reborrow_mut<'a>(t: &'a mut &'a mut i32) -> &'a mut i32 where &'a mut i32: Copy {

| ~~~~~~~~~~~~~~~~~~~

| +++

Contributor

Love the result!

Ezrashaw reacted with thumbs up emoji

Contributor

Sorry I didn't re-review before the merge conflict. r=me after rebase.

Ezrashaw reacted with thumbs up emoji

estebank

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

May 5, 2023

This comment has been minimized.

Contributor

Author

@estebank Since I don't have those fun r= perms, it'd be great if you could do that :)

Contributor

@bors r=estebank

Ezrashaw reacted with heart emoji

Contributor

pushpin Commit 3e64e98 has been approved by estebank

It is now in the queue for this repository.

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

May 7, 2023

bors

merged commit ff30b8c into

rust-lang:master

May 9, 2023

11 checks passed

rustbot

added this to the 1.71.0 milestone

May 9, 2023

rust-timer

added a commit to rust-lang-ci/rust that referenced this pull request

May 9, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

estebank

estebank approved these changes
Assignees

estebank

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

Milestone

1.71.0

Development

Successfully merging this pull request may close these issues.

None yet

6 participants

About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK