librustc: adjust logic for cfg attribute and add not predicate. #5410
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Looks good. Is there a way to test this? |
|
Tests would be good here, yeah. Also -- this is weird to mention -- but reading it now and looking at the variants, I think maybe I'd expect the comma-list to be OR-ed and multiple juxtaposed as meaning "(linux OR osx) AND debug", rather than "(linux AND osx) OR debug" as I think this patch implements. But I'm not sure it matters, so long as there's some canonical reading. Do you have a strong feeling, yourself? |
|
I don't have a preference one way or the other. The only problem though is that the current behaviour is multiple cfg's are OR-ed so changing it now would be annoying to say the least. For your example maybe there should be something like: #[cfg(debug, or(linux, osx))]
fn foo() { ... } |
|
Ok. Commented in the original bug (which I will ask to remain open for now) concerning long-term fixes to this, but the patch as provided is acceptable. Can you add some tests? |
|
Obviously, I am amazing at naming :) |
Make comment describe actual behaviour.
bors
added a commit
that referenced
this pull request
Mar 20, 2013
This adopts the syntax from #2119. No more annoying workarounds involving wrapping in mods!
oli-obk
pushed a commit
to oli-obk/rust
that referenced
this pull request
May 2, 2020
Downgrade trivially_copy_pass_by_ref to pedantic
The rationale for this lint is documented as:
> In many calling conventions instances of structs will be passed through registers if they fit into two or less general purpose registers.
I think the purported performance benefits of clippy's recommendation are overstated. This isn't worth asking people to sprinkle code with more `*``*``&``*``&` to chase the alleged performance.
This should be a pedantic lint that is disabled by default and opted in if some specific performance sensitive codebase determines that it is worthwhile.
As a reminder, a typical place that a reference to a primitive would come up is if the function is used as a filter. Triggering a performance-oriented lint on this type of code is the definition of pedantic.
```rust
fn filter(_n: &i32) -> bool {
true
}
fn main() {
let v = vec![1, 2, 3];
v.iter().copied().filter(filter).for_each(drop);
}
```
```console
warning: this argument (4 byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte)
--> src/main.rs:1:15
|
1 | fn filter(_n: &i32) -> bool {
| ^^^^ help: consider passing by value instead: `i32`
```
changelog: Remove trivially_copy_pass_by_ref from default set of enabled lints
oli-obk
pushed a commit
to oli-obk/rust
that referenced
this pull request
May 2, 2020
Rollup of 11 pull requests Successful merges: - rust-lang#5406 (Fix update_lints) - rust-lang#5409 (Downgrade let_unit_value to pedantic) - rust-lang#5410 (Downgrade trivially_copy_pass_by_ref to pedantic) - rust-lang#5412 (Downgrade inefficient_to_string to pedantic) - rust-lang#5415 (Add new lint for `Result<T, E>.map_or(None, Some(T))`) - rust-lang#5417 (Update doc links and mentioned names in docs) - rust-lang#5419 (Downgrade unreadable_literal to pedantic) - rust-lang#5420 (Downgrade new_ret_no_self to pedantic) - rust-lang#5422 (CONTRIBUTING.md: fix broken triage link) - rust-lang#5424 (Incorrect suspicious_op_assign_impl) - rust-lang#5425 (Ehance opt_as_ref_deref lint.) Failed merges: - rust-lang#5345 (Add lint for float in array comparison) - rust-lang#5411 (Downgrade implicit_hasher to pedantic) - rust-lang#5428 (Move cognitive_complexity to nursery) r? @ghost changelog: rollup
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adopts the syntax from #2119. No more annoying workarounds involving wrapping in mods!