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

Implement PartialEq for all function pointers via a shim #108074

Closed
wants to merge 4 commits into from
Closed
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
1 change: 1 addition & 0 deletions compiler/rustc_const_eval/src/interpret/terminator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
| ty::InstanceDef::ReifyShim(..)
| ty::InstanceDef::ClosureOnceShim { .. }
| ty::InstanceDef::FnPtrShim(..)
| ty::InstanceDef::FnPtrEqShim(..)
| ty::InstanceDef::DropGlue(..)
| ty::InstanceDef::CloneShim(..)
| ty::InstanceDef::Item(_) => {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/transform/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
}
CastKind::FnPtrToPtr | CastKind::PtrToPtr => {
if !(op_ty.is_any_ptr() && target_type.is_unsafe_ptr()) {
self.fail(location, "Can't cast {op_ty} into 'Ptr'");
self.fail(location, format!("Can't cast {op_ty} into 'Ptr'"));
}
}
CastKind::FloatToFloat | CastKind::FloatToInt => {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/mir/mono.rs
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,7 @@ impl<'tcx> CodegenUnit<'tcx> {
| InstanceDef::ReifyShim(..)
| InstanceDef::Intrinsic(..)
| InstanceDef::FnPtrShim(..)
| InstanceDef::FnPtrEqShim(..)
| InstanceDef::Virtual(..)
| InstanceDef::ClosureOnceShim { .. }
| InstanceDef::DropGlue(..)
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,7 @@ macro_rules! make_mir_visitor {

ty::InstanceDef::FnPtrShim(_def_id, ty) |
ty::InstanceDef::DropGlue(_def_id, Some(ty)) |
ty::InstanceDef::FnPtrEqShim(_def_id, ty) |
ty::InstanceDef::CloneShim(_def_id, ty) => {
// FIXME(eddyb) use a better `TyContext` here.
self.visit_ty($(& $mutability)? *ty, TyContext::Location(location));
Expand Down
10 changes: 10 additions & 0 deletions compiler/rustc_middle/src/ty/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,11 @@ 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>),

/// `<fn() as PartialEq>::eq` (generated `PartialEq` implementation for `fn()` pointers).
///
/// `DefId` is `PartialEq::eq`.
FnPtrEqShim(DefId, Ty<'tcx>),
}

impl<'tcx> Instance<'tcx> {
Expand Down Expand Up @@ -147,6 +152,7 @@ impl<'tcx> InstanceDef<'tcx> {
InstanceDef::VTableShim(def_id)
| InstanceDef::ReifyShim(def_id)
| InstanceDef::FnPtrShim(def_id, _)
| InstanceDef::FnPtrEqShim(def_id, _)
| InstanceDef::Virtual(def_id, _)
| InstanceDef::Intrinsic(def_id)
| InstanceDef::ClosureOnceShim { call_once: def_id, track_caller: _ }
Expand All @@ -163,6 +169,7 @@ impl<'tcx> InstanceDef<'tcx> {
InstanceDef::VTableShim(..)
| InstanceDef::ReifyShim(..)
| InstanceDef::FnPtrShim(..)
| InstanceDef::FnPtrEqShim(..)
| InstanceDef::Virtual(..)
| InstanceDef::Intrinsic(..)
| InstanceDef::ClosureOnceShim { .. }
Expand All @@ -178,6 +185,7 @@ impl<'tcx> InstanceDef<'tcx> {
InstanceDef::VTableShim(def_id)
| InstanceDef::ReifyShim(def_id)
| InstanceDef::FnPtrShim(def_id, _)
| InstanceDef::FnPtrEqShim(def_id, _)
| InstanceDef::Virtual(def_id, _)
| InstanceDef::Intrinsic(def_id)
| InstanceDef::ClosureOnceShim { call_once: def_id, track_caller: _ }
Expand Down Expand Up @@ -265,6 +273,7 @@ impl<'tcx> InstanceDef<'tcx> {
match *self {
InstanceDef::CloneShim(..)
| InstanceDef::FnPtrShim(..)
| InstanceDef::FnPtrEqShim(..)
| InstanceDef::DropGlue(_, Some(_)) => false,
InstanceDef::ClosureOnceShim { .. }
| InstanceDef::DropGlue(..)
Expand Down Expand Up @@ -298,6 +307,7 @@ fn fmt_instance(
InstanceDef::Intrinsic(_) => write!(f, " - intrinsic"),
InstanceDef::Virtual(_, num) => write!(f, " - virtual#{}", num),
InstanceDef::FnPtrShim(_, ty) => write!(f, " - shim({})", ty),
InstanceDef::FnPtrEqShim(_, ty) => write!(f, " - shim({ty})"),
InstanceDef::ClosureOnceShim { .. } => write!(f, " - shim"),
InstanceDef::DropGlue(_, None) => write!(f, " - shim(None)"),
InstanceDef::DropGlue(_, Some(ty)) => write!(f, " - shim(Some({}))", ty),
Expand Down
11 changes: 11 additions & 0 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2293,6 +2293,7 @@ impl<'tcx> TyCtxt<'tcx> {
ty::InstanceDef::VTableShim(..)
| ty::InstanceDef::ReifyShim(..)
| ty::InstanceDef::Intrinsic(..)
| ty::InstanceDef::FnPtrEqShim(..)
| ty::InstanceDef::FnPtrShim(..)
| ty::InstanceDef::Virtual(..)
| ty::InstanceDef::ClosureOnceShim { .. }
Expand All @@ -2301,6 +2302,16 @@ impl<'tcx> TyCtxt<'tcx> {
}
}

pub fn trait_method(self, trait_def_id: DefId, method_name: Symbol) -> DefId {
// The unhygienic comparison here is acceptable because this is only
// used on known traits.
self.associated_items(trait_def_id)
.filter_by_name_unhygienic(method_name)
.find(|item| item.kind == ty::AssocKind::Fn)
.expect("trait method not found")
.def_id
}

// FIXME(@lcnr): Remove this function.
pub fn get_attrs_unchecked(self, did: DefId) -> &'tcx [ast::Attribute] {
if let Some(did) = did.as_local() {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2088,6 +2088,7 @@ impl<'tcx> Ty<'tcx> {
}
}

#[track_caller]
pub fn fn_sig(self, tcx: TyCtxt<'tcx>) -> PolyFnSig<'tcx> {
match self.kind() {
FnDef(def_id, substs) => tcx.fn_sig(*def_id).subst(tcx, substs),
Expand Down
12 changes: 3 additions & 9 deletions compiler/rustc_mir_build/src/build/matches/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -837,15 +837,9 @@ fn trait_method<'tcx>(
method_name: Symbol,
substs: impl IntoIterator<Item = impl Into<GenericArg<'tcx>>>,
) -> ConstantKind<'tcx> {
Copy link
Contributor

Choose a reason for hiding this comment

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

why does this not use lang_items?

// The unhygienic comparison here is acceptable because this is only
// used on known traits.
let item = tcx
.associated_items(trait_def_id)
.filter_by_name_unhygienic(method_name)
.find(|item| item.kind == ty::AssocKind::Fn)
.expect("trait method not found");

let method_ty = tcx.mk_fn_def(item.def_id, substs);
let def_id = tcx.trait_method(trait_def_id, method_name);

let method_ty = tcx.mk_fn_def(def_id, substs);

ConstantKind::zero_sized(method_ty)
}
1 change: 1 addition & 0 deletions compiler/rustc_mir_transform/src/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ impl<'tcx> Inliner<'tcx> {
InstanceDef::VTableShim(_)
| InstanceDef::ReifyShim(_)
| InstanceDef::FnPtrShim(..)
| InstanceDef::FnPtrEqShim(..)
| InstanceDef::ClosureOnceShim { .. }
| InstanceDef::DropGlue(..)
| InstanceDef::CloneShim(..) => return Ok(()),
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_transform/src/inline/cycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ pub(crate) fn mir_callgraph_reachable<'tcx>(
InstanceDef::VTableShim(_)
| InstanceDef::ReifyShim(_)
| InstanceDef::FnPtrShim(..)
| InstanceDef::FnPtrEqShim(..)
| InstanceDef::ClosureOnceShim { .. }
| InstanceDef::CloneShim(..) => {}
InstanceDef::DropGlue(..) => {
Expand Down
94 changes: 94 additions & 0 deletions compiler/rustc_mir_transform/src/shim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> Body<'

build_call_shim(tcx, instance, Some(adjustment), CallKind::Indirect(ty))
}
ty::InstanceDef::FnPtrEqShim(def_id, ty) => {
build_partial_eq_shim(tcx, instance, def_id, ty)
}
// We are generating a call back to our def-id, which the
// codegen backend knows to turn to an actual call, be it
// a virtual call, or a direct call to a function for which
Expand Down Expand Up @@ -791,6 +794,97 @@ fn build_call_shim<'tcx>(
body
}

/// Builds a "call" shim for `PartialEq::eq`.
#[instrument(level = "debug", skip(tcx), ret)]
fn build_partial_eq_shim<'tcx>(
tcx: TyCtxt<'tcx>,
instance: ty::InstanceDef<'tcx>,
partial_eq_eq: DefId,
ty: Ty<'tcx>,
) -> Body<'tcx> {
let span = tcx.def_span(partial_eq_eq);

let source_info = SourceInfo::outermost(span);

let mut statements = vec![];

let sig = tcx.fn_sig(partial_eq_eq);
let sig = sig.map_bound(|sig| tcx.erase_late_bound_regions(sig));
let sig = sig.subst(tcx, &[ty.into(), ty.into()]);
let mut local_decls = local_decls_for_sig(&sig, span);
let raw_unit = tcx.mk_imm_ptr(tcx.types.unit);

let mut cast = |place| {
let tmp = local_decls.push(LocalDecl::new(raw_unit, span).immutable());
// let tmp = *place as *const ();
statements.push(Statement {
source_info,
kind: StatementKind::Assign(Box::new((
tmp.into(),
Rvalue::Cast(
CastKind::FnPtrToPtr,
Operand::Move(tcx.mk_place_deref(place)),
raw_unit,
),
))),
});
// let tmp2 = &tmp;
let tmp2 = local_decls.push(
LocalDecl::new(tcx.mk_imm_ref(tcx.lifetimes.re_erased, raw_unit), span).immutable(),
);
statements.push(Statement {
source_info,
kind: StatementKind::Assign(Box::new((
tmp2.into(),
Rvalue::Ref(tcx.lifetimes.re_erased, BorrowKind::Shared, tmp.into()),
))),
});
Operand::Move(tmp2.into())
};

// Cast both fn ptr arguments to `*const ()`
let a = cast(Local::new(1).into());
let b = cast(Local::new(2).into());

let callee = Operand::Constant(Box::new(Constant {
span,
user_ty: None,
literal: ConstantKind::zero_sized(
tcx.bound_type_of(partial_eq_eq).subst(tcx, &[raw_unit.into(), raw_unit.into()]),
),
}));

let mut blocks = IndexVec::with_capacity(2);
let block = |blocks: &mut IndexVec<_, _>, statements, kind, is_cleanup| {
blocks.push(BasicBlockData {
statements,
terminator: Some(Terminator { source_info, kind }),
is_cleanup,
})
};

// BB #0
block(
&mut blocks,
statements,
TerminatorKind::Call {
func: callee,
args: vec![a, b],
destination: Place::return_place(),
target: Some(BasicBlock::new(1)),
cleanup: None,
from_hir_call: true,
fn_span: span,
},
false,
);

// BB #1 - return
block(&mut blocks, vec![], TerminatorKind::Return, false);

new_body(MirSource::from_instance(instance), blocks, local_decls, sig.inputs().len(), span)
}

pub fn build_adt_ctor(tcx: TyCtxt<'_>, ctor_id: DefId) -> Body<'_> {
debug_assert!(tcx.is_constructor(ctor_id));

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_monomorphize/src/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -975,6 +975,7 @@ fn visit_instance_use<'tcx>(
| ty::InstanceDef::ClosureOnceShim { .. }
| ty::InstanceDef::Item(..)
| ty::InstanceDef::FnPtrShim(..)
| ty::InstanceDef::FnPtrEqShim(..)
| ty::InstanceDef::CloneShim(..) => {
output.push(create_fn_mono_item(tcx, instance, source));
}
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_monomorphize/src/partitioning/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@ fn characteristic_def_id_of_mono_item<'tcx>(
ty::InstanceDef::VTableShim(..)
| ty::InstanceDef::ReifyShim(..)
| ty::InstanceDef::FnPtrShim(..)
| ty::InstanceDef::FnPtrEqShim(..)
| ty::InstanceDef::ClosureOnceShim { .. }
| ty::InstanceDef::Intrinsic(..)
| ty::InstanceDef::DropGlue(..)
Expand Down Expand Up @@ -428,6 +429,7 @@ fn mono_item_visibility<'tcx>(
InstanceDef::VTableShim(..)
| InstanceDef::ReifyShim(..)
| InstanceDef::FnPtrShim(..)
| InstanceDef::FnPtrEqShim(..)
| InstanceDef::Virtual(..)
| InstanceDef::Intrinsic(..)
| InstanceDef::ClosureOnceShim { .. }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,12 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
// types have builtin support for `Clone`.
let clone_conditions = self.copy_clone_conditions(obligation);
self.assemble_builtin_bound_candidates(clone_conditions, &mut candidates);
}

if lang_items.gen_trait() == Some(def_id) {
} else if lang_items.gen_trait() == Some(def_id) {
self.assemble_generator_candidates(obligation, &mut candidates);
} else if lang_items.future_trait() == Some(def_id) {
self.assemble_future_candidates(obligation, &mut candidates);
} else if lang_items.eq_trait() == Some(def_id) {
self.assemble_eq_candidates(obligation, &mut candidates);
}

self.assemble_closure_candidates(obligation, &mut candidates);
Expand Down Expand Up @@ -236,6 +236,21 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
}
}

fn assemble_eq_candidates(
&mut self,
obligation: &TraitObligation<'tcx>,
candidates: &mut SelectionCandidateSet<'tcx>,
) {
let [self_ty, other_ty] = &obligation.predicate.skip_binder().trait_ref.substs[..] else {
span_bug!(obligation.cause.span, "PartialEq has two generic params: `Self` and `Other`, but got {obligation:#?}")
};
if let ty::FnPtr(..) = self_ty.expect_ty().kind() {
if let ty::FnPtr(..) = other_ty.expect_ty().kind() {
candidates.vec.push(BuiltinCandidate { has_nested: false });
}
}
}

/// Checks for the artificial impl that the compiler will create for an obligation like `X :
/// FnMut<..>` where `X` is a closure type.
///
Expand Down
16 changes: 16 additions & 0 deletions compiler/rustc_ty_utils/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,22 @@ fn resolve_associated_item<'tcx>(
let substs = tcx.erase_regions(rcvr_substs);
Some(ty::Instance::new(trait_item_id, substs))
}
} else if Some(trait_ref.def_id) == tcx.lang_items().eq_trait() {
// FIXME(eddyb) use lang items for methods instead of names.
let name = tcx.item_name(trait_item_id);
if name == sym::eq {
let self_ty = trait_ref.self_ty();
Some(Instance {
def: ty::InstanceDef::FnPtrEqShim(trait_item_id, self_ty),
substs: rcvr_substs,
})
} else {
assert_eq!(name, sym::ne);

// Use the default `fn ne` from `trait PartialEq`.
let substs = tcx.erase_regions(rcvr_substs);
Some(ty::Instance::new(trait_item_id, substs))
}
} else {
None
}
Expand Down
33 changes: 33 additions & 0 deletions tests/ui/fn/fn-ptr-eq-op.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
fn foo(_: &()) {}

fn main() {
let x: for<'a> fn(&'a ()) = foo;
let y: for<'a> fn(&'a ()) = foo;
x == y; //~ ERROR: `==` cannot be applied

let x: for<'a> fn(&'a ()) = foo;
let y: fn(&()) = foo;
x == y; //~ ERROR: `==` cannot be applied

let x: fn(&()) = foo;
let y: for<'a> fn(&'a ()) = foo;
x == y; //~ ERROR: `==` cannot be applied

let x: for<'a> fn(&'a ()) = foo;
let y: for<'a> fn(&'a ()) = foo;
x == foo; //~ ERROR: `==` cannot be applied
y == foo; //~ ERROR: `==` cannot be applied
foo == x; //~ ERROR: `==` cannot be applied
//~^ ERROR mismatched types
foo == y; //~ ERROR: `==` cannot be applied
//~^ ERROR mismatched types

let x: for<'a> fn(&'a ()) = foo;
let y: fn(&'static ()) = foo;
x == y; //~ ERROR: `==` cannot be applied

let x: fn(&'static ()) = foo;
let y: fn(&'static ()) = foo;
foo == x; //~ ERROR: `==` cannot be applied
//~^ ERROR mismatched types
}
Loading