Skip to content

Commit

Permalink
Auto merge of #108080 - oli-obk:FnPtr-trait, r=lcnr
Browse files Browse the repository at this point in the history
Add a builtin `FnPtr` trait that is implemented for all function pointers

r? `@ghost`

Rebased version of #99531 (plus adjustments mentioned in the PR).

If perf is happy with this version, I would like to land it, even if the diagnostics fix in 9df8e1befb5031a5bf9d8dfe25170620642d3c59 only works for `FnPtr` specifically, and does not generally improve blanket impls.
  • Loading branch information
bors committed Mar 28, 2023
2 parents 6066037 + 5ae6caa commit bf57e8a
Show file tree
Hide file tree
Showing 23 changed files with 476 additions and 180 deletions.
1 change: 1 addition & 0 deletions compiler/rustc_const_eval/src/interpret/terminator.rs
Expand Up @@ -382,6 +382,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
| ty::InstanceDef::FnPtrShim(..)
| ty::InstanceDef::DropGlue(..)
| ty::InstanceDef::CloneShim(..)
| ty::InstanceDef::FnPtrAddrShim(..)
| ty::InstanceDef::Item(_) => {
// We need MIR for this fn
let Some((body, instance)) =
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_hir/src/lang_items.rs
Expand Up @@ -166,6 +166,9 @@ language_item_table! {

Freeze, sym::freeze, freeze_trait, Target::Trait, GenericRequirement::Exact(0);

FnPtrTrait, sym::fn_ptr_trait, fn_ptr_trait, Target::Trait, GenericRequirement::Exact(0);
FnPtrAddr, sym::fn_ptr_addr, fn_ptr_addr, Target::Method(MethodKind::Trait { body: false }), GenericRequirement::None;

Drop, sym::drop, drop_trait, Target::Trait, GenericRequirement::None;
Destruct, sym::destruct, destruct_trait, Target::Trait, GenericRequirement::None;

Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_middle/src/mir/mono.rs
Expand Up @@ -381,7 +381,8 @@ impl<'tcx> CodegenUnit<'tcx> {
| InstanceDef::Virtual(..)
| InstanceDef::ClosureOnceShim { .. }
| InstanceDef::DropGlue(..)
| InstanceDef::CloneShim(..) => None,
| InstanceDef::CloneShim(..)
| InstanceDef::FnPtrAddrShim(..) => None,
}
}
MonoItem::Static(def_id) => def_id.as_local().map(Idx::index),
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_middle/src/mir/visit.rs
Expand Up @@ -340,7 +340,8 @@ macro_rules! make_mir_visitor {

ty::InstanceDef::FnPtrShim(_def_id, ty) |
ty::InstanceDef::DropGlue(_def_id, Some(ty)) |
ty::InstanceDef::CloneShim(_def_id, ty) => {
ty::InstanceDef::CloneShim(_def_id, ty) |
ty::InstanceDef::FnPtrAddrShim(_def_id, ty) => {
// FIXME(eddyb) use a better `TyContext` here.
self.visit_ty($(& $mutability)? *ty, TyContext::Location(location));
}
Expand Down
18 changes: 15 additions & 3 deletions compiler/rustc_middle/src/ty/instance.rs
Expand Up @@ -96,6 +96,13 @@ pub enum InstanceDef<'tcx> {
///
/// The `DefId` is for `Clone::clone`, the `Ty` is the type `T` with the builtin `Clone` impl.
CloneShim(DefId, Ty<'tcx>),

/// Compiler-generated `<T as FnPtr>::addr` implementation.
///
/// Automatically generated for all potentially higher-ranked `fn(I) -> R` types.
///
/// The `DefId` is for `FnPtr::addr`, the `Ty` is the type `T`.
FnPtrAddrShim(DefId, Ty<'tcx>),
}

impl<'tcx> Instance<'tcx> {
Expand Down Expand Up @@ -151,7 +158,8 @@ impl<'tcx> InstanceDef<'tcx> {
| InstanceDef::Intrinsic(def_id)
| InstanceDef::ClosureOnceShim { call_once: def_id, track_caller: _ }
| InstanceDef::DropGlue(def_id, _)
| InstanceDef::CloneShim(def_id, _) => def_id,
| InstanceDef::CloneShim(def_id, _)
| InstanceDef::FnPtrAddrShim(def_id, _) => def_id,
}
}

Expand All @@ -167,7 +175,8 @@ impl<'tcx> InstanceDef<'tcx> {
| InstanceDef::Intrinsic(..)
| InstanceDef::ClosureOnceShim { .. }
| InstanceDef::DropGlue(..)
| InstanceDef::CloneShim(..) => None,
| InstanceDef::CloneShim(..)
| InstanceDef::FnPtrAddrShim(..) => None,
}
}

Expand All @@ -182,7 +191,8 @@ impl<'tcx> InstanceDef<'tcx> {
| InstanceDef::Intrinsic(def_id)
| InstanceDef::ClosureOnceShim { call_once: def_id, track_caller: _ }
| InstanceDef::DropGlue(def_id, _)
| InstanceDef::CloneShim(def_id, _) => ty::WithOptConstParam::unknown(def_id),
| InstanceDef::CloneShim(def_id, _)
| InstanceDef::FnPtrAddrShim(def_id, _) => ty::WithOptConstParam::unknown(def_id),
}
}

Expand Down Expand Up @@ -268,6 +278,7 @@ impl<'tcx> InstanceDef<'tcx> {
pub fn has_polymorphic_mir_body(&self) -> bool {
match *self {
InstanceDef::CloneShim(..)
| InstanceDef::FnPtrAddrShim(..)
| InstanceDef::FnPtrShim(..)
| InstanceDef::DropGlue(_, Some(_)) => false,
InstanceDef::ClosureOnceShim { .. }
Expand Down Expand Up @@ -306,6 +317,7 @@ fn fmt_instance(
InstanceDef::DropGlue(_, None) => write!(f, " - shim(None)"),
InstanceDef::DropGlue(_, Some(ty)) => write!(f, " - shim(Some({}))", ty),
InstanceDef::CloneShim(_, ty) => write!(f, " - shim({})", ty),
InstanceDef::FnPtrAddrShim(_, ty) => write!(f, " - shim({})", ty),
}
}

Expand Down
39 changes: 10 additions & 29 deletions compiler/rustc_middle/src/ty/mod.rs
Expand Up @@ -2279,76 +2279,56 @@ impl<'tcx> TyCtxt<'tcx> {

/// Returns `true` if the impls are the same polarity and the trait either
/// has no items or is annotated `#[marker]` and prevents item overrides.
#[instrument(level = "debug", skip(self), ret)]
pub fn impls_are_allowed_to_overlap(
self,
def_id1: DefId,
def_id2: DefId,
) -> Option<ImplOverlapKind> {
let impl_trait_ref1 = self.impl_trait_ref(def_id1);
let impl_trait_ref2 = self.impl_trait_ref(def_id2);
// If either trait impl references an error, they're allowed to overlap,
// as one of them essentially doesn't exist.
if self.impl_trait_ref(def_id1).map_or(false, |tr| tr.subst_identity().references_error())
|| self
.impl_trait_ref(def_id2)
.map_or(false, |tr| tr.subst_identity().references_error())
if impl_trait_ref1.map_or(false, |tr| tr.subst_identity().references_error())
|| impl_trait_ref2.map_or(false, |tr| tr.subst_identity().references_error())
{
return Some(ImplOverlapKind::Permitted { marker: false });
}

match (self.impl_polarity(def_id1), self.impl_polarity(def_id2)) {
(ImplPolarity::Reservation, _) | (_, ImplPolarity::Reservation) => {
// `#[rustc_reservation_impl]` impls don't overlap with anything
debug!(
"impls_are_allowed_to_overlap({:?}, {:?}) = Some(Permitted) (reservations)",
def_id1, def_id2
);
return Some(ImplOverlapKind::Permitted { marker: false });
}
(ImplPolarity::Positive, ImplPolarity::Negative)
| (ImplPolarity::Negative, ImplPolarity::Positive) => {
// `impl AutoTrait for Type` + `impl !AutoTrait for Type`
debug!(
"impls_are_allowed_to_overlap({:?}, {:?}) - None (differing polarities)",
def_id1, def_id2
);
return None;
}
(ImplPolarity::Positive, ImplPolarity::Positive)
| (ImplPolarity::Negative, ImplPolarity::Negative) => {}
};

let is_marker_overlap = {
let is_marker_impl = |def_id: DefId| -> bool {
let trait_ref = self.impl_trait_ref(def_id);
let is_marker_impl = |trait_ref: Option<EarlyBinder<TraitRef<'_>>>| -> bool {
trait_ref.map_or(false, |tr| self.trait_def(tr.skip_binder().def_id).is_marker)
};
is_marker_impl(def_id1) && is_marker_impl(def_id2)
is_marker_impl(impl_trait_ref1) && is_marker_impl(impl_trait_ref2)
};

if is_marker_overlap {
debug!(
"impls_are_allowed_to_overlap({:?}, {:?}) = Some(Permitted) (marker overlap)",
def_id1, def_id2
);
Some(ImplOverlapKind::Permitted { marker: true })
} else {
if let Some(self_ty1) = self.issue33140_self_ty(def_id1) {
if let Some(self_ty2) = self.issue33140_self_ty(def_id2) {
if self_ty1 == self_ty2 {
debug!(
"impls_are_allowed_to_overlap({:?}, {:?}) - issue #33140 HACK",
def_id1, def_id2
);
return Some(ImplOverlapKind::Issue33140);
} else {
debug!(
"impls_are_allowed_to_overlap({:?}, {:?}) - found {:?} != {:?}",
def_id1, def_id2, self_ty1, self_ty2
);
debug!("found {self_ty1:?} != {self_ty2:?}");
}
}
}

debug!("impls_are_allowed_to_overlap({:?}, {:?}) = None", def_id1, def_id2);
None
}
}
Expand Down Expand Up @@ -2405,7 +2385,8 @@ impl<'tcx> TyCtxt<'tcx> {
| ty::InstanceDef::Virtual(..)
| ty::InstanceDef::ClosureOnceShim { .. }
| ty::InstanceDef::DropGlue(..)
| ty::InstanceDef::CloneShim(..) => self.mir_shims(instance),
| ty::InstanceDef::CloneShim(..)
| ty::InstanceDef::FnPtrAddrShim(..) => self.mir_shims(instance),
}
}

Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_mir_transform/src/inline.rs
Expand Up @@ -270,7 +270,8 @@ impl<'tcx> Inliner<'tcx> {
| InstanceDef::FnPtrShim(..)
| InstanceDef::ClosureOnceShim { .. }
| InstanceDef::DropGlue(..)
| InstanceDef::CloneShim(..) => return Ok(()),
| InstanceDef::CloneShim(..)
| InstanceDef::FnPtrAddrShim(..) => return Ok(()),
}

if self.tcx.is_constructor(callee_def_id) {
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_mir_transform/src/inline/cycle.rs
Expand Up @@ -84,6 +84,9 @@ pub(crate) fn mir_callgraph_reachable<'tcx>(
| InstanceDef::FnPtrShim(..)
| InstanceDef::ClosureOnceShim { .. }
| InstanceDef::CloneShim(..) => {}

// This shim does not call any other functions, thus there can be no recursion.
InstanceDef::FnPtrAddrShim(..) => continue,
InstanceDef::DropGlue(..) => {
// FIXME: A not fully substituted drop shim can cause ICEs if one attempts to
// have its MIR built. Likely oli-obk just screwed up the `ParamEnv`s, so this
Expand Down
37 changes: 37 additions & 0 deletions compiler/rustc_mir_transform/src/shim.rs
Expand Up @@ -77,6 +77,7 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> Body<'
build_drop_shim(tcx, def_id, ty)
}
ty::InstanceDef::CloneShim(def_id, ty) => build_clone_shim(tcx, def_id, ty),
ty::InstanceDef::FnPtrAddrShim(def_id, ty) => build_fn_ptr_addr_shim(tcx, def_id, ty),
ty::InstanceDef::Virtual(..) => {
bug!("InstanceDef::Virtual ({:?}) is for direct calls only", instance)
}
Expand Down Expand Up @@ -861,3 +862,39 @@ pub fn build_adt_ctor(tcx: TyCtxt<'_>, ctor_id: DefId) -> Body<'_> {

body
}

/// ```ignore (pseudo-impl)
/// impl FnPtr for fn(u32) {
/// fn addr(self) -> usize {
/// self as usize
/// }
/// }
/// ```
fn build_fn_ptr_addr_shim<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, self_ty: Ty<'tcx>) -> Body<'tcx> {
assert!(matches!(self_ty.kind(), ty::FnPtr(..)), "expected fn ptr, found {self_ty}");
let span = tcx.def_span(def_id);
let Some(sig) = tcx.fn_sig(def_id).subst(tcx, &[self_ty.into()]).no_bound_vars() else {
span_bug!(span, "FnPtr::addr with bound vars for `{self_ty}`");
};
let locals = local_decls_for_sig(&sig, span);

let source_info = SourceInfo::outermost(span);
// FIXME: use `expose_addr` once we figure out whether function pointers have meaningful provenance.
let rvalue = Rvalue::Cast(
CastKind::FnPtrToPtr,
Operand::Move(Place::from(Local::new(1))),
tcx.mk_imm_ptr(tcx.types.unit),
);
let stmt = Statement {
source_info,
kind: StatementKind::Assign(Box::new((Place::return_place(), rvalue))),
};
let statements = vec![stmt];
let start_block = BasicBlockData {
statements,
terminator: Some(Terminator { source_info, kind: TerminatorKind::Return }),
is_cleanup: false,
};
let source = MirSource::from_instance(ty::InstanceDef::FnPtrAddrShim(def_id, self_ty));
new_body(source, IndexVec::from_elem_n(start_block, 1), locals, sig.inputs().len(), span)
}
3 changes: 2 additions & 1 deletion compiler/rustc_monomorphize/src/collector.rs
Expand Up @@ -974,7 +974,8 @@ fn visit_instance_use<'tcx>(
| ty::InstanceDef::ClosureOnceShim { .. }
| ty::InstanceDef::Item(..)
| ty::InstanceDef::FnPtrShim(..)
| ty::InstanceDef::CloneShim(..) => {
| ty::InstanceDef::CloneShim(..)
| ty::InstanceDef::FnPtrAddrShim(..) => {
output.push(create_fn_mono_item(tcx, instance, source));
}
}
Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_monomorphize/src/partitioning/default.rs
Expand Up @@ -278,7 +278,8 @@ fn characteristic_def_id_of_mono_item<'tcx>(
| ty::InstanceDef::Intrinsic(..)
| ty::InstanceDef::DropGlue(..)
| ty::InstanceDef::Virtual(..)
| ty::InstanceDef::CloneShim(..) => return None,
| ty::InstanceDef::CloneShim(..)
| ty::InstanceDef::FnPtrAddrShim(..) => return None,
};

// If this is a method, we want to put it into the same module as
Expand Down Expand Up @@ -432,7 +433,8 @@ fn mono_item_visibility<'tcx>(
| InstanceDef::Intrinsic(..)
| InstanceDef::ClosureOnceShim { .. }
| InstanceDef::DropGlue(..)
| InstanceDef::CloneShim(..) => return Visibility::Hidden,
| InstanceDef::CloneShim(..)
| InstanceDef::FnPtrAddrShim(..) => return Visibility::Hidden,
};

// The `start_fn` lang item is actually a monomorphized instance of a
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_span/src/symbol.rs
Expand Up @@ -722,6 +722,8 @@ symbols! {
fn_mut,
fn_once,
fn_once_output,
fn_ptr_addr,
fn_ptr_trait,
forbid,
forget,
format,
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_trait_selection/src/solve/assembly.rs
Expand Up @@ -153,6 +153,12 @@ pub(super) trait GoalKind<'tcx>: TypeFoldable<TyCtxt<'tcx>> + Copy + Eq {
goal: Goal<'tcx, Self>,
) -> QueryResult<'tcx>;

// A type is a `FnPtr` if it is of `FnPtr` type.
fn consider_builtin_fn_ptr_trait_candidate(
ecx: &mut EvalCtxt<'_, 'tcx>,
goal: Goal<'tcx, Self>,
) -> QueryResult<'tcx>;

// A callable type (a closure, fn def, or fn ptr) is known to implement the `Fn<A>`
// family of traits where `A` is given by the signature of the type.
fn consider_builtin_fn_trait_candidates(
Expand Down Expand Up @@ -331,6 +337,8 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
G::consider_builtin_copy_clone_candidate(self, goal)
} else if lang_items.pointer_like() == Some(trait_def_id) {
G::consider_builtin_pointer_like_candidate(self, goal)
} else if lang_items.fn_ptr_trait() == Some(trait_def_id) {
G::consider_builtin_fn_ptr_trait_candidate(self, goal)
} else if let Some(kind) = self.tcx().fn_trait_kind_from_def_id(trait_def_id) {
G::consider_builtin_fn_trait_candidates(self, goal, kind)
} else if lang_items.tuple_trait() == Some(trait_def_id) {
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_trait_selection/src/solve/project_goals.rs
Expand Up @@ -261,6 +261,13 @@ impl<'tcx> assembly::GoalKind<'tcx> for ProjectionPredicate<'tcx> {
bug!("`PointerLike` does not have an associated type: {:?}", goal);
}

fn consider_builtin_fn_ptr_trait_candidate(
_ecx: &mut EvalCtxt<'_, 'tcx>,
goal: Goal<'tcx, Self>,
) -> QueryResult<'tcx> {
bug!("`FnPtr` does not have an associated type: {:?}", goal);
}

fn consider_builtin_fn_trait_candidates(
ecx: &mut EvalCtxt<'_, 'tcx>,
goal: Goal<'tcx, Self>,
Expand Down
16 changes: 13 additions & 3 deletions compiler/rustc_trait_selection/src/solve/trait_goals.rs
Expand Up @@ -222,9 +222,8 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> {
let self_ty = tcx.erase_regions(goal.predicate.self_ty());

if let Ok(layout) = tcx.layout_of(goal.param_env.and(self_ty))
&& let usize_layout = tcx.layout_of(ty::ParamEnv::empty().and(tcx.types.usize)).unwrap().layout
&& layout.layout.size() == usize_layout.size()
&& layout.layout.align().abi == usize_layout.align().abi
&& layout.layout.size() == tcx.data_layout.pointer_size
&& layout.layout.align().abi == tcx.data_layout.pointer_align.abi
{
// FIXME: We could make this faster by making a no-constraints response
ecx.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
Expand All @@ -233,6 +232,17 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> {
}
}

fn consider_builtin_fn_ptr_trait_candidate(
ecx: &mut EvalCtxt<'_, 'tcx>,
goal: Goal<'tcx, Self>,
) -> QueryResult<'tcx> {
if let ty::FnPtr(..) = goal.predicate.self_ty().kind() {
ecx.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
} else {
Err(NoSolution)
}
}

fn consider_builtin_fn_trait_candidates(
ecx: &mut EvalCtxt<'_, 'tcx>,
goal: Goal<'tcx, Self>,
Expand Down
13 changes: 13 additions & 0 deletions compiler/rustc_trait_selection/src/traits/coherence.rs
Expand Up @@ -411,11 +411,24 @@ fn resolve_negative_obligation<'tcx>(
infcx.resolve_regions(&outlives_env).is_empty()
}

/// Returns whether all impls which would apply to the `trait_ref`
/// e.g. `Ty: Trait<Arg>` are already known in the local crate.
///
/// This both checks whether any downstream or sibling crates could
/// implement it and whether an upstream crate can add this impl
/// without breaking backwards compatibility.
#[instrument(level = "debug", skip(tcx), ret)]
pub fn trait_ref_is_knowable<'tcx>(
tcx: TyCtxt<'tcx>,
trait_ref: ty::TraitRef<'tcx>,
) -> Result<(), Conflict> {
if Some(trait_ref.def_id) == tcx.lang_items().fn_ptr_trait() {
// The only types implementing `FnPtr` are function pointers,
// so if there's no impl of `FnPtr` in the current crate,
// then such an impl will never be added in the future.
return Ok(());
}

if orphan_check_trait_ref(trait_ref, InCrate::Remote).is_ok() {
// A downstream or cousin crate is allowed to implement some
// substitution of this trait-ref.
Expand Down

0 comments on commit bf57e8a

Please sign in to comment.