Object safety check fixes

[bkoropoff]

• Always consider static methods object-safe since they can never be called through a trait object anyway. The RFC doesn't mention static methods at all, so I'm assuming this is reasonable behavior.
• Fix issue ICE: 'rustc' panicked at 'assertion failed: *start <= *end' in slice.rs:396 #18490 as a side-effect since we avoid trying to take an out-of-bounds slice later in the code

Closes #18490

r? @nick29581

[aturon]

Part of the motivation for the object safety change was that if Trait is object safe then Box<Trait>: Trait. But if Trait has static methods, then there's no way to automatically implement it for Box<Trait>, sadly.

(That said, static methods weren't discussed when making the decision, and seem to be a significant downside to this change, so we may need to reconsider the whole thing.)

[bkoropoff]

That's a good point. The current behavior permits static methods as long as they follow the other rules (e.g. not referring to Self), so I erred on the side of permissivity. Should I tweak the patch to just avoid the panic and maintain the status quo otherwise?

[nrc]

@aturon to be precise with the static method issue, is this the situation you worry about:

trait T {
    fn foo();
}

fn bar<X: T>(x: Box<T>) {
    X::foo();
}

fn main() {
    let y: Box<T> = ...;
    bar(y);
}

Where the monomorphised version of X::foo() for X as T can't have an impl?

[aturon]

@nick29581 Yes, that's one example.

More generally, there have been a lot of complaints about the new object safety rules not allowing you to have constructors-like methods (that produce Self) even though these are, in some sense, perfectly reasonable for trait objects. After all, you still construct the initial object with a concrete type before boxing it up, and after that point all actual methods may be perfectly usable with trait objects. Having to split out the constructor(s) into a separate trait seems rather unfortunate.

Unlike other examples we've seen, where splitting the trait made things more flexible, this seems to unequivocally make the API worse.

Some real-life examples where the splitting up is having to occur:

https://github.com/hyperium/hyper/blob/master/src/header/mod.rs#L26-L43
https://github.com/hyperium/hyper/blob/master/src/net.rs#L44-L61

[nrc]

@bkoropoff Yeah, I think taking a patch like this is the thing to do for now - given that we don't use object safety for the auto-impl stuff, this seems like the right fix (and brings us as close as possible to the old situation of per-method object safety).

We do need to think about object safety in general though, it seems like all static methods object-unsafe once we do the auto-impls.

[nrc]

I opened rust-lang/rfcs#428 to discuss the proper fix for the object-safety/static method issue.