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

rustc_codegen_llvm: replace the first argument early in FnType::new_vtable. #52089

Merged
merged 1 commit into from
Jul 12, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 43 additions & 37 deletions src/librustc_codegen_llvm/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,9 +261,12 @@ pub trait FnTypeExt<'a, 'tcx> {
fn new_vtable(cx: &CodegenCx<'a, 'tcx>,
sig: ty::FnSig<'tcx>,
extra_args: &[Ty<'tcx>]) -> Self;
fn unadjusted(cx: &CodegenCx<'a, 'tcx>,
sig: ty::FnSig<'tcx>,
extra_args: &[Ty<'tcx>]) -> Self;
fn new_internal(
cx: &CodegenCx<'a, 'tcx>,
sig: ty::FnSig<'tcx>,
extra_args: &[Ty<'tcx>],
mk_arg_type: impl Fn(Ty<'tcx>, Option<usize>) -> ArgType<'tcx, Ty<'tcx>>,
) -> Self;
fn adjust_for_abi(&mut self,
cx: &CodegenCx<'a, 'tcx>,
abi: Abi);
Expand All @@ -285,40 +288,40 @@ impl<'a, 'tcx> FnTypeExt<'a, 'tcx> for FnType<'tcx, Ty<'tcx>> {
fn new(cx: &CodegenCx<'a, 'tcx>,
sig: ty::FnSig<'tcx>,
extra_args: &[Ty<'tcx>]) -> Self {
let mut fn_ty = FnType::unadjusted(cx, sig, extra_args);
fn_ty.adjust_for_abi(cx, sig.abi);
fn_ty
FnType::new_internal(cx, sig, extra_args, |ty, _| {
ArgType::new(cx.layout_of(ty))
})
}

fn new_vtable(cx: &CodegenCx<'a, 'tcx>,
sig: ty::FnSig<'tcx>,
extra_args: &[Ty<'tcx>]) -> Self {
let mut fn_ty = FnType::unadjusted(cx, sig, extra_args);
// Don't pass the vtable, it's not an argument of the virtual fn.
{
let self_arg = &mut fn_ty.args[0];
match self_arg.mode {
PassMode::Pair(data_ptr, _) => {
self_arg.mode = PassMode::Direct(data_ptr);
}
_ => bug!("FnType::new_vtable: non-pair self {:?}", self_arg)
}

let pointee = self_arg.layout.ty.builtin_deref(true)
.unwrap_or_else(|| {
bug!("FnType::new_vtable: non-pointer self {:?}", self_arg)
}).ty;
let fat_ptr_ty = cx.tcx.mk_mut_ptr(pointee);
self_arg.layout = cx.layout_of(fat_ptr_ty).field(cx, 0);
}
fn_ty.adjust_for_abi(cx, sig.abi);
fn_ty
FnType::new_internal(cx, sig, extra_args, |ty, arg_idx| {
let mut layout = cx.layout_of(ty);
// Don't pass the vtable, it's not an argument of the virtual fn.
// Instead, pass just the (thin pointer) first field of `*dyn Trait`.
if arg_idx == Some(0) {
// FIXME(eddyb) `layout.field(cx, 0)` is not enough because e.g.
// `Box<dyn Trait>` has a few newtype wrappers around the raw
// pointer, so we'd have to "dig down" to find `*dyn Trait`.
let pointee = layout.ty.builtin_deref(true)
.unwrap_or_else(|| {
bug!("FnType::new_vtable: non-pointer self {:?}", layout)
}).ty;
let fat_ptr_ty = cx.tcx.mk_mut_ptr(pointee);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This variable name is confusing, given that this is not a type of a fat pointer, but rather its data pointer only.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, never mind, I think I got it wrong myself lol.

layout = cx.layout_of(fat_ptr_ty).field(cx, 0);
}
ArgType::new(layout)
})
}

fn unadjusted(cx: &CodegenCx<'a, 'tcx>,
sig: ty::FnSig<'tcx>,
extra_args: &[Ty<'tcx>]) -> Self {
debug!("FnType::unadjusted({:?}, {:?})", sig, extra_args);
fn new_internal(
cx: &CodegenCx<'a, 'tcx>,
sig: ty::FnSig<'tcx>,
extra_args: &[Ty<'tcx>],
mk_arg_type: impl Fn(Ty<'tcx>, Option<usize>) -> ArgType<'tcx, Ty<'tcx>>,
) -> Self {
debug!("FnType::new_internal({:?}, {:?})", sig, extra_args);

use self::Abi::*;
let conv = match cx.sess().target.target.adjust_abi(sig.abi) {
Expand Down Expand Up @@ -435,8 +438,9 @@ impl<'a, 'tcx> FnTypeExt<'a, 'tcx> for FnType<'tcx, Ty<'tcx>> {
}
};

let arg_of = |ty: Ty<'tcx>, is_return: bool| {
let mut arg = ArgType::new(cx.layout_of(ty));
let arg_of = |ty: Ty<'tcx>, arg_idx: Option<usize>| {
let is_return = arg_idx.is_none();
let mut arg = mk_arg_type(ty, arg_idx);
if arg.layout.is_zst() {
// For some forsaken reason, x86_64-pc-windows-gnu
// doesn't ignore zero-sized struct arguments.
Expand Down Expand Up @@ -479,14 +483,16 @@ impl<'a, 'tcx> FnTypeExt<'a, 'tcx> for FnType<'tcx, Ty<'tcx>> {
arg
};

FnType {
ret: arg_of(sig.output(), true),
args: inputs.iter().chain(extra_args.iter()).map(|ty| {
arg_of(ty, false)
let mut fn_ty = FnType {
ret: arg_of(sig.output(), None),
args: inputs.iter().chain(extra_args).enumerate().map(|(i, ty)| {
arg_of(ty, Some(i))
}).collect(),
variadic: sig.variadic,
conv,
}
};
fn_ty.adjust_for_abi(cx, sig.abi);
fn_ty
}

fn adjust_for_abi(&mut self,
Expand Down
26 changes: 26 additions & 0 deletions src/test/run-pass/issue-51907.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

trait Foo {
extern fn borrow(&self);
extern fn take(self: Box<Self>);
}

struct Bar;
impl Foo for Bar {
extern fn borrow(&self) {}
extern fn take(self: Box<Self>) {}
}

fn main() {
let foo: Box<dyn Foo> = Box::new(Bar);
foo.borrow();
foo.take()
}