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

Implement ~const item bounds in RPIT #133218

Merged
merged 2 commits into from
Nov 21, 2024
Merged

Conversation

compiler-errors
Copy link
Member

an RPIT in a const fn is allowed to be conditionally const itself :)

r? fee1-dead or reroll

@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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Nov 19, 2024
@@ -339,6 +339,10 @@ impl<'tcx> Visitor<'tcx> for CollectItemTypesVisitor<'tcx> {
self.tcx.ensure().explicit_item_super_predicates(def_id);
self.tcx.ensure().item_bounds(def_id);
self.tcx.ensure().item_super_predicates(def_id);
if self.tcx.is_conditionally_const(def_id) {
Copy link
Member Author

Choose a reason for hiding this comment

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

I don't believe this is necessary but it helps make sure we report errors in the right order.

Comment on lines +2120 to +2121
DefKind::OpaqueTy => match self.opaque_ty_origin(def_id) {
hir::OpaqueTyOrigin::FnReturn { parent, .. } => self.is_conditionally_const(parent),
Copy link
Member

Choose a reason for hiding this comment

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

Am I understanding this correctly that this would treat the impl Trait in

const fn a() -> impl Trait {}

as conditionally const? If so, then this is wrong, and the ~constness of an opaque type should be recorded in some way.

Copy link
Member Author

Choose a reason for hiding this comment

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

No, you're misreading. It's conditionally const, but it just has no const item bounds.

Copy link
Member Author

@compiler-errors compiler-errors Nov 20, 2024

Choose a reason for hiding this comment

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

I'll try to state the situation in a bit clearer terms.

[...] and the ~constness of an opaque type [...]

So, opaques are not ~const. ~const is a modifier on a bound, and RPITs can have some ~const bounds and some non-~const bounds.

What marking them as conditionally const means, for an alias, is that we try to look at the implied_const_bounds to when proving effect predicates. These implied const bounds only hold if the const conditions of the opaque hold -- in all cases, since an opaque can't have its own where clauses, it just inherits the const conditions of the function that defines it.

So for something like:

const test<T: ~const Foo>() -> impl ~const Bar {}

We know that the RPIT implements ~const Bar only if T: ~const Foo. This is analogous to a trait impl like:

#[const_trait] trait Foo {
    type Item: ~const Bar;
} 

where <T as Foo>::Item only implements ~const Bar if T: ~const Foo.

Copy link
Member

Choose a reason for hiding this comment

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

Got it, thanks

@fee1-dead
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Nov 21, 2024

📌 Commit 5eeaf2e 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 Nov 21, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 21, 2024
…e1-dead

Implement `~const` item bounds in RPIT

an RPIT in a `const fn` is allowed to be conditionally const itself :)

r? fee1-dead or reroll
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 21, 2024
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#131736 (Emscripten: link with -sWASM_BIGINT)
 - rust-lang#132207 (Store resolution for self and crate root module segments)
 - rust-lang#133153 (Add visits to nodes that already have flat_maps in ast::MutVisitor)
 - rust-lang#133218 (Implement `~const` item bounds in RPIT)
 - rust-lang#133228 (Rewrite `show_md_content_with_pager`)
 - rust-lang#133247 (Reduce integer `Display` implementation size)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 21, 2024
…iaskrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#131736 (Emscripten: link with -sWASM_BIGINT)
 - rust-lang#132207 (Store resolution for self and crate root module segments)
 - rust-lang#133153 (Add visits to nodes that already have flat_maps in ast::MutVisitor)
 - rust-lang#133218 (Implement `~const` item bounds in RPIT)
 - rust-lang#133228 (Rewrite `show_md_content_with_pager`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 9200925 into rust-lang:master Nov 21, 2024
6 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Nov 21, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 21, 2024
Rollup merge of rust-lang#133218 - compiler-errors:const-opaque, r=fee1-dead

Implement `~const` item bounds in RPIT

an RPIT in a `const fn` is allowed to be conditionally const itself :)

r? fee1-dead or reroll
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants