More Clippy fixes #635
More Clippy fixes #635
Conversation
| @@ -703,7 +703,7 @@ impl<'t> YkPTBlockIterator<'t> { | |||
|
|
|||
| /// Fetch the next packet and update iterator state. | |||
| fn packet(&mut self) -> Result<Packet, HWTracerError> { | |||
| let ret = if let Some(pkt_or_err) = self.parser.next() { | |||
There was a problem hiding this comment.
This'll be an artifact of debugging. I wish there was a way to print the returned value without making a binding. Do you know a way?
| @@ -25,6 +24,7 @@ use ykrt::{HotThreshold, Location, MT}; | |||
| use yksmp::{Location as SMLocation, StackMapParser}; | |||
|
|
|||
| #[no_mangle] | |||
| #[allow(clippy::not_unsafe_ptr_arg_deref)] | |||
There was a problem hiding this comment.
Is this related to: rust-lang/rust-clippy#3045 ?
There was a problem hiding this comment.
Should we just let it warn? I worry we will forget to remove these annotations.
This would of course work against the idea of adding regular automated clippy checks...
There was a problem hiding this comment.
Notice this warning is only disabled for this one function.
There was a problem hiding this comment.
Yes, I know. Still...
Also we have many instances of this for other functions.
There was a problem hiding this comment.
The issue I linked explains that the clippy warning often triggers erroneously, as a false positive, right? As I understand, you are silencing a false positive here, right?
So what if a future code edit means that the clippy warning is actually justified, and that we've silenced a real concern?
There was a problem hiding this comment.
I don't understand what you're saying: you appear to be asking me to suppress the false positive now while allowing true positives in the future. How can I do that?
There was a problem hiding this comment.
I'm asking: should we not suppress the false positive. We don't have to fix every warning.
There was a problem hiding this comment.
What's the point of a warning that we then ignore? It's literally worse than suppressing it!
These are the last remaining Clippy fixes (well, at least until we introduce some more or Clippy becomes more stringent!).
ltratt
force-pushed
the
more_clippy_fixes
branch
from
19f7a70 to
b7575b5
Compare
|
Squashed. |
|
bors r+ |
|
Build failed: |
|
Our old friend again. Retry? |
|
We really need to fix that. bors r+ |
|
Build failed: |
|
Uh oh... again... Perhaps I should pause what I'm doing and priorities fixing this? |
|
bors r+ |
|
Build succeeded: |
The first of these required a manual change (5bc2998) and the rest are fairly mechanical (f35941d).