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

[WIP] experiment with a new way of effects desugaring #120639

Draft
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

fee1-dead
Copy link
Member

@fee1-dead fee1-dead commented Feb 4, 2024

cc @rust-lang/project-const-traits. Will write down notes once I have finished.

  • See if we want T: Tr to desugar into T: Tr, T::Effects: Compat<true>
  • Fix ICEs on type Assoc: ~const Tr and type Assoc<T: ~const Tr>
  • add types and traits to minicore test
  • update rustc-dev-guide

@rustbot
Copy link
Collaborator

rustbot commented Feb 4, 2024

r? @oli-obk

(rustbot has picked a reviewer for you, use r? to override)

@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. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Feb 4, 2024
@rust-log-analyzer

This comment has been minimized.

@@ -104,6 +104,11 @@ pub(super) fn explicit_item_bounds(
None => {}
}

// effects desugared associated types have no bounds.
Copy link
Contributor

Choose a reason for hiding this comment

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

Could feed this query.

Also: don't we need a where Self: Sized bound to avoid breaking all dyn Trait for now?

Copy link
Member Author

Choose a reason for hiding this comment

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

assoc types don't break object safety (we'd just need to make it default to Runtime)

what breaks object safety would probably be the const host: bool that we now insert onto each method.

@bors
Copy link
Contributor

bors commented Feb 6, 2024

☔ The latest upstream changes (presumably #120392) made this pull request unmergeable. Please resolve the merge conflicts.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Feb 13, 2024

☔ The latest upstream changes (presumably #120991) made this pull request unmergeable. Please resolve the merge conflicts.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@fee1-dead
Copy link
Member Author

This PR changes T: Tr where Tr is #[const_trait] to also lower into an additional bound of <T as Tr>::{opaque}: Compat<true>. That means potentially more work for the trait solver. Let's see what the perf impact is like, as right now we have Add as a #[const_trait]

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Feb 26, 2024
@fee1-dead
Copy link
Member Author

@bors try

@bors
Copy link
Contributor

bors commented Feb 26, 2024

⌛ Trying commit f1cd746 with merge c5d6267...

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 26, 2024
…ring, r=<try>

[WIP] experiment with a new way of effects desugaring

cc `@rust-lang/project-const-traits.` Will write down notes once I have finished.

* [ ] See if we want `T: Tr` to desugar into `T: Tr, T::Effects: Compat<true>`
* [ ] Fix ICEs on `type Assoc: ~const Tr` and `type Assoc<T: ~const Tr>`
* [ ] add types and traits to minicore test
* [ ] update rustc-dev-guide
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Feb 26, 2024

💔 Test failed - checks-actions

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 26, 2024
@rustbot rustbot added the A-rustdoc-json Area: Rustdoc JSON backend label Feb 27, 2024
@fee1-dead
Copy link
Member Author

@bors try @rust-timer queue

@fee1-dead
Copy link
Member Author

The compile error from perf is interesting. The existence of closures that only implement FnOnce for use in compile time makes us unable to assume that any trait implementation would allow for usage in runtime. (this would extend to future user-defined types if we allow ~const dyn and ~const fn and the such)

This means for any trait that has a const_trait super trait, like trait Curve: Add, we also want this to implicitly mean that trait Curve: Add<Effects: ComptabileWith<Runtime>>. This mostly works fine (?) by adding to the implied bounds, but causes trouble when binders are involved, as seen with those compile failures.

I've opened #121929, and fixing that would probably unblock this. If that cannot be fixed easily, we might need to explore a workaround such as emitting two separate associated types each encoding whether it is compatible with runtime=true and compatible with runtime=false.

@bors
Copy link
Contributor

bors commented Mar 7, 2024

☔ The latest upstream changes (presumably #122113) made this pull request unmergeable. Please resolve the merge conflicts.

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Apr 23, 2024

☔ The latest upstream changes (presumably #124289) made this pull request unmergeable. Please resolve the merge conflicts.

@fee1-dead
Copy link
Member Author

Noting, with initial implementation of Min (still quite wrong in its desugaring with binders and etc), need to investigate the tests/ui/rfcs/rfc-2632-const-trait-impl/super-traits tests.

  • Consider appending Self::Effects into the clause as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-json Area: Rustdoc JSON backend S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-perf Status: Waiting on a perf run to be completed. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants