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

do not promote comparing function pointers #54702

Merged
merged 2 commits into from
Oct 3, 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
2 changes: 1 addition & 1 deletion src/librustc_mir/transform/qualify_consts.rs
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
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
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

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

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

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".

15 changes: 13 additions & 2 deletions src/test/ui/consts/const-eval/promoted_raw_ptr_ops.nll.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ error[E0716]: temporary value dropped while borrowed
|
LL | let y: &'static usize = &(&1 as *const i32 as usize + 1); //~ ERROR does not live long enough
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ creates a temporary which is freed while still in use
LL | let z: &'static i32 = &(unsafe { *(42 as *const i32) }); //~ ERROR does not live long enough
...
LL | }
| - temporary value is freed at the end of this statement
|
Expand All @@ -25,11 +25,22 @@ error[E0716]: temporary value dropped while borrowed
|
LL | let z: &'static i32 = &(unsafe { *(42 as *const i32) }); //~ ERROR does not live long enough
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ creates a temporary which is freed while still in use
LL | let a: &'static bool = &(main as fn() == main as fn()); //~ ERROR does not live long enough
LL | }
| - temporary value is freed at the end of this statement
|
= note: borrowed value must be valid for the static lifetime...

error[E0716]: temporary value dropped while borrowed
--> $DIR/promoted_raw_ptr_ops.rs:18:29
|
LL | let a: &'static bool = &(main as fn() == main as fn()); //~ ERROR does not live long enough
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ creates a temporary which is freed while still in use
LL | }
| - temporary value is freed at the end of this statement
|
= note: borrowed value must be valid for the static lifetime...

error: aborting due to 3 previous errors
error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0716`.
1 change: 1 addition & 0 deletions src/test/ui/consts/const-eval/promoted_raw_ptr_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,5 @@ fn main() {
//~^ ERROR does not live long enough
let y: &'static usize = &(&1 as *const i32 as usize + 1); //~ ERROR does not live long enough
let z: &'static i32 = &(unsafe { *(42 as *const i32) }); //~ ERROR does not live long enough
let a: &'static bool = &(main as fn() == main as fn()); //~ ERROR does not live long enough
}
15 changes: 13 additions & 2 deletions src/test/ui/consts/const-eval/promoted_raw_ptr_ops.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ error[E0597]: borrowed value does not live long enough
|
LL | let y: &'static usize = &(&1 as *const i32 as usize + 1); //~ ERROR does not live long enough
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ temporary value does not live long enough
LL | let z: &'static i32 = &(unsafe { *(42 as *const i32) }); //~ ERROR does not live long enough
...
LL | }
| - temporary value only lives until here
|
Expand All @@ -25,11 +25,22 @@ error[E0597]: borrowed value does not live long enough
|
LL | let z: &'static i32 = &(unsafe { *(42 as *const i32) }); //~ ERROR does not live long enough
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ temporary value does not live long enough
LL | let a: &'static bool = &(main as fn() == main as fn()); //~ ERROR does not live long enough
LL | }
| - temporary value only lives until here
|
= note: borrowed value must be valid for the static lifetime...

error[E0597]: borrowed value does not live long enough
--> $DIR/promoted_raw_ptr_ops.rs:18:29
|
LL | let a: &'static bool = &(main as fn() == main as fn()); //~ ERROR does not live long enough
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ temporary value does not live long enough
LL | }
| - temporary value only lives until here
|
= note: borrowed value must be valid for the static lifetime...

error: aborting due to 3 previous errors
error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0597`.