-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Fix suggestion of additional pub
when using pub pub fn ...
#87901
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
(rust-highfive has picked a reviewer for you, use r? to override) |
☔ The latest upstream changes (presumably #87781) made this pull request unmergeable. Please resolve the merge conflicts. |
9cc28cd
to
3316822
Compare
This is now ready for review, it suggests the correct fix for both visibility and keywords |
☔ The latest upstream changes (presumably #88992) made this pull request unmergeable. Please resolve the merge conflicts. |
Ping from triage: |
While we wait for the author to address the merge conflicts, I'll plan to review. |
Very sorry for the long response time, I have been otherwise occupied these past months. I'll try to rebase this over the weekend |
d95a128
to
e99e33d
Compare
Rebase done, integrated suggestion |
☔ The latest upstream changes (presumably #90671) made this pull request unmergeable. Please resolve the merge conflicts. |
e99e33d
to
ffd18ba
Compare
Sorry about commit c9d1d0b making things harder to review, I had to satisfy tidy and choose to do it once for all following Prs that will add tests to this folder |
This comment has been minimized.
This comment has been minimized.
☔ The latest upstream changes (presumably #91418) made this pull request unmergeable. Please resolve the merge conflicts. |
@poliorcetics why are you moving so many tests as part of this PR? If you move them, it should be as a separate PR. |
ffd18ba
to
d1fef2e
Compare
As said in the original commit, there were too many tests in the folder. This has been fixed in a commit in a separate PR in between so I removed the commit on mine. |
This comment has been minimized.
This comment has been minimized.
d1fef2e
to
be33ca7
Compare
@bors r+ |
📌 Commit be33ca7 has been approved by |
…askrgr Rollup of 7 pull requests Successful merges: - rust-lang#87901 (Fix suggestion of additional `pub` when using `pub pub fn ...`) - rust-lang#89090 (Lint bare traits in AstConv.) - rust-lang#91818 (Show the unused type for `unused_results` lint) - rust-lang#91910 (miri: lift restriction on extern types being the only field in a struct) - rust-lang#91928 (Constify (most) `Option` methods) - rust-lang#91975 (Move generator check earlier in inlining.) - rust-lang#92016 (builtin_macros: allow external consumers for AsmArgs parsing) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Fix #87694.
Marked as draft to start with because I want to explore doing the same fix for
const const fn
and other repeated-but-valid keywords.@rustbot label A-diagnostics D-invalid-suggestion T-compiler