Skip to content

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

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Jan 9, 2024

Conversation

compiler-errors
Copy link
Member

@compiler-errors compiler-errors commented Jan 7, 2024

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 #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:

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 #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:

#[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.

#[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.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 7, 2024
@rust-cloud-vms rust-cloud-vms bot force-pushed the constness-implication branch from b3067c4 to ab364e2 Compare January 7, 2024 23:24
Copy link
Member

@fmease fmease left a comment

Choose a reason for hiding this comment

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

I think that you can / should also remove the ~const-related logic from one_bound_for_assoc_item, right?

@rust-cloud-vms rust-cloud-vms bot force-pushed the constness-implication branch from ab364e2 to f36ddf2 Compare January 8, 2024 01:07
@rust-log-analyzer

This comment has been minimized.

@rust-cloud-vms rust-cloud-vms bot force-pushed the constness-implication branch from f36ddf2 to 99292c9 Compare January 8, 2024 01:40
@rust-log-analyzer

This comment has been minimized.

@rust-cloud-vms rust-cloud-vms bot force-pushed the constness-implication branch from 99292c9 to d2f2a24 Compare January 8, 2024 15:01
@rust-log-analyzer

This comment has been minimized.

@rust-cloud-vms rust-cloud-vms bot force-pushed the constness-implication branch from d2f2a24 to 760673e Compare January 8, 2024 15:32
Copy link
Member

@fee1-dead fee1-dead left a comment

Choose a reason for hiding this comment

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

Thanks

@fee1-dead
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Jan 9, 2024

📌 Commit 760673e has been approved by fee1-dead

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 9, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 9, 2024
…llaumeGomez

Rollup of 5 pull requests

Successful merges:

 - rust-lang#118241 (Making `User<T>` and `User<[T]>` `Send`)
 - rust-lang#118645 (chore: Bump compiler_builtins)
 - rust-lang#118680 (Add support for shell argfiles)
 - rust-lang#119721 (`~const` trait and projection bounds do not imply their non-const counterparts)
 - rust-lang#119768 (core: panic: fix broken link)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f4d0625 into rust-lang:master Jan 9, 2024
@rustbot rustbot added this to the 1.77.0 milestone Jan 9, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request 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.
@fee1-dead fee1-dead added F-const_trait_impl `#![feature(const_trait_impl)]` PG-const-traits Project group: Const traits labels Aug 9, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
F-const_trait_impl `#![feature(const_trait_impl)]` PG-const-traits Project group: Const traits S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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