From 012116f860eb5b859218640e963ba32545311a24 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Thu, 3 Oct 2019 19:10:34 -0700 Subject: [PATCH 01/12] Add InstanceDef::ReifyShim for track_caller functions. --- src/librustc/mir/mono.rs | 1 + src/librustc/ty/instance.rs | 7 +++++++ src/librustc/ty/mod.rs | 1 + src/librustc/ty/structural_impls.rs | 5 ++++- src/librustc_mir/interpret/terminator.rs | 1 + src/librustc_mir/monomorphize/collector.rs | 2 ++ src/librustc_mir/monomorphize/partitioning.rs | 2 ++ src/librustc_mir/shim.rs | 9 +++++++++ 8 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/librustc/mir/mono.rs b/src/librustc/mir/mono.rs index 313b2a5d50a30..265ac975ed7a2 100644 --- a/src/librustc/mir/mono.rs +++ b/src/librustc/mir/mono.rs @@ -386,6 +386,7 @@ impl<'tcx> CodegenUnit<'tcx> { tcx.hir().as_local_hir_id(def_id) } InstanceDef::VtableShim(..) | + InstanceDef::ReifyShim(..) | InstanceDef::Intrinsic(..) | InstanceDef::FnPtrShim(..) | InstanceDef::Virtual(..) | diff --git a/src/librustc/ty/instance.rs b/src/librustc/ty/instance.rs index 93a8341e74628..e13e097b482f7 100644 --- a/src/librustc/ty/instance.rs +++ b/src/librustc/ty/instance.rs @@ -25,6 +25,9 @@ pub enum InstanceDef<'tcx> { /// `::method` where `method` receives unsizeable `self: Self`. VtableShim(DefId), + /// `fn()` where the function is annotated with `#[track_caller]`. + ReifyShim(DefId), + /// `::call_*` /// `DefId` is `FnTrait::call_*` FnPtrShim(DefId, Ty<'tcx>), @@ -123,6 +126,7 @@ impl<'tcx> InstanceDef<'tcx> { match *self { InstanceDef::Item(def_id) | InstanceDef::VtableShim(def_id) | + InstanceDef::ReifyShim(def_id) | InstanceDef::FnPtrShim(def_id, _) | InstanceDef::Virtual(def_id, _) | InstanceDef::Intrinsic(def_id, ) | @@ -178,6 +182,9 @@ impl<'tcx> fmt::Display for Instance<'tcx> { InstanceDef::VtableShim(_) => { write!(f, " - shim(vtable)") } + InstanceDef::ReifyShim(_) => { + write!(f, " - shim(reify)") + } InstanceDef::Intrinsic(_) => { write!(f, " - intrinsic") } diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index cfd859c33c2ef..3692caada577c 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -3026,6 +3026,7 @@ impl<'tcx> TyCtxt<'tcx> { self.optimized_mir(did) } ty::InstanceDef::VtableShim(..) | + ty::InstanceDef::ReifyShim(..) | ty::InstanceDef::Intrinsic(..) | ty::InstanceDef::FnPtrShim(..) | ty::InstanceDef::Virtual(..) | diff --git a/src/librustc/ty/structural_impls.rs b/src/librustc/ty/structural_impls.rs index 5aa59cc309fc7..8945e1a1debdb 100644 --- a/src/librustc/ty/structural_impls.rs +++ b/src/librustc/ty/structural_impls.rs @@ -761,6 +761,8 @@ impl<'a, 'tcx> Lift<'tcx> for ty::InstanceDef<'a> { Some(ty::InstanceDef::Item(def_id)), ty::InstanceDef::VtableShim(def_id) => Some(ty::InstanceDef::VtableShim(def_id)), + ty::InstanceDef::ReifyShim(def_id) => + Some(ty::InstanceDef::ReifyShim(def_id)), ty::InstanceDef::Intrinsic(def_id) => Some(ty::InstanceDef::Intrinsic(def_id)), ty::InstanceDef::FnPtrShim(def_id, ref ty) => @@ -966,6 +968,7 @@ impl<'tcx> TypeFoldable<'tcx> for ty::instance::Instance<'tcx> { def: match self.def { Item(did) => Item(did.fold_with(folder)), VtableShim(did) => VtableShim(did.fold_with(folder)), + ReifyShim(did) => ReifyShim(did.fold_with(folder)), Intrinsic(did) => Intrinsic(did.fold_with(folder)), FnPtrShim(did, ty) => FnPtrShim( did.fold_with(folder), @@ -994,7 +997,7 @@ impl<'tcx> TypeFoldable<'tcx> for ty::instance::Instance<'tcx> { use crate::ty::InstanceDef::*; self.substs.visit_with(visitor) || match self.def { - Item(did) | VtableShim(did) | Intrinsic(did) | Virtual(did, _) => { + Item(did) | VtableShim(did) | ReifyShim(did) | Intrinsic(did) | Virtual(did, _) => { did.visit_with(visitor) }, FnPtrShim(did, ty) | CloneShim(did, ty) => { diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index ef6b7d626e7a4..11c7cd0d901d0 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -263,6 +263,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Ok(()) } ty::InstanceDef::VtableShim(..) | + ty::InstanceDef::ReifyShim(..) | ty::InstanceDef::ClosureOnceShim { .. } | ty::InstanceDef::FnPtrShim(..) | ty::InstanceDef::DropGlue(..) | diff --git a/src/librustc_mir/monomorphize/collector.rs b/src/librustc_mir/monomorphize/collector.rs index 3f0a2674305d4..07ae4837d1cb3 100644 --- a/src/librustc_mir/monomorphize/collector.rs +++ b/src/librustc_mir/monomorphize/collector.rs @@ -747,6 +747,7 @@ fn visit_instance_use<'tcx>( } } ty::InstanceDef::VtableShim(..) | + ty::InstanceDef::ReifyShim(..) | ty::InstanceDef::Virtual(..) | ty::InstanceDef::DropGlue(_, None) => { // don't need to emit shim if we are calling directly. @@ -773,6 +774,7 @@ fn should_monomorphize_locally<'tcx>(tcx: TyCtxt<'tcx>, instance: &Instance<'tcx let def_id = match instance.def { ty::InstanceDef::Item(def_id) => def_id, ty::InstanceDef::VtableShim(..) | + ty::InstanceDef::ReifyShim(..) | ty::InstanceDef::ClosureOnceShim { .. } | ty::InstanceDef::Virtual(..) | ty::InstanceDef::FnPtrShim(..) | diff --git a/src/librustc_mir/monomorphize/partitioning.rs b/src/librustc_mir/monomorphize/partitioning.rs index 6d9dae7214cc2..b9d38028b72a8 100644 --- a/src/librustc_mir/monomorphize/partitioning.rs +++ b/src/librustc_mir/monomorphize/partitioning.rs @@ -339,6 +339,7 @@ fn mono_item_visibility( // These are all compiler glue and such, never exported, always hidden. InstanceDef::VtableShim(..) | + InstanceDef::ReifyShim(..) | InstanceDef::FnPtrShim(..) | InstanceDef::Virtual(..) | InstanceDef::Intrinsic(..) | @@ -677,6 +678,7 @@ fn characteristic_def_id_of_mono_item<'tcx>( let def_id = match instance.def { ty::InstanceDef::Item(def_id) => def_id, ty::InstanceDef::VtableShim(..) | + ty::InstanceDef::ReifyShim(..) | ty::InstanceDef::FnPtrShim(..) | ty::InstanceDef::ClosureOnceShim { .. } | ty::InstanceDef::Intrinsic(..) | diff --git a/src/librustc_mir/shim.rs b/src/librustc_mir/shim.rs index 78008e2fcd361..10240271eb8ea 100644 --- a/src/librustc_mir/shim.rs +++ b/src/librustc_mir/shim.rs @@ -41,6 +41,15 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> &'tcx None, ) } + ty::InstanceDef::ReifyShim(def_id) => { + build_call_shim( + tcx, + def_id, + Adjustment::DerefMove, + CallKind::Direct(def_id), + None, + ) + } ty::InstanceDef::FnPtrShim(def_id, ty) => { let trait_ = tcx.trait_of_item(def_id).unwrap(); let adjustment = match tcx.lang_items().fn_trait_kind(trait_) { From f5f67e78bb3bfbc391699021c89b07f1aff3aa26 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Sat, 5 Oct 2019 14:24:07 -0700 Subject: [PATCH 02/12] Add Instance::resolve_for_fn_ptr --- src/librustc/ty/instance.rs | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/librustc/ty/instance.rs b/src/librustc/ty/instance.rs index e13e097b482f7..f547da00b1ad9 100644 --- a/src/librustc/ty/instance.rs +++ b/src/librustc/ty/instance.rs @@ -25,7 +25,7 @@ pub enum InstanceDef<'tcx> { /// `::method` where `method` receives unsizeable `self: Self`. VtableShim(DefId), - /// `fn()` where the function is annotated with `#[track_caller]`. + /// `fn()` pointer where the function is annotated with `#[track_caller]`. ReifyShim(DefId), /// `::call_*` @@ -297,6 +297,28 @@ impl<'tcx> Instance<'tcx> { result } + pub fn resolve_for_fn_ptr( + tcx: TyCtxt<'tcx>, + param_env: ty::ParamEnv<'tcx>, + def_id: DefId, + substs: SubstsRef<'tcx>, + ) -> Option> { + debug!("resolve(def_id={:?}, substs={:?})", def_id, substs); + let fn_sig = tcx.fn_sig(def_id); + // let is_reify_shim = fn_sig.inputs().skip_binder().len() > 0 + // && fn_sig.input(0).skip_binder().is_param(0) + // && tcx.generics_of(def_id).has_self; + if is_reify_shim { + debug!(" => fn ptr with implicit caller location"); + Some(Instance { + def: InstanceDef::ReifyShim(def_id), + substs, + }) + } else { + Instance::resolve(tcx, param_env, def_id, substs) + } + } + pub fn resolve_for_vtable( tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>, From 01327384370b81e3a155dc70a2533935c4495db8 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Wed, 9 Oct 2019 06:05:49 -0700 Subject: [PATCH 03/12] Reifying callers of Instance::resolve use resolve_for_fn_ptr. --- src/librustc/ty/instance.rs | 9 +++------ src/librustc_codegen_ssa/callee.rs | 17 +++++++++++++++++ src/librustc_codegen_ssa/mir/rvalue.rs | 2 +- src/librustc_mir/monomorphize/collector.rs | 10 ++++++---- src/librustc_mir/shim.rs | 2 +- 5 files changed, 28 insertions(+), 12 deletions(-) diff --git a/src/librustc/ty/instance.rs b/src/librustc/ty/instance.rs index f547da00b1ad9..471bd1637546c 100644 --- a/src/librustc/ty/instance.rs +++ b/src/librustc/ty/instance.rs @@ -1,3 +1,4 @@ +// use crate::hir::CodegenFnAttrFlags; use crate::hir::Unsafety; use crate::hir::def::Namespace; use crate::hir::def_id::DefId; @@ -304,12 +305,8 @@ impl<'tcx> Instance<'tcx> { substs: SubstsRef<'tcx>, ) -> Option> { debug!("resolve(def_id={:?}, substs={:?})", def_id, substs); - let fn_sig = tcx.fn_sig(def_id); - // let is_reify_shim = fn_sig.inputs().skip_binder().len() > 0 - // && fn_sig.input(0).skip_binder().is_param(0) - // && tcx.generics_of(def_id).has_self; - if is_reify_shim { - debug!(" => fn ptr with implicit caller location"); + if false { + debug!(" => fn pointer created for function with #[track_caller]"); Some(Instance { def: InstanceDef::ReifyShim(def_id), substs, diff --git a/src/librustc_codegen_ssa/callee.rs b/src/librustc_codegen_ssa/callee.rs index 4744dd6302fb3..6ba6774cbf881 100644 --- a/src/librustc_codegen_ssa/callee.rs +++ b/src/librustc_codegen_ssa/callee.rs @@ -18,6 +18,23 @@ pub fn resolve_and_get_fn<'tcx, Cx: CodegenMethods<'tcx>>( ) } +pub fn resolve_and_get_fn_for_ptr<'tcx, + Cx: Backend<'tcx> + MiscMethods<'tcx> + TypeMethods<'tcx> +>( + cx: &Cx, + def_id: DefId, + substs: SubstsRef<'tcx>, +) -> Cx::Value { + cx.get_fn( + ty::Instance::resolve_for_fn_ptr( + cx.tcx(), + ty::ParamEnv::reveal_all(), + def_id, + substs + ).unwrap() + ) +} + pub fn resolve_and_get_fn_for_vtable<'tcx, Cx: Backend<'tcx> + MiscMethods<'tcx> + TypeMethods<'tcx> >( diff --git a/src/librustc_codegen_ssa/mir/rvalue.rs b/src/librustc_codegen_ssa/mir/rvalue.rs index 6ffa561f3fecf..978e7218aa745 100644 --- a/src/librustc_codegen_ssa/mir/rvalue.rs +++ b/src/librustc_codegen_ssa/mir/rvalue.rs @@ -190,7 +190,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { bug!("reifying a fn ptr that requires const arguments"); } OperandValue::Immediate( - callee::resolve_and_get_fn(bx.cx(), def_id, substs)) + callee::resolve_and_get_fn_for_ptr(bx.cx(), def_id, substs)) } _ => { bug!("{} cannot be reified to a fn ptr", operand.layout.ty) diff --git a/src/librustc_mir/monomorphize/collector.rs b/src/librustc_mir/monomorphize/collector.rs index 07ae4837d1cb3..a1f5562035c69 100644 --- a/src/librustc_mir/monomorphize/collector.rs +++ b/src/librustc_mir/monomorphize/collector.rs @@ -721,10 +721,12 @@ fn visit_fn_use<'tcx>( output: &mut Vec>, ) { if let ty::FnDef(def_id, substs) = ty.kind { - let instance = ty::Instance::resolve(tcx, - ty::ParamEnv::reveal_all(), - def_id, - substs).unwrap(); + let resolver = if is_direct_call { + ty::Instance::resolve + } else { + ty::Instance::resolve_for_fn_ptr + }; + let instance = resolver(tcx, ty::ParamEnv::reveal_all(), def_id, substs).unwrap(); visit_instance_use(tcx, instance, is_direct_call, output); } } diff --git a/src/librustc_mir/shim.rs b/src/librustc_mir/shim.rs index 10240271eb8ea..eced7eb09ae87 100644 --- a/src/librustc_mir/shim.rs +++ b/src/librustc_mir/shim.rs @@ -45,7 +45,7 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> &'tcx build_call_shim( tcx, def_id, - Adjustment::DerefMove, + Adjustment::Identity, // TODO(anp) is this the right kind of adjustment? CallKind::Direct(def_id), None, ) From b8414c13abf9dc994f9c0dccee848a8b0fb1b061 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Wed, 9 Oct 2019 06:20:23 -0700 Subject: [PATCH 04/12] Return ReifyShim from Instance::resolve_for_fn_ptr when track_caller present. This ICEs in MIR currently, which I think is to be expected since none of the MIR plumbing is set up. I added a test which confirms that the shim is being used for reifying a track_caller function. --- src/librustc/ty/instance.rs | 4 ++-- .../taking-fn-pointer.rs | 16 ++++++++++++++ .../taking-fn-pointer.stderr | 21 +++++++++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) create mode 100644 src/test/ui/rfc-2091-track-caller/taking-fn-pointer.rs create mode 100644 src/test/ui/rfc-2091-track-caller/taking-fn-pointer.stderr diff --git a/src/librustc/ty/instance.rs b/src/librustc/ty/instance.rs index 471bd1637546c..18b625984e1a2 100644 --- a/src/librustc/ty/instance.rs +++ b/src/librustc/ty/instance.rs @@ -1,4 +1,4 @@ -// use crate::hir::CodegenFnAttrFlags; +use crate::hir::CodegenFnAttrFlags; use crate::hir::Unsafety; use crate::hir::def::Namespace; use crate::hir::def_id::DefId; @@ -305,7 +305,7 @@ impl<'tcx> Instance<'tcx> { substs: SubstsRef<'tcx>, ) -> Option> { debug!("resolve(def_id={:?}, substs={:?})", def_id, substs); - if false { + if tcx.codegen_fn_attrs(def_id).flags.contains(CodegenFnAttrFlags::TRACK_CALLER) { debug!(" => fn pointer created for function with #[track_caller]"); Some(Instance { def: InstanceDef::ReifyShim(def_id), diff --git a/src/test/ui/rfc-2091-track-caller/taking-fn-pointer.rs b/src/test/ui/rfc-2091-track-caller/taking-fn-pointer.rs new file mode 100644 index 0000000000000..df3893f2a6a19 --- /dev/null +++ b/src/test/ui/rfc-2091-track-caller/taking-fn-pointer.rs @@ -0,0 +1,16 @@ +// failure-status: 101 +// normalize-stderr-test "note: rustc 1.* running on .*" -> "note: rustc VERSION running on TARGET" +// normalize-stderr-test "note: compiler flags: .*" -> "note: compiler flags: FLAGS" + +#![feature(track_caller)] //~ WARN the feature `track_caller` is incomplete + +#[track_caller] +fn f() {} + +fn call_it(x: fn()) { + x(); +} + +fn main() { + call_it(f); +} \ No newline at end of file diff --git a/src/test/ui/rfc-2091-track-caller/taking-fn-pointer.stderr b/src/test/ui/rfc-2091-track-caller/taking-fn-pointer.stderr new file mode 100644 index 0000000000000..fb625ae116e7c --- /dev/null +++ b/src/test/ui/rfc-2091-track-caller/taking-fn-pointer.stderr @@ -0,0 +1,21 @@ +warning: the feature `track_caller` is incomplete and may cause the compiler to crash + --> $DIR/taking-fn-pointer.rs:5:12 + | +LL | #![feature(track_caller)] + | ^^^^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + +thread 'rustc' panicked at 'index out of bounds: the len is 1 but the index is 1', $SRC_DIR/libcore/slice/mod.rs:LL:COL +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace. + +error: internal compiler error: unexpected panic + +note: the compiler unexpectedly panicked. this is a bug. + +note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports + +note: rustc VERSION running on TARGET + +note: compiler flags: FLAGS + From 8a097f26198d9979d88e7c0cd0a3bce283ff038a Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Wed, 9 Oct 2019 21:02:54 -0700 Subject: [PATCH 05/12] Clarify comment, dedupe match arms in shim.rs. Also add a missing terminal newline to a test. --- src/librustc/ty/instance.rs | 4 +++- src/librustc_mir/shim.rs | 17 +++++------------ .../rfc-2091-track-caller/taking-fn-pointer.rs | 2 +- 3 files changed, 9 insertions(+), 14 deletions(-) diff --git a/src/librustc/ty/instance.rs b/src/librustc/ty/instance.rs index 18b625984e1a2..2b9d09b20ccfc 100644 --- a/src/librustc/ty/instance.rs +++ b/src/librustc/ty/instance.rs @@ -26,7 +26,9 @@ pub enum InstanceDef<'tcx> { /// `::method` where `method` receives unsizeable `self: Self`. VtableShim(DefId), - /// `fn()` pointer where the function is annotated with `#[track_caller]`. + /// `fn()` pointer where the function itself cannot be turned into a pointer. + /// + /// One example in the compiler today is functions annotated with `#[track_caller]`. ReifyShim(DefId), /// `::call_*` diff --git a/src/librustc_mir/shim.rs b/src/librustc_mir/shim.rs index eced7eb09ae87..0d13daa264309 100644 --- a/src/librustc_mir/shim.rs +++ b/src/librustc_mir/shim.rs @@ -41,15 +41,6 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> &'tcx None, ) } - ty::InstanceDef::ReifyShim(def_id) => { - build_call_shim( - tcx, - def_id, - Adjustment::Identity, // TODO(anp) is this the right kind of adjustment? - CallKind::Direct(def_id), - None, - ) - } ty::InstanceDef::FnPtrShim(def_id, ty) => { let trait_ = tcx.trait_of_item(def_id).unwrap(); let adjustment = match tcx.lang_items().fn_trait_kind(trait_) { @@ -75,9 +66,11 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> &'tcx Some(arg_tys) ) } - ty::InstanceDef::Virtual(def_id, _) => { - // We are generating a call back to our def-id, which the - // codegen backend knows to turn to an actual virtual call. + // We are generating a call back to our def-id, which the + // codegen backend knows to turn to an actual virtual call. + ty::InstanceDef::Virtual(def_id, _) | + // ...or we are generating a call to the inner closure defined by #[track_caller] + ty::InstanceDef::ReifyShim(def_id) => { build_call_shim( tcx, def_id, diff --git a/src/test/ui/rfc-2091-track-caller/taking-fn-pointer.rs b/src/test/ui/rfc-2091-track-caller/taking-fn-pointer.rs index df3893f2a6a19..2882d2d83ff81 100644 --- a/src/test/ui/rfc-2091-track-caller/taking-fn-pointer.rs +++ b/src/test/ui/rfc-2091-track-caller/taking-fn-pointer.rs @@ -13,4 +13,4 @@ fn call_it(x: fn()) { fn main() { call_it(f); -} \ No newline at end of file +} From ea3ecf22ae911b7e32b889b636bce86c165908c4 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Wed, 9 Oct 2019 21:13:08 -0700 Subject: [PATCH 06/12] miri calls resolve_for_fn_ptr when reifying. --- src/librustc_mir/interpret/cast.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/cast.rs b/src/librustc_mir/interpret/cast.rs index d120412c901a6..9ab347957f97a 100644 --- a/src/librustc_mir/interpret/cast.rs +++ b/src/librustc_mir/interpret/cast.rs @@ -44,7 +44,14 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { if self.tcx.has_attr(def_id, sym::rustc_args_required_const) { bug!("reifying a fn ptr that requires const arguments"); } - let instance = self.resolve(def_id, substs)?; + + let instance = ty::Instance::resolve_for_fn_ptr( + *self.tcx, + self.param_env, + def_id, + substs, + ).ok_or_else(|| err_inval!(TooGeneric))?; + let fn_ptr = self.memory.create_fn_alloc(FnVal::Instance(instance)); self.write_scalar(Scalar::Ptr(fn_ptr.into()), dest)?; } From 1cdd5d099d3a8da42db6fbd259d3f757bccb961f Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Thu, 10 Oct 2019 07:25:54 -0700 Subject: [PATCH 07/12] Improve docs for InstanceDef::ReifyShim. --- src/librustc/ty/instance.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/librustc/ty/instance.rs b/src/librustc/ty/instance.rs index 2b9d09b20ccfc..b35b577ddf716 100644 --- a/src/librustc/ty/instance.rs +++ b/src/librustc/ty/instance.rs @@ -28,7 +28,10 @@ pub enum InstanceDef<'tcx> { /// `fn()` pointer where the function itself cannot be turned into a pointer. /// - /// One example in the compiler today is functions annotated with `#[track_caller]`. + /// One example in the compiler today is functions annotated with `#[track_caller]`, which + /// must have their implicit caller location argument populated for a call. Because this is a + /// required part of the function's ABI but can't be tracked as a property of the function + /// pointer, we create a single "caller location" at the site where the function is reified. ReifyShim(DefId), /// `::call_*` From 407d1d5fd4a81b2bc717fc86475209eb59d50fb8 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Thu, 10 Oct 2019 07:31:22 -0700 Subject: [PATCH 08/12] Instance::resolve_for_fn_ptr unconditionally resolves first. Per review feedback. --- src/librustc/ty/instance.rs | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/librustc/ty/instance.rs b/src/librustc/ty/instance.rs index b35b577ddf716..0a86b0b414802 100644 --- a/src/librustc/ty/instance.rs +++ b/src/librustc/ty/instance.rs @@ -310,15 +310,19 @@ impl<'tcx> Instance<'tcx> { substs: SubstsRef<'tcx>, ) -> Option> { debug!("resolve(def_id={:?}, substs={:?})", def_id, substs); - if tcx.codegen_fn_attrs(def_id).flags.contains(CodegenFnAttrFlags::TRACK_CALLER) { - debug!(" => fn pointer created for function with #[track_caller]"); - Some(Instance { - def: InstanceDef::ReifyShim(def_id), - substs, - }) - } else { - Instance::resolve(tcx, param_env, def_id, substs) - } + Instance::resolve(tcx, param_env, def_id, substs).map(|resolved| { + let resolved_def = resolved.def_id(); + let codegen_attrs = tcx.codegen_fn_attrs(resolved_def); + if codegen_attrs.flags.contains(CodegenFnAttrFlags::TRACK_CALLER) { + debug!(" => fn pointer created for function with #[track_caller]"); + Instance { + def: InstanceDef::ReifyShim(resolved_def), + substs, + } + } else { + resolved + } + }) } pub fn resolve_for_vtable( From 2dbc62b56a1ce0457108c86156a6ab357d870f08 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Thu, 10 Oct 2019 07:50:33 -0700 Subject: [PATCH 09/12] Clarify shim implementation comment. --- src/librustc_mir/shim.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/shim.rs b/src/librustc_mir/shim.rs index 0d13daa264309..c4d66ee7dbd77 100644 --- a/src/librustc_mir/shim.rs +++ b/src/librustc_mir/shim.rs @@ -69,7 +69,8 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> &'tcx // We are generating a call back to our def-id, which the // codegen backend knows to turn to an actual virtual call. ty::InstanceDef::Virtual(def_id, _) | - // ...or we are generating a call to the inner closure defined by #[track_caller] + // ...or we are generating a direct call to our #[track_caller] function which + // requires an implicit caller location that the virtual call won't pass ty::InstanceDef::ReifyShim(def_id) => { build_call_shim( tcx, From d92cef74736aa4503e37ee67806ce5ab3a4eb97a Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Fri, 11 Oct 2019 07:44:01 -0700 Subject: [PATCH 10/12] resolve_for_fn_ptr checks that the instance is an Item before returning shim. --- src/librustc/ty/instance.rs | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/librustc/ty/instance.rs b/src/librustc/ty/instance.rs index 0a86b0b414802..5139c8085a583 100644 --- a/src/librustc/ty/instance.rs +++ b/src/librustc/ty/instance.rs @@ -311,16 +311,18 @@ impl<'tcx> Instance<'tcx> { ) -> Option> { debug!("resolve(def_id={:?}, substs={:?})", def_id, substs); Instance::resolve(tcx, param_env, def_id, substs).map(|resolved| { - let resolved_def = resolved.def_id(); - let codegen_attrs = tcx.codegen_fn_attrs(resolved_def); - if codegen_attrs.flags.contains(CodegenFnAttrFlags::TRACK_CALLER) { - debug!(" => fn pointer created for function with #[track_caller]"); - Instance { - def: InstanceDef::ReifyShim(resolved_def), - substs, - } - } else { - resolved + let has_track_caller = |def| tcx.codegen_fn_attrs(def).flags + .contains(CodegenFnAttrFlags::TRACK_CALLER); + + match resolved.def { + InstanceDef::Item(def_id) if has_track_caller(def_id) => { + debug!(" => fn pointer created for function with #[track_caller]"); + Instance { + def: InstanceDef::ReifyShim(def_id), + substs, + } + }, + _ => resolved, } }) } From 2d5ef8f9ff091aa6e2758958745cc3edfa84f937 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Fri, 11 Oct 2019 07:45:59 -0700 Subject: [PATCH 11/12] Clarify comment about purpose of ReifyShim. --- src/librustc_mir/shim.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/shim.rs b/src/librustc_mir/shim.rs index c4d66ee7dbd77..f532a18072fbd 100644 --- a/src/librustc_mir/shim.rs +++ b/src/librustc_mir/shim.rs @@ -69,8 +69,8 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> &'tcx // We are generating a call back to our def-id, which the // codegen backend knows to turn to an actual virtual call. ty::InstanceDef::Virtual(def_id, _) | - // ...or we are generating a direct call to our #[track_caller] function which - // requires an implicit caller location that the virtual call won't pass + // ...or we are generating a direct call to a function for which indirect calls must be + // codegen'd differently than direct ones (example: #[track_caller]) ty::InstanceDef::ReifyShim(def_id) => { build_call_shim( tcx, From 19f26fafdd6e27847a155a2ba8854c8d4ff61597 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Sat, 12 Oct 2019 14:25:34 -0700 Subject: [PATCH 12/12] Remove the fn pointer #[track_caller] test. The ICE stderr isn't normalizing correctly on some builders. --- .../taking-fn-pointer.rs | 16 -------------- .../taking-fn-pointer.stderr | 21 ------------------- 2 files changed, 37 deletions(-) delete mode 100644 src/test/ui/rfc-2091-track-caller/taking-fn-pointer.rs delete mode 100644 src/test/ui/rfc-2091-track-caller/taking-fn-pointer.stderr diff --git a/src/test/ui/rfc-2091-track-caller/taking-fn-pointer.rs b/src/test/ui/rfc-2091-track-caller/taking-fn-pointer.rs deleted file mode 100644 index 2882d2d83ff81..0000000000000 --- a/src/test/ui/rfc-2091-track-caller/taking-fn-pointer.rs +++ /dev/null @@ -1,16 +0,0 @@ -// failure-status: 101 -// normalize-stderr-test "note: rustc 1.* running on .*" -> "note: rustc VERSION running on TARGET" -// normalize-stderr-test "note: compiler flags: .*" -> "note: compiler flags: FLAGS" - -#![feature(track_caller)] //~ WARN the feature `track_caller` is incomplete - -#[track_caller] -fn f() {} - -fn call_it(x: fn()) { - x(); -} - -fn main() { - call_it(f); -} diff --git a/src/test/ui/rfc-2091-track-caller/taking-fn-pointer.stderr b/src/test/ui/rfc-2091-track-caller/taking-fn-pointer.stderr deleted file mode 100644 index fb625ae116e7c..0000000000000 --- a/src/test/ui/rfc-2091-track-caller/taking-fn-pointer.stderr +++ /dev/null @@ -1,21 +0,0 @@ -warning: the feature `track_caller` is incomplete and may cause the compiler to crash - --> $DIR/taking-fn-pointer.rs:5:12 - | -LL | #![feature(track_caller)] - | ^^^^^^^^^^^^ - | - = note: `#[warn(incomplete_features)]` on by default - -thread 'rustc' panicked at 'index out of bounds: the len is 1 but the index is 1', $SRC_DIR/libcore/slice/mod.rs:LL:COL -note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace. - -error: internal compiler error: unexpected panic - -note: the compiler unexpectedly panicked. this is a bug. - -note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports - -note: rustc VERSION running on TARGET - -note: compiler flags: FLAGS -