Incorrect effect of stability attribute on calling private const fn

[dtolnay]

In general we don't require private functions to have stability attributes. Stable functions are free to call private functions, and stable const functions are free to call private const functions that have no stability attribute.

But when a private const function inherits a stability attribute from an enclosing module, it currently can no longer be called from stable const functions. I think this behavior is not correct. For a private function, I would think stability attributes should be irrelevant.

This is causing trouble in #75772 (comment).

#![feature(staged_api, const_fn)]
#![stable(feature = "repro", since = "0")]

mod detail {
    #![unstable(feature = "detail", issue = "none")]  // works if removed

    pub(crate) const fn f() {}

    // presumably other public stable things
}

#[stable(feature = "repro", since = "0")]
#[rustc_const_stable(feature = "repro", since = "0")]
pub const fn f() {
    detail::f();
}

error[E0723]: can only call other `const fn` within a `const fn`, but `const detail::f` is not stable as `const fn`
  --> src/lib.rs:15:5
   |
15 |     detail::f();
   |     ^^^^^^^^^^^
   |
   = note: see issue #57563 <https://github.com/rust-lang/rust/issues/57563> for more information
   = help: add `#![feature(const_fn)]` to the crate attributes to enable

Cross referencing const fn tracking issue #57563.

[ecstatic-morse]

This was a conscious choice:

rust/src/librustc_mir/const_eval/fn_queries.rs

Lines 66 to 71 in 663d2f5

	// Internal functions are forced to conform to min_const_fn.
	// Annotate the internal function with a const stability attribute if
	// you need to use unstable features.
	// Note: this is an arbitrary choice that does not affect stability or const
	// safety or anything, it just changes whether we need to annotate some
	// internal functions with `rustc_const_stable` or with `rustc_const_unstable`

I suppose we could change this if it's causing problems, but some caution here is warranted. We've made some mistakes in the past with these checks.

[dtolnay]

Thanks for the link!

Looking at the workaround in https://github.com/rust-lang/rust/pull/75772/files#r474918624 (adding an unstable annotation on private function), something still seems wrong; it may not have been the thing I first thought.

If we want private helpers to be annotated with rustc_const_stable when part of the control flow of a public stable const fn, that's great and I am on board with that. We avoid accidentally committing to too much power in const evaluation. So I see why rustc_const_stable on private functions is a useful thing.

But from reading the comment you linked, I'm not sure why this specific error would go away by adding an unstable annotation on a private function -- how come stable or unstable on a private function has any meaning at all?

[ecstatic-morse]

Well the branch that makes unstable work around the restriction is immediately above the previous link:

rust/src/librustc_mir/const_eval/fn_queries.rs

Lines 62 to 63 in 45060c2

	// Unstable functions need not conform to min_const_fn.
	false

As for the "why", I didn't write the code or formulate the rules. My understanding is that, as long as a function is unstable in some way (either with the usual stability attributes or the similar const ones), there's no danger of accidentally depending on some nightly-only const feature since stable functions can't call unstable ones and those checks are relatively straightforward. Whether private functions are considered (const) "stable" or "unstable" is not obvious, so this code takes a conservative approach an considers all un-annotated functions possibly non-conformant.

[dtolnay]

What would go wrong if we treat all private functions as unstable for the purpose of this check in const eval?

They may then separately be rustc_const_stable or not, as appropriate.

[ecstatic-morse]

We would have to ensure that all private unstable but not rustc_const_unstable functions conform to the "min const fn" restrictions and do not use any unstable const-eval features. Also, if there were any existing private, unstable const fn that did not pass the "min const fn" checks, you would have to annotate them withrustc_const_unstable. I think this is not hard but also not high priority for me personally.

[dtolnay]

That seems backward--- If "Unstable functions need not conform to min_const_fn" according to #75794 (comment), and we were to treat all private functions as if they were unstable as per #75794 (comment), why would that imply ensuring that all private functions conform to min const fn restrictions?

[ecstatic-morse]

If you want to call a function (the callee) from a function that is marked both stable and rustc_const_stable (the caller), the callee must also pass the "min const fn" checks. This is enforced as part of the "min const fn" checks on the caller. In other words, "min const fn" conformance is transitive. I interpreted your comment by assuming you want to have unstable functions that don't have any of rustc_const_unstable or rustc_const_stable be subject to the "min const fn" checks. Is this incorrect?

The hard rule here is that we can't let const-stable functions call functions that do not pass the "min const fn" checks.

[dtolnay]

I'll poke @oli-obk because from looking at the commit history of the is_min_const_fn logic, this looks like maybe just an implementation oversight in 83b8a56.

The scenario is:

#![feature(staged_api, const_fn)]
#![stable(feature = "repro", since = "0")]
#![allow(dead_code)]

#[unstable(feature = "detail", issue = "none")]  // does not compile
//#[stable(feature = "repro", since = "0")]  // compiles
pub mod detail {
    pub(crate) const fn bar() {}
}

// not public, not rustc_const_stable, maybe not even used
const fn foo() {
    detail::bar();
}

(playground)

@oli-obk can you tell whether it's intentional in this code that foo would compile or not based on whether the module containing the private helper is stable or unstable?

[dtolnay]

@ecstatic-morse:

I interpreted your comment by assuming you want to have unstable functions that don't have any of rustc_const_unstable or rustc_const_stable be subject to the "min const fn" checks. Is this incorrect?

The thing I want is for stable vs unstable to not be meaningful whatsoever on a private function. Those attributes are meant to describe the public API of a crate only.

I think the code I opened this issue with ended up not being representative of the confusion in #75772. The code in my previous comment is the most accurate.

[ecstatic-morse]

In that example, the choice of whether foo must pass the "min const fn" checks is arbitrary as explained here:

rust/src/librustc_mir/const_eval/fn_queries.rs

Lines 66 to 71 in 663d2f5

	// Internal functions are forced to conform to min_const_fn.
	// Annotate the internal function with a const stability attribute if
	// you need to use unstable features.
	// Note: this is an arbitrary choice that does not affect stability or const
	// safety or anything, it just changes whether we need to annotate some
	// internal functions with `rustc_const_stable` or with `rustc_const_unstable`

However, in your original example, the outer function is const-stable, and thus must be "min const fn".

There's also the question of whether bar must conform to "min const fn". I think we could (not necessarily should) force all unstable functions to conform to "min const fn" and allow opt-out solely via rustc_const_unstable, in which case the process in #75794 (comment) would have to be followed. edit: In that comment, I used the term "private" instead of the very specific combination of stability attributes that applies to bar. This probably caused some of the confusion.