WIP: rustup by oli-obk · Pull Request #1373 · rust-lang/rust-clippy

oli-obk · 2016-12-01T21:33:43Z

going to bed. Didn't get this fixed all the way. I think the vec! macro changed, so higher::vec_macro is borked (but not in the part that I changed I think)

mcarton · 2016-12-01T21:34:24Z

@oli-obk did you find the responsible PR from rustc? That usually really helps.

oli-obk · 2016-12-01T21:36:55Z

rust-lang/rust#37676 that was one of the changes, but there are multiple things going on at once I think

oli-obk · 2016-12-02T16:25:02Z

@jseyfried : Why is the qpath.def for use std::cmp::Ordering::*; (extracted through ItemUse(ref qpath, UseKind::Glob)) a Mod and not an Enum? (asking here again, since I'm going offline in irc now)

oli-obk · 2016-12-02T16:40:30Z

hashes of paths are wrong... for copies.rs

not sure how to fix enum_glob_use

mcarton · 2016-12-02T16:49:34Z

hashes of paths are wrong... for copies.rs

As long as equality works and a == b ⇒ hash(a) == hash(b) it's fine. You don't need perfect hashes.

oli-obk · 2016-12-02T18:30:55Z

ok... we're down to equal for loops not comparing equal

oli-obk · 2016-12-02T18:43:41Z

If someone could pick this up, that would be great. I'm not gonna be near a pc until sunday.

All that is left is to dump the failing test (copy just the failing expression to a new file and dump the ast) and figure out what part of a desugared for loop fails to compare

It is slow but it can be useful and can be set/unset explicitly before running tests if needed. There is no backtrace by default anyway.

mcarton · 2016-12-02T19:50:12Z

-    }
+    fn eq_qpath(&self, left: &QPath, lid: NodeId, right: &QPath, rid: NodeId) -> bool {
+        let l = self.cx.tcx.tables().qpath_def(left, lid);
+        let r = self.cx.tcx.tables().qpath_def(right, rid);


Now foo::<u8> and foo::<u32> compare equal, this is wrong.

mcarton · 2016-12-02T19:51:37Z

                self.eq_expr(lc, rc) && self.eq_block(lt, rt) && both(le, re, |l, r| self.eq_expr(l, r))
            }
            (&ExprLit(ref l), &ExprLit(ref r)) => l.node == r.node,
-            (&ExprLoop(ref lb, ref ll, ref lls), &ExprLoop(ref rb, ref rl, ref rls)) => {


Why did you remove the loop source ({l,r}ls)? It's just a variant-less Copy enum.

mcarton · 2016-12-02T19:51:47Z

            }
-            (&ExprMatch(ref le, ref la, ref ls), &ExprMatch(ref re, ref ra, ref rs)) => {
-                ls == rs && self.eq_expr(le, re) &&
+            (&ExprMatch(ref le, ref la, _), &ExprMatch(ref re, ref ra, _)) => {


mcarton · 2016-12-02T19:52:07Z

                if let Some(i) = *i {
                    self.hash_name(&i.node);
                }
-                j.hash(&mut self.s);


Because they don't really add any value... If a desugaring is the same as a manual write then it is the same

This reverts commit cf1fa94.

mcarton · 2016-12-02T21:06:56Z

 use std::hash::{Hash, Hasher};
 use std::collections::hash_map::DefaultHasher;
-use syntax::ast::Name;
+use syntax::ast::{Name, NodeId};


If you're ever going to use NodeIds or Spans in that file, you're doing something wrong 😛

mcarton · 2016-12-02T21:39:43Z

This works locally on rustc 1.15.0-nightly (1c448574b 2016-11-28) now but it looks like it need more updates for rustc 1.15.0-nightly (908dba0c9 2016-12-01) 😓

mcarton · 2016-12-02T21:41:57Z

+            // FIXME: ask jseyfried why the qpath.def for `use std::cmp::Ordering::*;`
+            // extracted through `ItemUse(ref qpath, UseKind::Glob)` is a `Mod` and not an `Enum`
+            //if let Def::Enum(_) = path.def {
+            if path.segments.last().and_then(|seg| seg.name.as_str().chars().next()).map_or(false, char::is_uppercase) {


Did you had an answer for that?

mcarton · 2016-12-02T21:43:25Z

+            }
+        },
+        None => false,
+    })


Why did you have to change this? IMO we don't have enough in_macro tests.

Because without this, we can't do anything useful with ranges in hir, since they all have that expansion span

mcarton · 2016-12-02T21:48:04Z


 fn main(){
-    test(LinkedList::new());
+    test(LinkedList::new()); //~ ERROR I see you're using a LinkedList!


I thought this wasn't here on purpose: if test takes a LinkedList, the user has no choice but to build one to call the function. Hence the function should be warned about, not its call.

Makes sense, I'm surprised it ever worked...

mcarton · 2016-12-02T22:25:38Z

The new update for rustc 1.15.0-nightly (908dba0c9 2016-12-01) is quite big too, I will finish that tomorrow.

jseyfried · 2016-12-02T22:37:52Z

@oli-obk

Why is the qpath.def for use std::cmp::Ordering::*; a Mod and not an Enum?

Because this code is incorrect -- it should record module.def().unwrap(), not Def::Mod(module.def_id().unwrap()).
This is a very minor change -- I'll fix in my next PR (probably tomorrow).

oli-obk · 2016-12-04T20:29:17Z

The new update for rustc 1.15.0-nightly (908dba0 2016-12-01) is quite big too, I will finish that tomorrow.

we need to patch rustc first. rust-lang/rust#37918 broke all lints that forward to an inner Visitor, since LateContext's methods do not have lifetime annotations binding the given ast/hir node's lifetime to the TyCtxt's. A hacky workaround in clippy would be to do an intermediate call to tcx.map.expect_* which again yields a tcx lifetime, but I think we'd save us many headaches by simply fixing rustc.

oli-obk · 2016-12-05T16:46:59Z

I have a rustc pr ready but didn't get clippy ported all the way. Will finish tomorrow

oli-obk · 2016-12-06T10:31:57Z

got it to work with rust-lang/rust#38191

oli-obk · 2016-12-09T14:49:18Z

rust PR has been merged, now we wait for a new nightly to survive the buildbots: http://rusty-dash.com/nightlies

Arnavion · 2016-12-11T07:39:44Z

rust-lang/rust#38097 broke this a little more. rustc::ty::FnSig no longer has fields called inputs and output

Edit: Fixed in https://github.com/Arnavion/rust-clippy/commit/3de6f9db202c612aad15397fb5b3320497dc2f25

…-existent fields.

mcarton · 2016-12-11T11:31:40Z

Thanks @Arnavion!
Now we just have to wait for a new nightly to be published, but it's been a few days since there hasn't been any.

Wabuo · 2016-12-16T15:56:39Z

There is a nightly release now ...

oli-obk · 2016-12-16T16:27:39Z

Travis doesn't have it yet

mcarton · 2016-12-17T00:35:40Z

🎉 Published 🎉

WIP compiles and doesn't crash (much) but tests are failing

59b0077

enum glob use and copies left

16aab71

oli-obk added 3 commits December 2, 2016 19:19

down to for-loops not comparing as equal

cf1fa94

permit more equalities

ff2487b

enum detection by style convention

8e1f6dc

Don't set RUST_BACKTRACE for tests

42fb11f

It is slow but it can be useful and can be set/unset explicitly before running tests if needed. There is no backtrace by default anyway.

mcarton suggested changes Dec 2, 2016

View reviewed changes

Revert "down to for-loops not comparing as equal"

fe15880

This reverts commit cf1fa94.

mcarton reviewed Dec 2, 2016

View reviewed changes

mcarton added 2 commits December 2, 2016 22:23

Fix copy lints

65ff2df

Fix dogfood

1ba8ed9

mcarton suggested changes Dec 2, 2016

View reviewed changes

uncomment code that was commented out for testing

2e63a56

oli-obk changed the title ~~DO NOT MERGE: compiles and doesn't crash (much) but tests are failing~~ WIP: rustup Dec 4, 2016

oli-obk added 3 commits December 6, 2016 11:32

rustup to rustc 1.15.0-dev (3b248a184 2016-12-05)

c6e2967

remove wrong rustc version so we don't accidentally forget to update

4a05fbb

update to the rust-PR that unblocks clippy

778ce4d

Use inputs/output getters for rustc::ty::FnSig instead of the now non…

3de6f9d

…-existent fields.

mcarton mentioned this pull request Dec 11, 2016

clippy fails compile on nightly -- QSelf undefined #1381

Closed

oli-obk mentioned this pull request Dec 12, 2016

Fails to compile on latest Nightly #1382

Closed

Bump to 0.0.104

4fcefe2

mcarton merged commit ea0227f into master Dec 17, 2016

mcarton deleted the rustup branch December 17, 2016 00:35

ordovicia mentioned this pull request Mar 5, 2018

Don't lint range syntax with var name start and/or end #2507

Merged

Conversation

oli-obk commented Dec 1, 2016

Uh oh!

mcarton commented Dec 1, 2016

Uh oh!

oli-obk commented Dec 1, 2016

Uh oh!

oli-obk commented Dec 2, 2016

Uh oh!

oli-obk commented Dec 2, 2016

Uh oh!

mcarton commented Dec 2, 2016

Uh oh!

oli-obk commented Dec 2, 2016

Uh oh!

oli-obk commented Dec 2, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcarton commented Dec 2, 2016

Uh oh!

mcarton Dec 2, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcarton commented Dec 2, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jseyfried commented Dec 2, 2016

Uh oh!

oli-obk commented Dec 4, 2016

Uh oh!

oli-obk commented Dec 5, 2016

Uh oh!

oli-obk commented Dec 6, 2016

Uh oh!

oli-obk commented Dec 9, 2016

Uh oh!

Arnavion commented Dec 11, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mcarton commented Dec 11, 2016

Uh oh!

Wabuo commented Dec 16, 2016

Uh oh!

oli-obk commented Dec 16, 2016

Uh oh!

mcarton commented Dec 17, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mcarton Dec 2, 2016 •

edited

Loading

mcarton commented Dec 2, 2016 •

edited

Loading

Arnavion commented Dec 11, 2016 •

edited

Loading