new large_enum_variants lint

[gnieto]

Added lint to detect large variants and suggesting to box them

Solves https://github.com/Manishearth/rust-clippy/issues/1127

[gnieto]

Used the same value as 'too-large-for-stack'. I don't know which value use as default

[mcarton]

That's already quite big for an enum variant IMO.

[gnieto]

So is the default value ok? Should I leave it to 200?

[mcarton]

r+ modulo the default value of enum-variant-size-threshold.

[mcarton]

Should be named LARGE_ENUM_VARIANT IMO.

[gnieto]

Changed all references to 'enum_large_variant' to 'large_enum_variant' on 088bb8f

[oli-obk]

needs a rebase + some research into enum variant sizes in existing code bases for choosing the default value.

[gnieto]

While I was doing some research on popular Rust projects with this lint, I discovered that under some conditions, this lint will panic. Consider the following code:

trait SomeTrait {
    type Item;
}

enum LargeEnumGeneric<A: SomeTrait> {
    Var(A::Item),
}

With that code, one of the calls to rustc is panicking with:

../src/librustc/ty/context.rs:161: Attempted to intern _ which contains inference types/regions in the global type context

Is obvious that the compiler is not able to know the size of 'SomeTrait::Item' because it's generic and it depends on which concrete trait implementations is given. In this scenario, the lint should ignore generic enums depending on associated types? Or the lint should search for any concretion of the enum (I don't know the technical name of this, I mean: LargeEnumGeneric), and then calculate the variant sizes of this?

[mcarton]

@gnieto Good catch! IMO it's ok to bail (cleanly, not ICEing 😄) when the size is unknown. You could manage to look at the enumeration instances, but it might be overkill.

Although, while testing this I noticed it looks like compiletest-rs does not catch ICE if all expected error are found before the ICE. This is bad, cc @Manishearth.

[Manishearth]

hmm, weird. IIRC compiletest does check the exit code.

[mcarton]

@Manishearth compiletest-rs, rustc

The actual error is

{all}
{other}
{errors}
{…}
{"message":"../src/librustc/ty/context.rs:161: Attempted to intern `_` which contains inference types/regions in the global type context","code":null,"level":"error: internal compiler error","spans":[],"children":[],"rendered":null}
note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports

note: run with `RUST_BACKTRACE=1` for a backtrace

It looks like that function was not adapted to the JSON format. I'll do a PR tomorrow. As for the error code, it is checked but rustc returns 101 for any kind of error (so it should work in run-pass, but not compile-fail).

[gnieto]

It has been a while since I opened the PR , and I tried a couple of ways to avoid the internal compiler error:

• Try to get a distinct "infer context" (I was using "borrowck_fake_infer_ctxt") to avoid the ICE (without success)
• Tried to inspect recursively the TypeVariants of each field of each variant in search of associated types (TyProjection). This can lead to a infinite loop if there are types depending on each other and... well, it seems like an ugly hack.

If anyone has some idea on how I can approach the issue, I'll try to do it, but at the moment, I don't know how to continue.

[oli-obk]

Tried to inspect recursively the TypeVariants of each field of each variant in search of associated types (TyProjection). This can lead to a infinite loop if there are types depending on each other and... well, it seems like an ugly hack.

I think that's the right approach for now. You can always use a HashSet to prevent recursing into types you've already seen.

[oli-obk]

@gnieto: are you planning to continue with this PR?

[gnieto]

@oli-obk I don't think that I will not have time to dedicate to this in the following weeks.
A part of that, I think that i picked a task that was too hard for me. Anyway, thanks for your help.

[oli-obk]

I just wanted to ask, to make sure I don't steal your PR that you still planned to continue. I'll finish it.