Add diagnostic item for std::iter::Iterator::enumerate
#124542
Conversation
rustbot
added
S-waiting-on-review
| @@ -974,6 +974,7 @@ pub trait Iterator { | |||
| #[inline] | |||
| #[stable(feature = "rust1", since = "1.0.0")] | |||
| #[rustc_do_not_const_check] | |||
| #[cfg_attr(not(test), rustc_diagnostic_item = "enumerate_method")] | |||
There was a problem hiding this comment.
Maybe a silly question, but why not(test)?
A quick rg rustc_diagnostic_item library shows lots with, but also lots without, and I don't understand why.
There was a problem hiding this comment.
The Rust Compiler Development Guide explains, not including not(test) can cause compilation errors while running some tests. It goes on to say it's okay to add it as a preventative measure for all diagnostic items.
I did not test whether this specific diagnostic item causes any testing compilation errors. I will look into this further to determine if it does.
There was a problem hiding this comment.
I ran ./x test, which I believe runs all tests given my current configuration. There were no failures with not(test) removed.
There was a problem hiding this comment.
The cfg(not(test)) is required in alloc and std, but not in core.
This is because alloc and std contain some unit #[test]s as part of the library crates and the crates therefore have to be compiled twice: Once as a library as a dependency of the test crate (which contains the default test runner) and once as an executable that imports the test crate.
When the crates are compiled twice and the diagnostic items don't have cfg(not(test)), then we get an error that the diagnostic items are defined multiple times. (The same also happens for lang items.)
The core package explicitly doesn't test its library crate and only has integration tests, so the cfg(not(test)) is not required there.
Lines 14 to 16 in 20aa2d8
There was a problem hiding this comment.
Since cfg(not(test)) is not required for this diagnostic item, should I remove it? Or is it okay to leave it on as a preventative measure?
|
Thanks for teaching me about this! Since this is far from the only place that would be doing this in core, I think it's fine to accept as-is and if people want to change them all later for consistency that can be a different PR. @bors r+ rollup=always |
bors
added
S-waiting-on-bors
…method, r=scottmcm Add diagnostic item for `std::iter::Iterator::enumerate` Adds a diagnostic item for the `std::iter:Iterator::enumerate` trait method. This change, along with PR rust-lang#124308, will be used by the clippy `unused_enumerate_index` lint to move away from paths to using diagnostic items. see: rust-lang/rust-clippy#5393
…iaskrgr Rollup of 3 pull requests Successful merges: - rust-lang#124529 (Rewrite select (in the new solver) to use a `ProofTreeVisitor`) - rust-lang#124542 (Add diagnostic item for `std::iter::Iterator::enumerate`) - rust-lang#124566 (fix `NormalizesTo` proof tree issue) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 2 pull requests Successful merges: - rust-lang#124542 (Add diagnostic item for `std::iter::Iterator::enumerate`) - rust-lang#124566 (fix `NormalizesTo` proof tree issue) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#124542 - CBSpeir:diagnostic-item-enumerate-method, r=scottmcm Add diagnostic item for `std::iter::Iterator::enumerate` Adds a diagnostic item for the `std::iter:Iterator::enumerate` trait method. This change, along with PR rust-lang#124308, will be used by the clippy `unused_enumerate_index` lint to move away from paths to using diagnostic items. see: rust-lang/rust-clippy#5393
Adds a diagnostic item for the
std::iter:Iterator::enumeratetrait method. This change, along with PR #124308, will be used by the clippyunused_enumerate_indexlint to move away from paths to using diagnostic items.see: rust-lang/rust-clippy#5393