Skip to content

do not promote comparing function pointers #54702

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.

Sign up for GitHub

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

Merged
bors merged 2 commits into from
Oct 3, 2018
Merged

do not promote comparing function pointers #54702

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1397836
do not promote comparing function pointers
RalfJung Sep 30, 2018
4cbfc93
also compile-fail test fn ptr comparison promotion
RalfJung Sep 30, 2018
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
2 changes: 1 addition & 1 deletion src/librustc_mir/transform/qualify_consts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -750,7 +750,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
}

Rvalue::BinaryOp(op, ref lhs, _) => {
if let ty::RawPtr(_) = lhs.ty(self.mir, self.tcx).sty {
if let ty::RawPtr(_) | ty::FnPtr(..) = lhs.ty(self.mir, self.tcx).sty {
assert!(op == BinOp::Eq || op == BinOp::Ne ||
op == BinOp::Le || op == BinOp::Lt ||
op == BinOp::Ge || op == BinOp::Gt ||
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_passes/rvalue_promotion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ fn check_expr_kind<'a, 'tcx>(
return NotPromotable;
}
match v.tables.node_id_to_type(lhs.hir_id).sty {
ty::RawPtr(_) => {
ty::RawPtr(_) | ty::FnPtr(..) => {
assert!(op.node == hir::BinOpKind::Eq || op.node == hir::BinOpKind::Ne ||
op.node == hir::BinOpKind::Le || op.node == hir::BinOpKind::Lt ||
op.node == hir::BinOpKind::Ge || op.node == hir::BinOpKind::Gt);
Expand Down
8 changes: 8 additions & 0 deletions src/test/run-pass/issues/issue-54696.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// run-pass

fn main() {
// We shouldn't promote this
&(main as fn() == main as fn());
// Also check nested case
&(&(main as fn()) == &(main as fn()));
}
Copy link
Member

@Mark-Simulacrum Mark-Simulacrum Sep 30, 2018

Choose a reason for hiding this comment

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

How does this test verify lack of promotion? Shouldn't we instead have a compile-fail test?

Copy link
Contributor

@arielb1 arielb1 Sep 30, 2018

Choose a reason for hiding this comment

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

The promotion causes crashes, because function pointer equality can't be done in constants

Copy link
Member Author

@RalfJung RalfJung Sep 30, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I can also add a compile-fail test if you want -- though there is no fundamental reason we shouldn't want to promote this in the future, so that test wouldn't be "normative". OTOH, making this code run (and not emit a trap) is "normative".