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.

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

remove redundent "<>" for ty::Slice with reference type #103675

Merged
merged 1 commit into from
Nov 10, 2022

Conversation

lyming2007
Copy link

this fix #103271

@rustbot
Copy link
Collaborator

rustbot commented Oct 28, 2022

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @fee1-dead (or someone else) soon.

Please see the contribution instructions for more information.

@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 Oct 28, 2022
Copy link
Member

@fee1-dead fee1-dead left a comment

Choose a reason for hiding this comment

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

There might be a better way. AFAICT this diagnostic is erroneous because the span it covers does not contain the angle brackets, so it prints the suggestion on [_] instead of <[_]>. I believe this can be done pretty cheaply using span_to_source, skipping white space and enlarging the span if you see angle brackets from both sides.

@lyming2007
Copy link
Author

There might be a better way. AFAICT this diagnostic is erroneous because the span it covers does not contain the angle brackets, so it prints the suggestion on [_] instead of <[_]>. I believe this can be done pretty cheaply using span_to_source, skipping white space and enlarging the span if you see angle brackets from both sides.

pushed a new fix by seeing angle brackets from both sides.

@lyming2007 lyming2007 requested a review from fee1-dead November 2, 2022 03:32
Copy link
Member

@fee1-dead fee1-dead left a comment

Choose a reason for hiding this comment

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

One last nit. Should be good to go after this.

compiler/rustc_hir_typeck/src/method/suggest.rs Outdated Show resolved Hide resolved
@fee1-dead
Copy link
Member

Looks good to me, thanks for this PR!

@bors r+

@bors
Copy link
Contributor

bors commented Nov 3, 2022

📌 Commit 868ccd5 has been approved by fee1-dead

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 Nov 3, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Nov 3, 2022
…1-dead

remove redundent "<>" for ty::Slice with reference type

this fix rust-lang#103271
@Dylan-DPC
Copy link
Member

failed in rollup

@bors r-

@bors bors 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 Nov 3, 2022
@lyming2007
Copy link
Author

@fee1-dead should I do something for the merging conflict? This is the first time I encounter this type of conflict

@Dylan-DPC
Copy link
Member

What merging conflict? I don't see any conflict in your PR.

@lyming2007
Copy link
Author

failed in rollup

failed in rollup

It says failed in #103921 (comment)

@Dylan-DPC
Copy link
Member

Dylan-DPC commented Nov 4, 2022

yes it means your pr failed some tests when it was rolled up. You should see the failures in the error log (after a bit of scrolling ;P). You need to update those tests and then we can merge this again

@lyming2007
Copy link
Author

yes it means your pr failed some tests when it was rolled up. You should see the failures in the error log (after a bit of scrolling ;P). You need to update those tests and then we can merge this again

I changed according to that error log. But the CI didn't pass:

---- [ui] src/test/ui/type/issue-103271.rs stdout ----
diff of stderr:

7	   = note: the following trait bounds were not satisfied:
8	           `&[_]: ExactSizeIterator`
9	           which is required by `&mut &[_]: ExactSizeIterator`
+	   = help: items from traits can only be used if the trait is in scope
+	help: the following trait is implemented but not in scope; perhaps add a `use` for it:
+	   |
+	LL | use object::read::read_ref::ReadRef;
+	   |
10	help: the function `len` is implemented on `[_]`
11	   |
12	LL |     let length = <[_]>::len;

@lyming2007
Copy link
Author

yes it means your pr failed some tests when it was rolled up. You should see the failures in the error log (after a bit of scrolling ;P). You need to update those tests and then we can merge this again

I think my last code should fix the in the error log. can you merge it again?

@lyming2007
Copy link
Author

@Dylan-DPC any progress on it?

@Dylan-DPC
Copy link
Member

The CI doesn't seem to pass though? let me rerun it. Also i would wait for @fee1-dead 's confirmation whether it is fine to be merged again or not

@lyming2007
Copy link
Author

in the error log

I fixed it according to the error of this log error log
which seems conflict with the CI in PR #103675

@GuillaumeGomez
Copy link
Member

Failed in #104151.

@bors r-

@bors bors 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 Nov 8, 2022
@lyming2007
Copy link
Author

the test in https://github.com/rust-lang-ci/rust/actions/runs/3420197276/jobs/5694702753 failed as I mentioned before:

failures:

---- [ui] src/test/ui/type/issue-103271.rs stdout ----
diff of stderr:

7	   = note: the following trait bounds were not satisfied:
8	           `&[_]: ExactSizeIterator`
9	           which is required by `&mut &[_]: ExactSizeIterator`
-	   = help: items from traits can only be used if the trait is in scope
-	help: the following trait is implemented but not in scope; perhaps add a `use` for it:
-	   |
-	LL | use object::read::read_ref::ReadRef;
-	   |
15	help: the function `len` is implemented on `[_]`
16	   |
17	LL |     let length = <[_]>::len;

but if I removed these code:

-	   = help: items from traits can only be used if the trait is in scope
-	help: the following trait is implemented but not in scope; perhaps add a `use` for it:
-	   |
-	LL | use object::read::read_ref::ReadRef;
-	   |

This CI would fail at:

---- [ui] src/test/ui/type/issue-103271.rs stdout ----
diff of stderr:

7	   = note: the following trait bounds were not satisfied:
8	           `&[_]: ExactSizeIterator`
9	           which is required by `&mut &[_]: ExactSizeIterator`
+	   = help: items from traits can only be used if the trait is in scope
+	help: the following trait is implemented but not in scope; perhaps add a `use` for it:
+	   |
+	LL | use object::read::read_ref::ReadRef;
+	   |
10	help: the function `len` is implemented on `[_]`
11	   |
12	LL |     let length = <[_]>::len;

@fee1-dead

@fee1-dead
Copy link
Member

I asked on Zulip, and this is #26454. The solution here is to use another function name that does not have the suggestion.

@lyming2007
Copy link
Author

@fee1-dead should be ok now. please approve it again. :)

@fee1-dead
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Nov 10, 2022

📌 Commit 0b6934d has been approved by fee1-dead

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 10, 2022
Manishearth added a commit to Manishearth/rust that referenced this pull request Nov 10, 2022
…1-dead

remove redundent "<>" for ty::Slice with reference type

this fix rust-lang#103271
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 10, 2022
…mpiler-errors

Rollup of 9 pull requests

Successful merges:

 - rust-lang#102763 (Some diagnostic-related nits)
 - rust-lang#103443 (Parser: Recover from using colon as path separator in imports)
 - rust-lang#103675 (remove redundent "<>" for ty::Slice with reference type)
 - rust-lang#104046 (bootstrap: add support for running Miri on a file)
 - rust-lang#104115 (Migrate crate-search element to CSS variables)
 - rust-lang#104190 (Ignore "Change InferCtxtBuilder from enter to build" in git blame)
 - rust-lang#104201 (Add check in GUI test for file loading failure)
 - rust-lang#104211 (:arrow_up: rust-analyzer)
 - rust-lang#104231 (Update mailmap)

Failed merges:

 - rust-lang#104169 (Migrate `:target` rules to use CSS variables)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f916022 into rust-lang:master Nov 10, 2022
@rustbot rustbot added this to the 1.67.0 milestone Nov 10, 2022
@lyming2007 lyming2007 deleted the issue-103271-fix branch November 10, 2022 16:33
Aaron1011 pushed a commit to Aaron1011/rust that referenced this pull request Jan 6, 2023
…1-dead

remove redundent "<>" for ty::Slice with reference type

this fix rust-lang#103271
Aaron1011 pushed a commit to Aaron1011/rust that referenced this pull request Jan 6, 2023
…mpiler-errors

Rollup of 9 pull requests

Successful merges:

 - rust-lang#102763 (Some diagnostic-related nits)
 - rust-lang#103443 (Parser: Recover from using colon as path separator in imports)
 - rust-lang#103675 (remove redundent "<>" for ty::Slice with reference type)
 - rust-lang#104046 (bootstrap: add support for running Miri on a file)
 - rust-lang#104115 (Migrate crate-search element to CSS variables)
 - rust-lang#104190 (Ignore "Change InferCtxtBuilder from enter to build" in git blame)
 - rust-lang#104201 (Add check in GUI test for file loading failure)
 - rust-lang#104211 (:arrow_up: rust-analyzer)
 - rust-lang#104231 (Update mailmap)

Failed merges:

 - rust-lang#104169 (Migrate `:target` rules to use CSS variables)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

check_for_deref_method suggestion introduces a parse error
6 participants