(Preliminary) Results of building rust with clippy

[llogiq]

I was able to check the libfmt_macros crate with clippy and found one str_to_string, one while_let_loop and a few unneeded_return_statements.

Checking the libsyntax crate failed with the following stack trace:

thread '<unnamed>' panicked at 'index out of bounds: the len is 60 but the index is 190', src/libcollections/vec.rs:1047
stack backtrace:
   1:     0x7f21e6fc67c9 - sys::backtrace::tracing::imp::write::h9d995b4e2eebc457Mns
   2:     0x7f21e6fce74c - panicking::on_panic::h0af95cd615077cb32ox
   3:     0x7f21e6f925de - rt::unwind::begin_unwind_inner::ha19c9fa5f9b03821GRw
   4:     0x7f21e6f93327 - rt::unwind::begin_unwind_fmt::hff85cd541f1ded16MQw
   5:     0x7f21e6fce0e1 - rust_begin_unwind
   6:     0x7f21e701fbbf - panicking::panic_fmt::hd2a7f20260b38c91J7E
   7:     0x7f21e7019ed2 - panicking::panic_bounds_check::h270e000900cee620P6E
   8:     0x7f21e7531999 - parse::token::InternedString::new_from_name::h133415336b0a3dbbomT
   9:     0x7f21e88547ad - ast::Name.PartialEq<T>::eq::h8722861357916141903
  10:     0x7f21e886f3a3 - len_zero::LenZero.LintPass::check_item::h98dc7c5e93c3cd56WYc
  11:     0x7f21f47be707 - lint::context::Context<'a, 'tcx>::with_lint_attrs::h15905335786845705409
  12:     0x7f21f47c3a1e - lint::context::Context<'a, 'tcx>.Visitor<'v>::visit_mod::hbe9f346fd6f42bffkaw
  13:     0x7f21f47bedb4 - lint::context::Context<'a, 'tcx>::with_lint_attrs::h15905335786845705409
  14:     0x7f21f47c3a1e - lint::context::Context<'a, 'tcx>.Visitor<'v>::visit_mod::hbe9f346fd6f42bffkaw
  15:     0x7f21f47bedb4 - lint::context::Context<'a, 'tcx>::with_lint_attrs::h15905335786845705409
  16:     0x7f21f47c3a1e - lint::context::Context<'a, 'tcx>.Visitor<'v>::visit_mod::hbe9f346fd6f42bffkaw
  17:     0x7f21f47d0f09 - lint::context::check_crate::hf80c5adb00775b2ejuw
  18:     0x7f21f403a299 - driver::phase_3_run_analysis_passes::closure.20670
  19:     0x7f21f4034d8e - middle::ty::ctxt<'tcx>::create_and_enter::h18281789251420907216
  20:     0x7f21f402fd81 - driver::phase_3_run_analysis_passes::h17232049417500553259
  21:     0x7f21f4014440 - driver::compile_input::h37859d56374280caTba
  22:     0x7f21f40f4946 - run_compiler::h86a39f47860f70c7C7b
  23:     0x7f21f40f26c9 - boxed::F.FnBox<A>::call_box::h15702049526784006720
  24:     0x7f21f40f20a4 - rt::unwind::try::try_fn::h2747471524830943407
  25:     0x7f21f5b6e308 - __rust_try
  26:     0x7f21f5b60bc2 - rt::unwind::try::inner_try::ha3b130acdaf96544aWw
  27:     0x7f21f40f2248 - boxed::F.FnBox<A>::call_box::h2060115051421633638
  28:     0x7f21f5b6d6f3 - sys::thread::Thread::new::thread_start::h3819a1565b813297A5v
  29:     0x7f21f3a4a6a9 - start_thread
  30:     0x7f21f2e55eec - clone
  31:                0x0 - <unknown>

This may be due to a mismatch between the stage0 compiler and the compiler with which I built clippy, but I'm not sure.

Trying to check librustc with clippy causes a Segmentation fault on my system.

I'll try to check other rustc/std crates with clippy.

[Manishearth]

This probably is a mismatch; the method doesn't look panicky.

Compile rustc-stage1, compile clippy with rustc-stage1 (use multirust), and then do RUSTFLAGS_STAGE2=-Z .... make rustc-stage2. Or something like that.

[llogiq]

Yeah, probably. I'm in the process of installing multirust. I'll report back once I have more results.

[llogiq]

More results – it appears I've run into the same problem as Manish on servo (also some false positives that should be removed in the meantime). I'll update and run again.

Btw. if someone intends to replicate this, one first needs to make stage1, then use multirust run stage1 cargo build --release on clippy (I've linked a nightly cargo into my stage1 toolchain), then run VERBOSE=1 RUSTFLAGS_STAGE2='-L ~/projects/rust-clippy/target/release/ -Z extra-plugins=clippy' RUST_BACKTRACE=1 make rustc-stage2.

One thing I found very interesting:

src/libcore/hash/sip.rs:67:26: 152:47 error: the operation is ineffective. Consider reducing it to `t`, #[deny(identity_op)] on by default
src/libcore/hash/sip.rs:67             out |= ($buf[t+$i] as u64) << t*8;
src/libcore/hash/sip.rs:68             t += 1;
src/libcore/hash/sip.rs:69         }
src/libcore/hash/sip.rs:70         out
src/libcore/hash/sip.rs:71     });
src/libcore/hash/sip.rs:72 }
                           ...
src/libcore/hash/sip.rs:52:1: 72:2 note: in expansion of u8to64_le!
src/libcore/hash/sip.rs:152:30: 152:56 note: expansion site
src/libcore/hash/sip.rs:67:26: 152:47 help: for further information visit https://github.com/Manishearth/rust-clippy/wiki#identity_op

Isn't that a false positive here?

[Manishearth]

Isn't that a false positive here?

Yes. This is probably because we don't consider casts in our framework. But perhaps we should ignore this after verifying that casts indeed are the reason.

[Manishearth]

It might be possible to make use of Clippy's structured logging feature to output the lints as JSON and autofix them. For example, knowing that there's an elision issue on a range of line numbers, we may fix it via python or something.

[Manishearth]

rust-lang/rust@master...Manishearth:elide-core

[Manishearth]

My lifetime-applying script is:

while read in; do
 IFS=: read file line <<< "$in";

#echo $file;
#echo $line;
(sed -n -e "${line}p" $file | grep "Pattern" >/dev/null) || (
sed  -i "${line}s/\&'a /\&/g" $file;
sed  -i "${line}s/\&'a/\&/g" $file;
sed  -i "${line}s/'a, //g" $file;
sed  -i "${line}s/'a,//g" $file;
sed  -i "${line}s/<'a>//g" $file;
(sed -n -e "${line}p" $file | grep "'a" >/dev/null)  && (echo $file; echo $line);
);
done <lifetimesln

where lifetimesln was created from cat run | grep -A 1 "#\[deny(needless_lifetimes)\] on by default" | grep -v deny | grep -v "\-\-" | awk '{print $1}' (and run is just the above logs),

Far from perfect, but it handles most cases, and prints out whatever it can't handle.

[llogiq]

What about lifetimes with other names than 'a?

[Manishearth]

/me was lazy

[llogiq]

😄

while read in; do
 IFS=: read file line <<< "$in";

#echo $file;
#echo $line;
(sed -n -e "${line}p" $file | grep "Pattern" >/dev/null) || (
sed  -i "${line}s/\&'\\w+ /\&/g" $file;
sed  -i "${line}s/\&'\\w+/\&/g" $file;
sed  -i "${line}s/'\\w+, //g" $file;
sed  -i "${line}s/'\\w+,//g" $file;
sed  -i "${line}s/<'\\w+>//g" $file;
(sed -n -e "${line}p" $file | grep "'\\w+[^\\w']" >/dev/null)  && (echo $file; echo $line);
);
done <lifetimesln

Might work, but i think there might be a clash with char literals. For example let a = &'a'. Be careful.

[Manishearth]

Ideally, we should add JSON logging of specific lints. Eg the lifetime elision will log fileline info (preferably notify if the FnDecl is multiline, but that's hard), and the elidable lifetimes.

[llogiq]

I'm also unsure if this could constitute a false positive:

src/libcore/slice.rs:971:31: 971:42 help: for further information visit https://github.com/Manishearth/rust-clippy/wiki#mut_mut
src/libcore/slice.rs:991:40: 991:51 error: this expression mutably borrows a mutable reference. Consider reborrowing, #[deny(mut_mut)] on by default
src/libcore/slice.rs:991                 let tmp = mem::replace(&mut self.v, &mut []);
                                                                ^~~~~~~~~~~

But this surely does:

src/libcore/str/mod.rs:94:29: 94:38 help: for further information visit https://github.com/Manishearth/rust-clippy/wiki#unit_cmp
src/libcore/str/mod.rs:94:29: 94:38 error: !=-comparison of unit values detected. This will always be false, #[deny(unit_cmp)] on by default
src/libcore/str/mod.rs:94 pub struct ParseBoolError { _priv: () }
                                                      ^~~~~~~~~

(it appears unit_cmp needs an in_macro check)

By the way, I revisited the identity_op false positive – it's actually not really a false positive. $i will be 0 during macro execution, and the implementation relies on LLVM optimizing the + 0 away. I think we should broaden our macro check to all macro expansions (not only external ones) but the usual for-, if/while let- and other language-level expansions to get rid of warnings due to this pattern.

[llogiq]

I've tried to replicate my findings with a current Rust, but failed because I couldn't compile clippy with a stage1 toolchain, because it didn't find the syntax crate. I'll try and rebuild the toolchain.

[Manishearth]

You need to first make check-stage1-rpass-full to get the stage1 libs to build (terminate it when it starts running tests). By default make rustc-stage1 does not build librustc and libsyntax IIRC>

[camsteffen]

It seems like everything has changed since this was opened 😄