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

Const functions' effect parameter is early-bound, leading to them only satisfying const Fn bounds #119718

Closed
compiler-errors opened this issue Jan 7, 2024 · 2 comments · Fixed by #119721
Labels
C-bug Category: This is a bug. F-effects `#![feature(effects)]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@compiler-errors
Copy link
Member

I tried this code:

#![crate_type = "lib"]
#![allow(internal_features)]
#![no_std]
#![no_core]
#![feature(
    auto_traits,
    const_trait_impl,
    effects,
    lang_items,
    no_core,
    staged_api,
    unboxed_closures,
)]
#![stable(feature = "minicore", since = "1.0.0")]

fn test() {
    fn is_const_fn<F>(_: F)
    where
        F: const FnOnce<()>,
    {}

    const fn foo() {}

    is_const_fn(foo); // (*) See description in thread.
}

/// ---------------------------------------------------------------------- ///
/// Const fn trait definitions

#[const_trait]
#[lang = "fn"]
#[rustc_paren_sugar]
trait Fn<Args: Tuple>: ~const FnMut<Args> {
    extern "rust-call" fn call(&self, args: Args) -> Self::Output;
}

#[const_trait]
#[lang = "fn_mut"]
#[rustc_paren_sugar]
trait FnMut<Args: Tuple>: ~const FnOnce<Args> {
    extern "rust-call" fn call_mut(&mut self, args: Args) -> Self::Output;
}

#[const_trait]
#[lang = "fn_once"]
#[rustc_paren_sugar]
trait FnOnce<Args: Tuple> {
    #[lang = "fn_once_output"]
    type Output;

    extern "rust-call" fn call_once(self, args: Args) -> Self::Output;
}

/// ---------------------------------------------------------------------- ///
/// All this other stuff needed for core. Unrelated to test.

#[lang = "destruct"]
#[const_trait]
trait Destruct {}

#[lang = "freeze"]
unsafe auto trait Freeze {}

#[lang = "drop"]
#[const_trait]
trait Drop {
    fn drop(&mut self);
}

#[lang = "sized"]
trait Sized {}
#[lang = "copy"]
trait Copy {}

#[lang = "tuple_trait"]
trait Tuple {}

#[lang = "receiver"]
trait Receiver {}

impl<T: ?Sized> Receiver for &T {}

impl<T: ?Sized> Receiver for &mut T {}
@compiler-errors compiler-errors added C-bug Category: This is a bug. F-effects `#![feature(effects)]` labels Jan 7, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jan 7, 2024
@compiler-errors compiler-errors changed the title Const functions' effect parameter is early-bound, leading to them only satisfying Const functions' effect parameter is early-bound, leading to them only satisfying const Fn bounds Jan 7, 2024
@compiler-errors
Copy link
Member Author

compiler-errors commented Jan 7, 2024

(*)

When we instantiate the path foo above, it turns into foo::<?0e>, with an effect infer var. When we select foo::<?0e>: Fn<(), false>, where false is the host parameter, this constrains ?0e to be false as well.

Unfortunately, checking foo::<?0e>: Fn<(), false> requires us to also check foo::<?0e>: Fn<(), true>, due to this code:

let non_const_bound = if tcx.features().effects && tcx.has_attr(def_id, sym::const_trait) {
// when `Self` is a const trait, also add `Self: Trait<.., true>` as implied bound,
// because only implementing `Self: Trait<.., false>` is currently not possible.
Some((
ty::TraitRef::new(
tcx,
def_id,
ty::GenericArgs::for_item(tcx, def_id, |param, _| {
if param.is_host_effect() {
tcx.consts.true_.into()
} else {
tcx.mk_param_from_def(param)
}
}),
)
.to_predicate(tcx),
span,
))
} else {
None
};

This causes our previously constrained ?0e = false param to be equated to true, leading to a type error.


It turns out that const fn paths only implement one of const Fn() or Fn(). I don't believe there's a good way to make them implement both, since the effect parameter in a const fn is early-bound, and that effect param must agree with the constness of the Fn trait it's being proven to implement.

I'm not certain how we expect to reconcile the early-boundness of host parameters with the conceptual late-boundness of the constness effect being enforced at call sites. This doesn't seem concordant either with the fact that the host parameter is observable at monomorphization time via const_eval_select.

@compiler-errors
Copy link
Member Author

compiler-errors commented Jan 7, 2024

Notably, this bug prevents me from turning the implementation of const_eval_select into something that looks like:

pub fn const_eval_select<ARG: Tuple, F, G, RET>(
        arg: ARG,
        called_in_const: F,
        called_at_rt: G,
    ) -> RET
    where
        F: const FnOnce<ARG, Output = RET>,
        G: FnOnce<ARG, Output = RET>;

which accurately reflects the type checking of calling such an intrinsic. Fixing this would also allow me to remove the gross manual logic of faea6ad, which manually registers the correct const projection bound and doesn't even try to register the trait bound const FnOnce due to its manual checking of the TyKind::FnDef.

@fmease fmease added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jan 7, 2024
@fmease fmease changed the title Const functions' effect parameter is early-bound, leading to them only satisfying const Fn bounds Const functions' effect parameter is early-bound, leading to them only satisfying non-const Fn bounds Jan 7, 2024
@compiler-errors compiler-errors changed the title Const functions' effect parameter is early-bound, leading to them only satisfying non-const Fn bounds Const functions' effect parameter is early-bound, leading to them only satisfying const Fn bounds Jan 7, 2024
@bors bors closed this as completed in f4d0625 Jan 9, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jan 9, 2024
Rollup merge of rust-lang#119721 - compiler-errors:constness-implication, r=fee1-dead

`~const` trait and projection bounds do not imply their non-const counterparts

This PR removes the hack where we install a non-const trait and projection bound for every `const_trait` and `~const` projection bound we have in the AST. It ends up messing up more things than it fixes, see words below.

Fixes rust-lang#119718

cc `@fmease` `@fee1-dead` `@oli-obk`
r? fee1-dead or one of y'all i don't care

---

My understanding is that this hack was added to support the following code:

```rust
pub trait Owo<X = <Self as Uwu>::T> {}

#[const_trait]
pub trait Uwu: Owo {}
```

Which is concretely lifted from in the `FromResidual` and `Try` traits. Since within the param-env of `trait Uwu`, we only know that `Self: ~const Uwu` and not `Self: Uwu`, the projection `<Self as Uwu>::T` is not satsifyable.

This causes problems such as rust-lang#119718, since instantiations of `FnDef` types coming from `const fn` really do **only** implement one of `FnOnce` or `const FnOnce`!

---

In the long-term, I believe that such code should really look something more like:

```rust
#[const_trait]
pub trait Owo<X = <Self as ~const Uwu>::T> {}

#[const_trait]
pub trait Uwu: Owo {}
```

... and that we should introduce some sort of `<T as ~const Foo>::Bar` bound syntax, since due to the fact that `~const` bounds can be present in item bounds, e.g.

```rust
#[const_trait] trait Foo { type Bar: ~const Destruct; }
```

It's easy to see that `<T as Foo>::Bar` and `<T as ~const Foo>::Bar` (or `<T as const Foo>::Bar`) can be distinct types with distinct item bounds!

**Admission**: I know I've said before that I don't like `~const` projection syntax, I do at this point believe they're necessary to fully express bounds and types in a maybe-const world.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. F-effects `#![feature(effects)]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants