Skip to content

LLVM error with trait objects #47638

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

Closed
matthewjasper opened this issue Jan 21, 2018 · 8 comments
Closed

LLVM error with trait objects #47638

matthewjasper opened this issue Jan 21, 2018 · 8 comments
Assignees
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. P-high High priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Milestone

Comments

@matthewjasper
Copy link
Contributor

matthewjasper commented Jan 21, 2018

The following code

fn id<'c, 'b>(f: &'c &'b Fn(&i32)) -> &'c &'b Fn(&'static i32) {
    f
}

fn main() {
    let f: &Fn(&i32) = &|x| {};
    id(&f);
}

gives the following error on beta (1.24) and nightly (1.25 2018-01-17). It compiles successfully on stable.

Function return type does not match operand type of return inst!
  ret { %"core::ops::function::Fn<(&i32), Output=()>.0"*, {}* }* %3, !dbg !36
 { %"core::ops::function::Fn<(&i32), Output=()>"*, {}* }*LLVM ERROR: Broken function found, compilation aborted!
@dtolnay dtolnay added regression-from-stable-to-beta Performance or correctness regression from stable to beta. A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 21, 2018
@dtolnay
Copy link
Member

dtolnay commented Jan 21, 2018

Mentioning @michaelwoerister @eddyb @nagisa who touched librustc_trans recently (disclaimer: I don't know if that's where this problem would come from but it sounds relevant).

@pietroalbini pietroalbini added this to the 1.24 milestone Jan 21, 2018
@eddyb
Copy link
Member

eddyb commented Jan 21, 2018

cc @pnkfelix @nikomatsakis Is this one of those "HRTB lifetimes in rustc_trans causes distinct LLVM types" bugs? It seems like there's two distinct types for the unsized pointee of Fn(&i32).

@TimNN TimNN added the C-bug Category: This is a bug. label Jan 23, 2018
@nikomatsakis nikomatsakis self-assigned this Jan 25, 2018
@nikomatsakis
Copy link
Contributor

triage: P-high

We need to figure this out.

@rust-highfive rust-highfive added the P-high High priority label Jan 25, 2018
@ritiek
Copy link
Member

ritiek commented Jan 27, 2018

So, I was testing nightly toolchains and this has been a problem since:

$ rustc -vV
rustc 1.23.0-nightly (33374fa9d 2017-11-20)
binary: rustc
commit-hash: 33374fa9d09e2a790979b31e61100dfed4b44139
commit-date: 2017-11-20
host: x86_64-unknown-linux-gnu
release: 1.23.0-nightly
LLVM version: 4.0

The code compiles OK with previous day's nightly:

$ rustc -vV
rustc 1.23.0-nightly (5041b3bb3 2017-11-19)
binary: rustc
commit-hash: 5041b3bb3d953a14f32b15d1e41341c629acae12
commit-date: 2017-11-19
host: x86_64-unknown-linux-gnu
release: 1.23.0-nightly
LLVM version: 4.0

@dtolnay
Copy link
Member

dtolnay commented Jan 27, 2018

5041b3b...33374fa then, almost certainly something in #45225. @eddyb

@eddyb
Copy link
Member

eddyb commented Jan 27, 2018

@dtolnay I introduced it for this specific case, yeah, but it's a long-standing bug AFAICT (#36744).
We have some hacks when doing stores and calls, at least, but it's not ideal.
I'm hoping @nikomatsakis can confirm and knows of a better direction for #36744.

@nikomatsakis
Copy link
Contributor

So I agree with @eddyb that it's the same bug. I think my preferred longer term fix is to change our rules around subtyping and higher-ranked regions -- but in the short term, I suspect we need some bit casts.

@eddyb
Copy link
Member

eddyb commented Jan 30, 2018

This is not a new regression in its general case, only specifically trait objects outside of structs:

struct Newtype<T>(T);

fn id<'c, 'b>(f: &'c Newtype<&'b Fn(&i32)>) -> &'c Newtype<&'b Fn(&'static i32)> {
    f
}

fn main() {
    let f: &Fn(&i32) = &|x| {};
    id(&Newtype(f));
}

This fails with the LLVM error in 1.0 through 1.22, according to godbolt.
Somehow, #45225 (presumably) fixed that and introduced a different form of it.

kennytm added a commit to kennytm/rust that referenced this issue Jan 31, 2018
rustc_trans: keep LLVM types for trait objects anonymous.

Fixes rust-lang#47638 by reverting the addition of readable LLVM trait object type names.
r? @nikomatsakis
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. P-high High priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. 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

8 participants