-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Simple modification of non_lifetime_binders
's diagnostic information to adapt to type binders
#119154
Conversation
(rustbot has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I have one nit and one suggestion. After that, it's good to go. Lastly, could you update the PR title to something more descriptive? It's pretty general right now.
r? fmease
tests/ui/traits/non_lifetime_binders/non-lifetime-binders-issue-119067.rs
Outdated
Show resolved
Hide resolved
tests/ui/traits/non_lifetime_binders/non-lifetime-binders-issue-119067.rs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be better to split this test file or to move it somewhere else since it not only contains non_lifetime_binders
but also closure_lifetime_binder
(tests for the latter are usually placed in tests/ui/closures/binder
). Note that there already exist tests for for<T> Trait<T>
as well as for for<T> || -> ()
, maybe you can extend them instead of creating new files (you can add a comment above the new test cases in the form of // Regression test for issue #NNN
)?
Also the name of this test isn't super descriptive, maybe call it bounds-on-type-binders.rs
(for NLB) and bounds-on-closure-type-binders.rs
(for CLB)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you.
According to the suggestion, split it into two files.
non_lifetime_binders
's diagnostic information to adapt to type binders
Thank you very much. Modified according to the comments, please help review again. |
Thanks a lot! |
…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#118729 (Add release notes for 1.75.0) - rust-lang#119124 (don't build `rust-analyzer-proc-macro-srv` on def config ) - rust-lang#119154 (Simple modification of `non_lifetime_binders`'s diagnostic information to adapt to type binders) - rust-lang#119176 (Fix name error in aarch64_apple_watchos tier 3 target) - rust-lang#119182 (Update sysinfo version to 0.30.0) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#118729 (Add release notes for 1.75.0) - rust-lang#119124 (don't build `rust-analyzer-proc-macro-srv` on def config ) - rust-lang#119154 (Simple modification of `non_lifetime_binders`'s diagnostic information to adapt to type binders) - rust-lang#119176 (Fix name error in aarch64_apple_watchos tier 3 target) - rust-lang#119182 (Update sysinfo version to 0.30.0) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#119154 - surechen:fix_119067, r=fmease Simple modification of `non_lifetime_binders`'s diagnostic information to adapt to type binders fixes rust-lang#119067 Replace diagnostic information "lifetime bounds cannot be used in this context" to "bounds cannot be used in this context". ```rust #![allow(incomplete_features)] #![feature(non_lifetime_binders)] trait Trait {} trait Trait2 where for <T: Trait> ():{} //~^ ERROR bounds cannot be used in this context ```
fixes #119067
Replace diagnostic information "lifetime bounds cannot be used in this context" to "bounds cannot be used in this context".