Skip to content
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

LLVM error: Call parameter type does not match function signature #36744

Closed
C4K3 opened this issue Sep 26, 2016 · 15 comments
Closed

LLVM error: Call parameter type does not match function signature #36744

C4K3 opened this issue Sep 26, 2016 · 15 comments
Assignees
Labels
P-high High priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@C4K3
Copy link

C4K3 commented Sep 26, 2016

I get the error

Call parameter type does not match function signature!
  %vec = alloca %"3.std::vec::Vec<fn(&B<'_>)>"
 %"3.std::vec::Vec<fn(&B)>"*  invoke void @_ZN5error4call17h314c247b38f70641E(i8* %15, %"3.std::vec::Vec<fn(&B<'_>)>"* noalias readonly dereferenceable(24) %vec)
          to label %normal-return unwind label %unwind_ast_73_, !dbg !407
LLVM ERROR: Broken function found, compilation aborted!
error: Could not compile `error`.

on this code (which I reduced as much as possible while still getting the error)

struct A<'a> {
    a: &'a i32,
}

fn call<T>(s: T, functions: &Vec<fn(&T)>) {
    for function in functions {
        function(&s);
    }
}

fn f(a: &A) { }

fn main() {
    let a = A { a: &10 };

    let vec: Vec<fn(&A)> = vec![f];
    call(a, &vec);
}

This is on Rust 1.11.0, on Linux 64.

@TimNN
Copy link
Contributor

TimNN commented Sep 26, 2016

stable 1.8.0 is fine, stable 1.9.0 fails

@alexcrichton alexcrichton added I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. labels Sep 26, 2016
@alexcrichton
Copy link
Member

Nominating as this was a stable-to-stable regression

@TimNN
Copy link
Contributor

TimNN commented Sep 26, 2016

On nightly, this was introduced between nightly-2016-03-24 and nightly-2016-03-25 (Changes)

@C4K3
Copy link
Author

C4K3 commented Sep 28, 2016

According to git-bisect f3ac509 is the cause.

@pnkfelix pnkfelix added P-high High priority and removed I-nominated labels Sep 29, 2016
@pnkfelix pnkfelix self-assigned this Sep 29, 2016
@nikomatsakis
Copy link
Contributor

First off, this is a more minimal test:

struct A<'a> {
    a: &'a i32,
}

fn call<T>(s: T, functions: &Vec<fn(&T)>) {
}

fn f(a: &A) { }

fn main() {
    let a = A { a: &10 };

    let vec: Vec<fn(&A)> = vec![f];
    call(a, &vec); // 'x
}

In any case, @pnkfelix, I looked into this a bit. I am not sure why this ever worked, but I think I see the problem. It has to do with the subtyping (as opposed to coercion) relationship between bound and free regions. In particular, call is expecting a &Vec<fn(&T)>. This implies that the T cannot contain any bound regions.

Now, in main, we have a Vec<for<'a, 'b> fn(&'a A<'b>)> (note that both regions are bound). When we call call, we will try to find a value for T and (I believe) we will get A<'x> where 'x is the lifetime of the call to call() (I put a comment in there). This means that the monomorphized, erased, formal argument type for call is &Vec<for<'a> fn(&'a A<'erased>)>. The actual type we are providing is of course &Vec<for<'a, 'b> fn(&'a A<'b>)>. Close but not the same.

Presumably some bug or other made this work before. We probably just want to insert an LLVM cast in such cases though.

@pnkfelix
Copy link
Member

pnkfelix commented Oct 5, 2016

The problem even arises with this slightly further reduced, independently-developed variant of what @nikomatsakis posted:

#![allow(dead_code)]
struct A<'l> { _a: &'l i32 }

fn call<'m, T>(_functions: &'m Vec<for <'n> fn(&'n T)>) { }

#[cfg(wont_compile_due_to_type_mismatch)]
fn caller1<'o>(vec: &'o Vec<for <'s> fn(&'s A<'s>)>) { call(vec); }

fn caller2<'t>(vec: &'t Vec<for <'u,'v> fn(&'u A<'v>)>) { call(vec); }

fn main() { }

Even here, where we are not passing a value of type T, we still are inferring a type for T with the erased region, as @nikomatsakis noted (I hacked rustc slightly so that -Z verbose caused us to explicitly print ReErased just like any other region...):

Expected %"3.std::vec::Vec<fn(&ReLateBound(DebruijnIndex { depth: 1 }, BrAnon(1)) A<ReErased>)>"* for param 1,
got      %"3.std::vec::Vec<fn(&ReLateBound(DebruijnIndex { depth: 1 }, BrAnon(1)) A<ReLateBound(DebruijnIndex { depth: 1 }, BrAnon(2))>)>"*

@nikomatsakis
Copy link
Contributor

It seems to me that fully erasing even bound regions in LLVM types might well be a reasonable thing to do. But that might then lead to duplicate definitions of types, I suppose? For example, these two vector types are distinct types that are both declared, presumably.

The only option I see is inserting casts to account for subtyping, which I believe we used to do (I remember hitting this problem before in the distant past). Perhaps this code just got left out of MIR trans?

@arielb1
Copy link
Contributor

arielb1 commented Oct 5, 2016

We could just erase all bound lifetimes in covariant slots - since these can't matter for layout.

@nikomatsakis
Copy link
Contributor

@arielb1 I'm not sure why you singled out "covariant slots" here (or even what you mean by a "covariant slot", actually). For example, if the type were Vec<for<'a> fn(Cell<&'a T>)>, we would have the same issue, but 'a would be in an invariant location, no?

@arielb1
Copy link
Contributor

arielb1 commented Oct 5, 2016

Sure, you have to look at the variance of the binder, not of the lifetime. OTOH, this would make extracting type parameters from TyAdt require a re-erasure with high potential for new bugs.

NOTE: this can be reached even without calls:

pub struct A<'a>(&'a ());
pub struct S<T>(T);

#[no_mangle]
pub fn bad<'s>(v: &mut S<fn(A<'s>)>, y: S<for<'b> fn(A<'b>)>) {
    *v = y;
}

fn main() {}

This occurs at every place MIR has subtyping. If MIR type-checking is correct, this is function arguments, function return values, and assignments. If we go by that, this bug could be fixed by checking whether the erased Rust types are different at these points and performing the appropriate cast (this could hide bugs, but that's life - maybe add "proper-subtype" annotations to MIR or something).

@nikomatsakis
Copy link
Contributor

If we go by that, this bug could be fixed by checking whether the erased Rust types are different at these points and performing the appropriate cast

Yeah, this is precisely what I was thinking of doing. I agree it can hide bugs, but we can at minimum add some kind of assertions (same size, etc).

One other thought I had is that we could probably erase lifetimes fully when generating LLVM types, but we'd have to be aware that distinct Rust types may map to the same LLVM type (but it should have the same runtime representation). We could probably just keep a set of the LLVM types that have been created. IOW, if we have Vec<for<'a> fn(&'a i32)> and Vec<fn(&'b i32)> those would both map to the same LLVM type (and we'd have to make sure we don't create two definitions). Not sure how annoying that would be-- should work though, I would think?

@eddyb
Copy link
Member

eddyb commented Oct 6, 2016

@nikomatsakis If I may ruin your dream... those types can be distinct in stable Rust.

@nikomatsakis
Copy link
Contributor

@eddyb I am aware of that, but they can still share the same LLVM representation without a problem, I think?

@eddyb
Copy link
Member

eddyb commented Oct 6, 2016

@nikomatsakis How would you even determine that automatically without building them first?
(What I'm saying is that you can't just replace all regions with 'erased and create the LLVM representation from that, because the LLVM type may depend on higher-ranked regions.)

@nikomatsakis
Copy link
Contributor

@eddyb hmm, ok, I see your point. Yeah, I was wrong. =)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-high High priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants