Skip to content

container.len() == 0 #32

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
lambda-fairy opened this issue Apr 16, 2015 · 10 comments · Fixed by #62
Closed

container.len() == 0 #32

lambda-fairy opened this issue Apr 16, 2015 · 10 comments · Fixed by #62
Assignees

Comments

@lambda-fairy
Copy link

For most types, this can be rewritten as container.is_empty().

cc rust-lang/rust#23682

@lambda-fairy
Copy link
Author

As a first approximation, we can warn when the self type is in collections.

If it's easy enough to check for an .is_empty() method, we can do that instead.

@llogiq
Copy link
Contributor

llogiq commented May 18, 2015

So, I now know how to get at the type (though I'm not sure what the exact contents would be), so how do we find out if the type has an is_empty() method?

@llogiq llogiq self-assigned this May 18, 2015
@Manishearth
Copy link
Member

I assume you go through the method map? Everything regarding type check and resolution is done through the tcx (type ctxt). Check out middle::ty for a ton of helper functions.

@llogiq
Copy link
Contributor

llogiq commented May 18, 2015

I know how to get the type definition (via expr_ty), but I'm a bit lost on how to go from there to the MethodMap. Do I construct a MethodCall via its expr and use that to query the MethodMap? Also what do I do with the MethodCallee once I have it?

@Manishearth
Copy link
Member

I'm not sure. Iterating through the tcx.impl_or_trait_items entry for the defid of the given type might work.

You'll have to explore middle::ty and ctxt to get this I guess :)

@llogiq
Copy link
Contributor

llogiq commented May 18, 2015

I've run into some wrestling with typeck, but at least I feel like I'm on the right track.

I'll see once the code compiles, anyway. 😄 However, it's getting late, so I'll resume tomorrow.

@llogiq
Copy link
Contributor

llogiq commented May 19, 2015

Now it compiles, but the type lookup fails for the simple case let x = [1, 2]; if x.len() == 0 { … }. I cannot figure out how to get from the DefLocal to a slice of its ImplOrTraitItemIds. Probably the whole approach is wrong.

@Manishearth
Copy link
Member

I've never really played with this so you're basically on your own here, sorry :) Poking people in #rust-internals may help

@llogiq
Copy link
Contributor

llogiq commented May 19, 2015

The problem is I'd either need to copy large swaths of typeck and patch rustc to allow me to access some private fields/methods, or just wait until the core team makes this available for lints.

The latter part opens up some of my time to try and implement some other things, so I guess I'll wait.

@llogiq
Copy link
Contributor

llogiq commented May 20, 2015

The lint in #60 basically encodes that every impl that has .len () should have .is_empty(), so we can get by without method lookup at the cost of possible false positives.

This is intended as a stop-gap solution, but may turn out to be good enough should we surmise that the cost of method lookup is too high.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants