Skip to content

RFC: Pointers to const fn:s #2238

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

Closed
wants to merge 1 commit into from

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Dec 7, 2017

Rendered.

You may now write:

const fn twice(arg: u32, fun: const fn(u32) -> u32) -> u32 {
    fun(fun(arg))
}

@Centril Centril added the T-lang Relevant to the language team, which will review and decide on the RFC. label Dec 7, 2017
@petrochenkov
Copy link
Contributor

How C++ does this:

using fnptr = auto (int) -> int;

constexpr auto const_hof(fnptr f) -> int { return f(1); }

constexpr auto const_fn(int) -> int { return 0; }
auto nonconst_fn(int) -> int { return 0; }

int main() {
    auto a = const_hof(const_fn); // OK, "runtime" context
    auto b = const_hof(nonconst_fn); // OK, "runtime" context
    constexpr auto c = const_hof(const_fn); // OK, forced constexpr context
    constexpr auto d = const_hof(nonconst_fn); // ERROR, call to non-constexpr function 'int nonconst_fn(int)'
                                               //        in constexpr expansion of 'const_hof(nonconst_fn)'
}

, i.e. the constexprness of a fnptr is checked on use of const fn in a context requiring constant evaluation, not on const fn definition. In that context we know exactly what function that fnptr came from so we can determine its constexprness.

@Centril
Copy link
Contributor Author

Centril commented Dec 7, 2017

@petrochenkov Thanks for writing that up =) What is your preferred solution of the two alternatives you list and why?

@petrochenkov
Copy link
Contributor

@Centril

What is your preferred solution

Adopting the approach from the C++ example and not introducing const fn pointers.
As constant evaluation improves constant function will have to detect evaluation of something non-constexpr "on use" anyway, e.g. to support something like this

const fn foo(a: bool) -> u8 {
    if a {
        10 // constexpr branch
    } else {
        panic!() // non-constexpr branch, errors when evaluated in constexpr context
    }
}

, so fn pointers can be checked for constexprness "on use" as well.


In addition, twice from the example in #2238 (comment) is supposed to report a type error when called with a non-const fn pointer even if it's called from "runtime" context, so you'll need a second function twice_runtime to cover both contexts. This seems worse than having a single function that can be called from both constexpr and non-constexpr contexts.

@Centril
Copy link
Contributor Author

Centril commented Dec 7, 2017

@petrochenkov I buy that, so I'll close this RFC (in short order) when I've written a replacement RFC fn_ptr_in_const_fn. Cheers =)

@eddyb
Copy link
Member

eddyb commented Dec 7, 2017

@petrochenkov I disagree with non-CTFE logic in const fn (except via unsafe), and panic!() will be made compile-time-friendly, hopefully earlier than dynamic allocation.

@Centril
Copy link
Contributor Author

Centril commented Dec 7, 2017

I guess I change my mind then - not closing after all.

@petrochenkov
Copy link
Contributor

@eddyb
Any non-constexpr call is pretty much equivalent to panic when evaluated in constexpr context - both can result in a "can't evaluate" compile-time error reported somewhere deep inside of constexpr evaluation (which can happen or not happen depending on arguments passed to a constexpr function from somewhere else), both execute something instead in runtime context. What are the reasons to allow one, but not another?

@eddyb
Copy link
Member

eddyb commented Dec 7, 2017

If we allow calls to runtime functions then there isn't any point to const fn at all, is there?
Which I'd be sympathetic too, and would simplify pretty much all the design work, except for versioning and compatibility concerns, that is, writing const fn as a guarantee to users of a library.

@gralpli
Copy link

gralpli commented Dec 13, 2017

@eddyb

If you allow annotating a function with const, then every function that is passed in as an argument needs to be const, too. Otherwise there is no guarantee that it behaves const, which makes the keyword useless. Consider the following function within a library:

const fn call_non_const(f: fn(bool)) {
    f(false)
}

The user of the library uses it as follows:

fn f(b: bool) {
    if b {
        // do something non-const
    }
}

call_non_const(f);

This last call is const, as long as the author of the library doesn't change false to true.

I would sympathize with completely relinquishing the const keyword on functions. If a regular function is called in a const context, the compiler tries to evaluate it and is happy if it can.

But if we allow functions to be const, then we need this RFC, too.

Another idea: We could have both. Allow the annotation with const as a guarantee, but also allow calling a non-const function in a const context if it can be evaluated at compile time.

@eddyb
Copy link
Member

eddyb commented Dec 13, 2017

@gralpli FWIW I expect fn pointers to be much rarer than trait bounds (for which we be parametric and thus more ergonomic, see my comments on #2237), but otherwise I agree it'd be easier without explicit const. For better or for worse, compatibility concerns have always influenced design.

@Centril
Copy link
Contributor Author

Centril commented Dec 13, 2017

Right ;) This RFC begins from the assumption that const fn will be stabilized and of course falls immediately if they won't.

Another idea: We could have both. Allow the annotation with const as a guarantee, but also allow calling a non-const function in a const context if it can be evaluated at compile time.

I like that, particularly the guarantee bit - but could the latter part increase compile times? There's a certain benefit to being able to assume that "yep, this function promised it was a const fn in the signature" and then separately ensure that it indeed was const fn. Being able to call fn in const fn might effectively amount to type checking the fn twice - tho perhaps there are caching opportunities that makes this viable? In any case I think that since that is an orthogonal proposal and compatible with this RFC, they can be proposed separately. Thoughts?

# Drawbacks
[drawbacks]: #drawbacks

Some may argue that we don't need HOFs that are `const fn`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Function pointers require some additional checks in miri to ensure noone transmutes function pointers in "bad" ways. It's probably fine to convert a const fn(&u32) to a const fn(Option<&u32>), but not the other way around. Additionally someone might convert from fn() to const fn() and try to call that.

While these things are "forbidden" at runtime (UB), it's not clear how they should be checked at compile-time.

Copy link
Member

Choose a reason for hiding this comment

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

Somewhat related: you can use the target's call ABI (or will be able to, soon enough), and if they match up, you can reuse the bits - if register bank/memory indirection doesn't match at all, it's quite UB. But also I mostly care about checking calls, not casts.

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

Additionally someone might convert from fn() to const fn() and try to call that.

Can't we simply forbid that conversion? Would an alternative be to have UB at compile time (is that possible?) if such a cast is done improperly?

@eddyb Elaborate?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we simply forbid that conversion? Would an alternative be to have UB at compile time (is that possible?) if such a cast is done improperly?

The conversion is not preventable, you can always do *(&foo as *const Foo as *const Bar) to hide any conversion. The final call can be checked, but doing that right will require some implementation effort.

@Diggsey
Copy link
Contributor

Diggsey commented Feb 26, 2018

I like the idea of just allowing calls to "not necessarily const" functions from "const" functions, and only giving a compile error if the function indeed turns out to have not been declared with "const".

This would work for all kinds of "indirect" calls, both via function pointers and via non-const trait methods (where the implementer of the trait does provide a const implementation, assuming #2237 is accepted in some form). Also, this would work for both trait objects and trait bounds in generic functions.

@oli-obk
Copy link
Contributor

oli-obk commented Feb 26, 2018

I also like that logic, because it feels like the const fn rules that we have now, just fully generalized.

@nikomatsakis
Copy link
Contributor

I like the idea of just allowing calls to "not necessarily const" functions from "const" functions, and only giving a compile error if the function indeed turns out to have not been declared with "const".

I'm torn. I see the appeal of this sort of "duck typing" approach, but it feels like it runs pretty counter to Rust's general approach. My main concern is that it complicates the meaning of semver around const fn. Consider:

Consider:

// In revision 1:
const fn foo(i: u32, _: fn() -> u32) -> u32 {
    i
}

// In revision 2:
const fn foo(i: u32, a: fn() -> u32) -> u32 {
    if i > 22 { a() } else { i }
}

Revisions 1 and 2 are semver compatible for ordinary functions, but -- for const fn -- consumers could stop compiling, if they happened to be passing a non-const fn.

That said, I do think that this is an interesting point in the space. It's not nearly as lassez faire as "get rid of const fn entirely", in which case any change you make could potentially break a consumer. At least all the code that runs has to have been declared as const. And one could imagine making semver rules that say that a const fn is allowed to assume that its fn arguments are const fn unless explicitly stated otherwise -- i.e., if your clients break, it's their fault. (We could even lint this.)

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Mar 16, 2018

One of the things I sort of regret in rust's design is that unsafe fn is a type. It seems logical enough, but it means that changing a function from unsafe fn foo() to fn foo() is a source of breakage, even if it is so rare that we tend to "do it anyway". This RFC seems poised to make "adding const to a fn" have the same problem, no?

That makes me wary. After all, we expect it to be very common that people declare functions as not const but then add const upon request, when they feel comfortable committing to that. But if each such change potentially breaks clients, that's a pain.

UPDATE: Oh, hmm, that breakage is only "unused unsafe lints". I was wondering about that -- is that the only source of problems? If so, that definitely affects my opinion. =)

@oli-obk
Copy link
Contributor

oli-obk commented Mar 17, 2018

Since you can't put a fn bound on a generic type, I don't think either removing unsafe or adding const will have any breaking changes. Adding const has a definite chance of triggering new instances of the const_err lint.

Revisions 1 and 2 are semver compatible for ordinary functions,

They are compile time compatible, but at runtime they'll still behave differently. Replacing the body of a function with panic!("don't use this") is just as much a breaking change as removing the function entirely.

@nikomatsakis
Copy link
Contributor

@oli-obk

They are compile time compatible, but at runtime they'll still behave differently.

I am talking about compile time. I'm presuming that the runtime behavior is considered acceptable. It's always a fine line between a "bug fix" and a "breaking change".

@Centril
Copy link
Contributor Author

Centril commented May 2, 2018

As with the #2237 RFC; I'll close this one for now and write up a more coherent and comprehensive proposal sometime later.

@Centril Centril closed this May 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants