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

Allow using fn pointers in const fn with unleash miri #64635

Merged
merged 3 commits into from
Sep 22, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
20 changes: 16 additions & 4 deletions src/librustc_mir/transform/qualify_consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1407,11 +1407,23 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> {
}
}
ty::FnPtr(_) => {
let unleash_miri = self
.tcx
.sess
.opts
.debugging_opts
.unleash_the_miri_inside_of_you;
Centril marked this conversation as resolved.
Show resolved Hide resolved
let const_fn_ptr = self.tcx.features().const_fn_ptr;
if self.mode.requires_const_checking() {
let mut err = self.tcx.sess.struct_span_err(
self.span,
&format!("function pointers are not allowed in const fn"));
err.emit();
if !(unleash_miri || const_fn_ptr) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Since the feature is inherently broken, maybe just do the unleash part and no feature gate?

emit_feature_err(
&self.tcx.sess.parse_sess,
sym::const_fn_ptr,
self.span,
GateIssue::Language,
"function pointers in const fn are unstable",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"function pointers in const fn are unstable",
"function pointers in `const fn` are unstable",

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The other errors do not follow this style. cc @oli-obk

Copy link
Contributor

Choose a reason for hiding this comment

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

The other errors are wrong then.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, but I'd rather not touch them before #64470 is merged

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that PR also enables function pointer calling behind unleash

);
}
}
}
_ => {
Expand Down
3 changes: 3 additions & 0 deletions src/libsyntax/feature_gate/active.rs
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,9 @@ declare_features! (
/// Allows macro invocations in `extern {}` blocks.
(active, macros_in_extern, "1.27.0", Some(49476), None),

/// Allows calling function pointers inside `const` functions.
(active, const_fn_ptr, "1.27.0", Some(51909), None),
gnzlbg marked this conversation as resolved.
Show resolved Hide resolved

/// Allows accessing fields of unions inside `const` functions.
(active, const_fn_union, "1.27.0", Some(51909), None),

Expand Down
1 change: 1 addition & 0 deletions src/libsyntax_pos/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ symbols! {
const_compare_raw_pointers,
const_constructor,
const_fn,
const_fn_ptr,
const_fn_union,
const_generics,
const_indexing,
Expand Down
21 changes: 21 additions & 0 deletions src/test/ui/consts/const-eval/const_fn_ptr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// run-pass
#![feature(const_fn)]
#![feature(const_fn_ptr)]

const fn double(x: usize) -> usize { x * 2 }
const X: fn(usize) -> usize = double;

const fn bar(x: usize) -> usize {
X(x)
}

const fn foo(x: fn(usize) -> usize, y: usize) -> usize {
x(y)
}

fn main() {
const Y: usize = bar(2);
assert_eq!(Y, 4);
const Z: usize = foo(double, 2);
assert_eq!(Z, 4);
}
15 changes: 15 additions & 0 deletions src/test/ui/consts/const-eval/const_fn_ptr_fail.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// run-pass

// FIXME: this should not pass

#![feature(const_fn)]
#![feature(const_fn_ptr)]

fn double(x: usize) -> usize { x * 2 }
const X: fn(usize) -> usize = double;

const fn bar(x: usize) -> usize {
X(x)
Copy link
Contributor Author

@gnzlbg gnzlbg Sep 21, 2019

Choose a reason for hiding this comment

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

@oli-obk this is the issue that we chatted about. Note that bar can be called at run-time, but it will always fail with a constant evaluation error when called in a const context because it tries to call a non-const fn.

I'm not sure how to generate an error in this case. Right now it generates a constant-evaluation error when used in const-context, but it would be nice to generate a better error message.

Copy link
Contributor

Choose a reason for hiding this comment

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

XD welcome to RFC territory. This needs an RFC, because we need surface language changes in order to differentiate between callable and not callable function pointers.

This is what I meant by "we can do it behind a feature gate for experimentation, but we'll never be able to stabilize it.". For all intents and purposes, this is const unsound just like casting pointers to usize or comparing pointers. The pointer ops have been made unsafe for this reason, so maybe we make this unsafe, too? Idk. Seems all fishy without an RFC but as an experimental unstable feature seems fine

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@oli-obk I've updated the OP with a description of the semantics that this PR implements. I'm not sure why this would be const unsound. The const fn taking pointers should be parametric over the pointer constness, and we should be able to reject that at type checking time without new syntax AFAICT.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, that would be one way to do it, it would mean you can't pass any function pointers, because there are no const function pointers. Not compiling is the current semantics and is totally sound. This is all discussed in my RFC and needs its own RFC. I'm 🤧 right now and my brain is mush. I'd prefer not to write out an explanation right now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I see what you mean now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure if we need language syntax for this initially, e.g., we could just add a type that users cannot write, add coercions to it from const fns, and then do the const parametricity part. But that's 99% of the work, and at that point not allowing people to write down the type feels like unnecessary.

Either way, the intent of this PR was never to go through all that trouble.

}

fn main() {}
20 changes: 20 additions & 0 deletions src/test/ui/consts/const-eval/const_fn_ptr_fail.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
warning: function is never used: `double`
Copy link
Contributor

Choose a reason for hiding this comment

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

The dead_code warnings are off-topic and shouldn't be here.

--> $DIR/const_fn_ptr_fail.rs:8:1
|
LL | fn double(x: usize) -> usize { x * 2 }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `#[warn(dead_code)]` on by default

warning: constant item is never used: `X`
--> $DIR/const_fn_ptr_fail.rs:9:1
|
LL | const X: fn(usize) -> usize = double;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

warning: function is never used: `bar`
--> $DIR/const_fn_ptr_fail.rs:11:1
|
LL | const fn bar(x: usize) -> usize {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

21 changes: 21 additions & 0 deletions src/test/ui/consts/const-eval/const_fn_ptr_fail2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#![feature(const_fn)]
#![feature(const_fn_ptr)]

fn double(x: usize) -> usize { x * 2 }
const X: fn(usize) -> usize = double;

const fn bar(x: fn(usize) -> usize, y: usize) -> usize {
x(y)
}

const Y: usize = bar(X, 2);
Copy link
Contributor

Choose a reason for hiding this comment

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

This and Z should not type-check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the same issue mentioned in the OP and here and should be fixed by fixing that as well.

//~^ ERROR any use of this value will cause an error

const Z: usize = bar(double, 2);
//~^ ERROR any use of this value will cause an error


fn main() {
assert_eq!(Y, 4);
assert_eq!(Z, 4);
}
28 changes: 28 additions & 0 deletions src/test/ui/consts/const-eval/const_fn_ptr_fail2.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
error: any use of this value will cause an error
--> $DIR/const_fn_ptr_fail2.rs:8:5
|
LL | x(y)
| ^^^^
| |
| calling non-const function `double`
| inside call to `bar` at $DIR/const_fn_ptr_fail2.rs:11:18
...
LL | const Y: usize = bar(X, 2);
| ---------------------------
|
= note: `#[deny(const_err)]` on by default

error: any use of this value will cause an error
--> $DIR/const_fn_ptr_fail2.rs:8:5
|
LL | x(y)
| ^^^^
| |
| calling non-const function `double`
| inside call to `bar` at $DIR/const_fn_ptr_fail2.rs:14:18
...
LL | const Z: usize = bar(double, 2);
| --------------------------------

error: aborting due to 2 previous errors

11 changes: 11 additions & 0 deletions src/test/ui/consts/const-eval/feature-gate-const_fn_ptr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#![feature(const_fn)]

fn main() {}

const fn foo() {}
const X: fn() = foo;

const fn bar() {
X()
//~^ ERROR function pointers in const fn are unstable
}
12 changes: 12 additions & 0 deletions src/test/ui/consts/const-eval/feature-gate-const_fn_ptr.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
error[E0658]: function pointers in const fn are unstable
--> $DIR/feature-gate-const_fn_ptr.rs:9:5
|
LL | X()
| ^^^
|
= note: for more information, see https://github.com/rust-lang/rust/issues/51909
= help: add `#![feature(const_fn_ptr)]` to the crate attributes to enable

error: aborting due to previous error

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