GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — librustc: adjust logic for cfg attribute and add not predicate. by luqmana · Pull Request #5410 · rust-lang/rust

Conversation

@luqmana

@luqmana luqmana commented Mar 16, 2013

Copy link
Copy Markdown
Contributor

This adopts the syntax from #2119. No more annoying workarounds involving wrapping in mods!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@sanxiyn

sanxiyn commented Mar 17, 2013

Copy link
Copy Markdown
Contributor

Looks good. Is there a way to test this?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@graydon

graydon commented Mar 19, 2013

Copy link
Copy Markdown
Contributor

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 #[cfg(...)] attributes to be AND-ed. That is, I think my eyes scan

#[cfg(linux,osx)]
#[cfg(debug)]
fn foo() { ... }

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?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@luqmana

luqmana commented Mar 19, 2013

Copy link
Copy Markdown
Contributor Author

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() { ... }
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@graydon graydon mentioned this pull request Mar 19, 2013
cfg() and multiple predicates, logic language, etc #2119
Closed
@graydon

graydon commented Mar 19, 2013

Copy link
Copy Markdown
Contributor

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?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

luqmana added 2 commits March 19, 2013 13:00
@luqmana
librustc: adjust logic for cfg attribute and add not predicate.
429fed0
@luqmana
Tests for cfg attribute.
1e16edd
@luqmana

luqmana commented Mar 19, 2013

Copy link
Copy Markdown
Contributor Author

Obviously, I am amazing at naming :)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@luqmana
Update run-pass/cfg-on-items.rs
b9fc773
Make comment describe actual behaviour.
luqmana added 3 commits March 19, 2013 16:26
@luqmana
Fix pretty printing for cfg test.
6450ee9
@luqmana
compiletest: Make pretty tests actually use compile-flags.
480e9ff
@luqmana
Mark cfg test xfail-fast since check-fast doesn't understand compile-…
811d880
…flags.
bors added a commit that referenced this pull request Mar 20, 2013
@bors
auto merge of #5410 : luqmana/rust/cfg-and, r=graydon
4cb9ca9
This adopts the syntax from #2119. No more annoying workarounds involving wrapping in mods!
@bors bors closed this Mar 20, 2013
@bors
bors merged commit 811d880 into rust-lang:incoming Mar 20, 2013
oli-obk pushed a commit to oli-obk/rust that referenced this pull request May 2, 2020
@flip1995
Rollup merge of rust-lang#5410 - dtolnay:trivially, r=flip1995
1e1bd51
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
@bors
Auto merge of rust-lang#5438 - flip1995:rollup-pi762oy, r=flip1995
f8308c8
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

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@luqmana @sanxiyn @graydon @bors
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.