You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
LLVM error: Call parameter type does not match function signature #36744
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)
structA<'a>{a:&'ai32,}fncall<T>(s:T,functions:&Vec<fn(&T)>){for function in functions {function(&s);}}fnf(a:&A){}fnmain(){let a = A{a:&10};let vec:Vec<fn(&A)> = vec![f];call(a,&vec);}
structA<'a>{a:&'ai32,}fncall<T>(s:T,functions:&Vec<fn(&T)>){}fnf(a:&A){}fnmain(){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.
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...):
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 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?
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.
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).
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?
@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.)
Activity
TimNN commentedon Sep 26, 2016
stable 1.8.0 is fine, stable 1.9.0 fails
alexcrichton commentedon Sep 26, 2016
Nominating as this was a stable-to-stable regression
TimNN commentedon Sep 26, 2016
On nightly, this was introduced between
nightly-2016-03-24andnightly-2016-03-25(Changes)C4K3 commentedon Sep 28, 2016
According to git-bisect f3ac509 is the cause.
nikomatsakis commentedon Oct 4, 2016
First off, this is a more minimal test:
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,
callis expecting a&Vec<fn(&T)>. This implies that theTcannot contain any bound regions.Now, in
main, we have aVec<for<'a, 'b> fn(&'a A<'b>)>(note that both regions are bound). When we callcall, we will try to find a value forTand (I believe) we will getA<'x>where'xis the lifetime of the call tocall()(I put a comment in there). This means that the monomorphized, erased, formal argument type forcallis&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 commentedon Oct 5, 2016
The problem even arises with this slightly further reduced, independently-developed variant of what @nikomatsakis posted:
Even here, where we are not passing a value of type
T, we still are inferring a type forTwith the erased region, as @nikomatsakis noted (I hackedrustcslightly so that-Z verbosecaused us to explicitly printReErasedjust like any other region...):nikomatsakis commentedon Oct 5, 2016
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 commentedon Oct 5, 2016
We could just erase all bound lifetimes in covariant slots - since these can't matter for layout.
nikomatsakis commentedon Oct 5, 2016
@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'awould be in an invariant location, no?arielb1 commentedon 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
TyAdtrequire a re-erasure with high potential for new bugs.NOTE: this can be reached even without calls:
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 commentedon Oct 5, 2016
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)>andVec<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 commentedon Oct 6, 2016
@nikomatsakis If I may ruin your dream... those types can be distinct in stable Rust.
nikomatsakis commentedon Oct 6, 2016
@eddyb I am aware of that, but they can still share the same LLVM representation without a problem, I think?
eddyb commentedon 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
'erasedand create the LLVM representation from that, because the LLVM type may depend on higher-ranked regions.)nikomatsakis commentedon Oct 11, 2016
@eddyb hmm, ok, I see your point. Yeah, I was wrong. =)
Auto merge of #37112 - pnkfelix:fix-issue-36744, r=arielb1