ICE defining function taking raw trait object

[ollie27]

The following code:

pub fn f(_: ToString) {}

fn main() {}

Produces the following in the playground:

rustc 1.17.0 (56124baa9 2017-04-24)
Incorrect number of arguments passed to called function!
  invoke void @_ZN4drop17h87bc09ef66481f99E(i8* %3)
          to label %bb1 unwind label %cleanup, !dbg !18
LLVM ERROR: Broken function found, compilation aborted!

rustc 1.18.0-beta.4 (0308c9865 2017-05-27)
Segmentation fault (core dumped)

rustc 1.19.0-nightly (5de00925b 2017-05-29)
rustc: /checkout/src/llvm/include/llvm/Support/Casting.h:95: static bool llvm::isa_impl_cl<To, const From*>::doit(const From*) [with To = llvm::Constant; From = llvm::Value]: Assertion `Val && "isa<> used on a null pointer"' failed.
Aborted (core dumped)

[eddyb]

cc @nikomatsakis More fallout from us not banning unsized argument types outright.

[nikomatsakis]

This is supposed to fail in WF checking, I believe. That is, any actual function body should demand that the arguments types are sized.

[eddyb]

Well it's ignored as a pattern, that might have something to do with it.

[nikomatsakis]

Good call, that's exactly it. It looks like we're missing some checks -- we should require that the arguments themselves are all sized, and not just the bindings. I'll note down some mentoring instructions.

[nikomatsakis]

It seems like the right place to enforce this is when type-checking function bodies. Right now, that code does the following checks:

• We enforce that the return type is sized here.
• We enforce that each variable binding is sized when doing the gather-locals walk, on this line.

So I think what we need to do is to add a call to require_type_is_sized() roughly around here, invoking it on the type of the argument itself, which can be found in the arg_ty variable in that loop.

[nikomatsakis]

While we're at it, I think we can remove these lines.

[venkatagiri]

I would like to work on this.
I have done the changes suggested in #42312 (comment) but this is causing a couple of tests to fail.

compile-fail/unsized6.rs is failing as same error is repeated twice. One pointing at the arguments and the other pointing at the function body(which is being created by my changes).
Is this the expected change?

error[E0277]: the trait bound `X: std::marker::Sized` is not satisfied
  --> /home/vagrant/repos/rust/src/test/compile-fail/unsized6.rs:37:18
   |
37 | fn g1<X: ?Sized>(x: X) {} //~ERROR `X: std::marker::Sized` is not satisfied
   |                  ^ `X` does not have a constant size known at compile-time
   |
   = help: the trait `std::marker::Sized` is not implemented for `X`
   = help: consider adding a `where X: std::marker::Sized` bound
   = note: all local variables must have a statically known size

error[E0277]: the trait bound `X: std::marker::Sized` is not satisfied
  --> /home/vagrant/repos/rust/src/test/compile-fail/unsized6.rs:37:24
   |
37 | fn g1<X: ?Sized>(x: X) {} //~ERROR `X: std::marker::Sized` is not satisfied
   |                        ^ `X` does not have a constant size known at compile-time
   |
   = help: the trait `std::marker::Sized` is not implemented for `X`
   = help: consider adding a `where X: std::marker::Sized` bound

compile-fail/issue-38954.rs is failing with below error when no error was expected before.

error[E0277]: the trait bound `str: std::marker::Sized` is not satisfied
  --> /home/vagrant/repos/rust/src/test/compile-fail/issue-38954.rs:13:23
   |
13 | fn _test(ref _p: str) {}
   |                       ^ `str` does not have a constant size known at compile-time
   |
   = help: the trait `std::marker::Sized` is not implemented for `str`

[nikomatsakis]

@venkatagiri

compile-fail/unsized6.rs is failing as same error is repeated twice

Ah, I guess the problem is that we are adding (1) check on "the binding" and one check on the variable as a whole. That's kind of annoying. I kind of feel like we potentially need both checks; e.g., if you had something like fn foo(&x: &Trait) then the parameter type is sized but the type of x is not.

We could avoid this in almost all cases via a simple hack, where we skip the new check if the pattern binding is simple (i.e., just some identifier x). Something like

// Hack to avoid duplicate warnings for simple cases like `fn foo(x: Trait)`, where we would error
// once on the parameter as a whole, and once on the binding `x`.
if arg.pat.simple_name().is_none() { require_type_is_sized(..) }

compile-fail/issue-38954.rs is failing with below error when no error was expected before.

that should have been failing all along I think. You can just adjust the //~ ERROR annotation.

[venkatagiri]

@nikomatsakis Thanks for the tip. Have opened #42642 with the fix.