Warning directives aren't respected when in front of methods in impls #2647
Labels
A-lints
Area: Lints (warnings about flaws in source code) such as unused_mut.
Comments
ghost
assigned 
|
Oh, argh. This is because methods aren't items, and lint only grabs warning attributes from crates and items. Furthermore, the table holding the lint settings is keyed off of item ids... |
|
I don't believe this is backwards incompatible, renominating. |
bors
added a commit
that referenced
this issue
May 17, 2013
Closes #2647 This way it's much easier to add lints throughout compilation correctly, and functions on impls can alter the way lints are emitted. This involved pretty much rewriting how lints are emitted. Beforehand, only items could alter the lint settings, so whenever a lint was added it had to be associated with whatever item id it was coming from. I removed this (possibly questionably) in favor of just specifying a span and a message when adding a lint. When lint checking comes around, it looks at all the lints and sees which node with attributes best encloses it and uses that level of linting. This means that all consumer code doesn't have to deal with what item things came from (especially because functions on impls aren't items). More details of this can be found in the code (and comments). As a bonus, I managed to greatly simplify emission of lints in resolve.rs about unused imports. Now instead of it manually tracking what the lint level is, it's all moved over into the lint module (as is to be expected).
|
This works as of #6093 |
RalfJung
pushed a commit
to RalfJung/rust
that referenced
this issue
Nov 27, 2022
Track local frames incrementally during execution rust-lang/miri#2646 currently introduces a performance regression. This change removes that regression, and provides a minor perf improvement. The existing lazy strategy for tracking the span we want to display is as efficient as it is only because we often create a `CurrentSpan` then never call `.get()`. Most of the calls to the `before_memory_read` and `before_memory_write` hooks do not create any event that we store in `AllocHistory`. But data races are totally different, any memory read or write may race, so every call to those hooks needs to access to the current local span. So this changes to a strategy where we update some state in a `Thread` and `FrameExtra` incrementally, upon entering and existing each function call. Before: ``` Benchmark 1: cargo +miri miri run --manifest-path /home/ben/miri/bench-cargo-miri/backtraces/Cargo.toml Time (mean ± σ): 5.532 s ± 0.022 s [User: 5.444 s, System: 0.073 s] Range (min … max): 5.516 s … 5.569 s 5 runs Benchmark 1: cargo +miri miri run --manifest-path /home/ben/miri/bench-cargo-miri/mse/Cargo.toml Time (mean ± σ): 831.4 ms ± 3.0 ms [User: 783.8 ms, System: 46.7 ms] Range (min … max): 828.7 ms … 836.1 ms 5 runs Benchmark 1: cargo +miri miri run --manifest-path /home/ben/miri/bench-cargo-miri/serde1/Cargo.toml Time (mean ± σ): 1.975 s ± 0.021 s [User: 1.914 s, System: 0.059 s] Range (min … max): 1.939 s … 1.990 s 5 runs Benchmark 1: cargo +miri miri run --manifest-path /home/ben/miri/bench-cargo-miri/serde2/Cargo.toml Time (mean ± σ): 4.060 s ± 0.051 s [User: 3.983 s, System: 0.071 s] Range (min … max): 3.972 s … 4.100 s 5 runs Benchmark 1: cargo +miri miri run --manifest-path /home/ben/miri/bench-cargo-miri/slice-get-unchecked/Cargo.toml Time (mean ± σ): 784.9 ms ± 8.2 ms [User: 746.5 ms, System: 37.7 ms] Range (min … max): 772.9 ms … 793.3 ms 5 runs Benchmark 1: cargo +miri miri run --manifest-path /home/ben/miri/bench-cargo-miri/unicode/Cargo.toml Time (mean ± σ): 1.679 s ± 0.006 s [User: 1.623 s, System: 0.055 s] Range (min … max): 1.673 s … 1.687 s 5 runs ``` After: ``` Benchmark 1: cargo +miri miri run --manifest-path /home/ben/miri/bench-cargo-miri/backtraces/Cargo.toml Time (mean ± σ): 5.330 s ± 0.037 s [User: 5.232 s, System: 0.084 s] Range (min … max): 5.280 s … 5.383 s 5 runs Benchmark 1: cargo +miri miri run --manifest-path /home/ben/miri/bench-cargo-miri/mse/Cargo.toml Time (mean ± σ): 818.9 ms ± 3.7 ms [User: 776.8 ms, System: 41.3 ms] Range (min … max): 813.5 ms … 822.5 ms 5 runs Benchmark 1: cargo +miri miri run --manifest-path /home/ben/miri/bench-cargo-miri/serde1/Cargo.toml Time (mean ± σ): 1.927 s ± 0.011 s [User: 1.864 s, System: 0.061 s] Range (min … max): 1.917 s … 1.945 s 5 runs Benchmark 1: cargo +miri miri run --manifest-path /home/ben/miri/bench-cargo-miri/serde2/Cargo.toml Time (mean ± σ): 3.974 s ± 0.020 s [User: 3.893 s, System: 0.076 s] Range (min … max): 3.956 s … 4.004 s 5 runs Benchmark 1: cargo +miri miri run --manifest-path /home/ben/miri/bench-cargo-miri/slice-get-unchecked/Cargo.toml Time (mean ± σ): 780.0 ms ± 5.3 ms [User: 740.3 ms, System: 39.0 ms] Range (min … max): 771.2 ms … 784.5 ms 5 runs Benchmark 1: cargo +miri miri run --manifest-path /home/ben/miri/bench-cargo-miri/unicode/Cargo.toml Time (mean ± σ): 1.643 s ± 0.007 s [User: 1.584 s, System: 0.058 s] Range (min … max): 1.635 s … 1.654 s 5 runs ``` (This change is marginal, but the point is that it avoids a much more significant regression)
Aaron1011
pushed a commit
to Aaron1011/rust
that referenced
this issue
Jan 6, 2023
Track local frames incrementally during execution rust-lang/miri#2646 currently introduces a performance regression. This change removes that regression, and provides a minor perf improvement. The existing lazy strategy for tracking the span we want to display is as efficient as it is only because we often create a `CurrentSpan` then never call `.get()`. Most of the calls to the `before_memory_read` and `before_memory_write` hooks do not create any event that we store in `AllocHistory`. But data races are totally different, any memory read or write may race, so every call to those hooks needs to access to the current local span. So this changes to a strategy where we update some state in a `Thread` and `FrameExtra` incrementally, upon entering and existing each function call. Before: ``` Benchmark 1: cargo +miri miri run --manifest-path /home/ben/miri/bench-cargo-miri/backtraces/Cargo.toml Time (mean ± σ): 5.532 s ± 0.022 s [User: 5.444 s, System: 0.073 s] Range (min … max): 5.516 s … 5.569 s 5 runs Benchmark 1: cargo +miri miri run --manifest-path /home/ben/miri/bench-cargo-miri/mse/Cargo.toml Time (mean ± σ): 831.4 ms ± 3.0 ms [User: 783.8 ms, System: 46.7 ms] Range (min … max): 828.7 ms … 836.1 ms 5 runs Benchmark 1: cargo +miri miri run --manifest-path /home/ben/miri/bench-cargo-miri/serde1/Cargo.toml Time (mean ± σ): 1.975 s ± 0.021 s [User: 1.914 s, System: 0.059 s] Range (min … max): 1.939 s … 1.990 s 5 runs Benchmark 1: cargo +miri miri run --manifest-path /home/ben/miri/bench-cargo-miri/serde2/Cargo.toml Time (mean ± σ): 4.060 s ± 0.051 s [User: 3.983 s, System: 0.071 s] Range (min … max): 3.972 s … 4.100 s 5 runs Benchmark 1: cargo +miri miri run --manifest-path /home/ben/miri/bench-cargo-miri/slice-get-unchecked/Cargo.toml Time (mean ± σ): 784.9 ms ± 8.2 ms [User: 746.5 ms, System: 37.7 ms] Range (min … max): 772.9 ms … 793.3 ms 5 runs Benchmark 1: cargo +miri miri run --manifest-path /home/ben/miri/bench-cargo-miri/unicode/Cargo.toml Time (mean ± σ): 1.679 s ± 0.006 s [User: 1.623 s, System: 0.055 s] Range (min … max): 1.673 s … 1.687 s 5 runs ``` After: ``` Benchmark 1: cargo +miri miri run --manifest-path /home/ben/miri/bench-cargo-miri/backtraces/Cargo.toml Time (mean ± σ): 5.330 s ± 0.037 s [User: 5.232 s, System: 0.084 s] Range (min … max): 5.280 s … 5.383 s 5 runs Benchmark 1: cargo +miri miri run --manifest-path /home/ben/miri/bench-cargo-miri/mse/Cargo.toml Time (mean ± σ): 818.9 ms ± 3.7 ms [User: 776.8 ms, System: 41.3 ms] Range (min … max): 813.5 ms … 822.5 ms 5 runs Benchmark 1: cargo +miri miri run --manifest-path /home/ben/miri/bench-cargo-miri/serde1/Cargo.toml Time (mean ± σ): 1.927 s ± 0.011 s [User: 1.864 s, System: 0.061 s] Range (min … max): 1.917 s … 1.945 s 5 runs Benchmark 1: cargo +miri miri run --manifest-path /home/ben/miri/bench-cargo-miri/serde2/Cargo.toml Time (mean ± σ): 3.974 s ± 0.020 s [User: 3.893 s, System: 0.076 s] Range (min … max): 3.956 s … 4.004 s 5 runs Benchmark 1: cargo +miri miri run --manifest-path /home/ben/miri/bench-cargo-miri/slice-get-unchecked/Cargo.toml Time (mean ± σ): 780.0 ms ± 5.3 ms [User: 740.3 ms, System: 39.0 ms] Range (min … max): 771.2 ms … 784.5 ms 5 runs Benchmark 1: cargo +miri miri run --manifest-path /home/ben/miri/bench-cargo-miri/unicode/Cargo.toml Time (mean ± σ): 1.643 s ± 0.007 s [User: 1.584 s, System: 0.058 s] Range (min … max): 1.635 s … 1.654 s 5 runs ``` (This change is marginal, but the point is that it avoids a much more significant regression)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The following code shouldn't have any warnings, but does:
The text was updated successfully, but these errors were encountered: