Install projection from RPITIT to default trait method opaque correctly #109198

compiler-errors · 2023-03-16T01:36:22Z

For new lowering strategy -Zlower-impl-trait-in-trait-to-assoc-ty, install the correct default trait method projection predicates (RPITIT -> opaque). This makes default trait body tests pass!
Fix two WF-checking bugs -- first, we want to make sure that we're always looking for an opaque type in check_return_position_impl_trait_in_trait_bounds. That's because the RPITIT projections are normalized to opaques during wfcheck. Second, fix RPITIT's param-envs by not adding the projection predicates that we install on trait methods to make default RPITITs work -- I left a comment why.
Also, just a small drive-by for rustc_on_unimplemented. Not sure if it affects any tests, but can't hurt.

r? @spastorino, based off of #109140

compiler-errors · 2023-03-16T01:44:19Z

@rustbot author

Actually don't think we need some of this stuff I'm doing here.

…ed type's param-env

compiler-errors · 2023-03-16T02:00:02Z

@rustbot ready

compiler-errors · 2023-03-16T05:33:33Z

Sorry, second commit is mistitled -- should probably be something like "fix param-env and wfcheck for default trait body RPITITs", but I'm going to sleep.

spastorino · 2023-03-16T15:37:05Z

compiler/rustc_ty_utils/src/ty.rs

+    // get out of alignment, or else we do actually need to substitute these predicates.
+    if let Some(ImplTraitInTraitData::Trait { fn_def_id, .. }) = tcx.opt_rpitit_info(def_id) {
+        predicates = tcx.predicates_of(fn_def_id).instantiate_identity(tcx).predicates;
+    }


I think this part of the change is what makes the tests included in this commit pass, right?.

both changes are needed to make the test pass, otherwise the error message goes from a "needs type info" error to a wfchecking error, iirc.

spastorino · 2023-03-16T15:37:54Z

compiler/rustc_hir_analysis/src/check/wfcheck.rs

+                && let Some(opaque_def_id) = opaque_ty.def_id.as_local()
+                && let opaque = tcx.hir().expect_item(opaque_def_id).expect_opaque_ty()
+                && let hir::OpaqueTyOrigin::FnReturn(source) | hir::OpaqueTyOrigin::AsyncFn(source) = opaque.origin
+                && source == fn_def_id


If I understand correct this is equivalent to what was before but done a little bit better, or are there actual difference? if so in which cases?.

This is not equivalent. If we call is_impl_trait_in_trait on an opaque def id, then it returns false.

Without this, some tests begin to spuriously pass wfcheck.

the first commit in the stack "breaks" tests/ui/impl-trait/in-trait/wf-bounds.rs by actually projecting the RPITIT to a real opaque type, so this commit fixes the test by always checking for an opaque type.

the first commit in the stack "breaks" tests/ui/impl-trait/in-trait/wf-bounds.rs by actually projecting the RPITIT to a real opaque type, so this commit fixes the test by always checking for an opaque type.

Ahh right, because we project the projection to an opaque, that's the thing.

spastorino · 2023-03-16T17:36:41Z

compiler/rustc_ty_utils/src/ty.rs

-    if let Some(ImplTraitInTraitData::Trait { fn_def_id, .. }) = tcx.opt_rpitit_info(def_id) {
-        return tcx.param_env(fn_def_id);
-    }
-
    // Compute the bounds on Self and the type parameters.
    let ty::InstantiatedPredicates { mut predicates, .. } =


Wouldn't be more clear if this is an else of the if let from below?.

Yeah, that works too.

spastorino · 2023-03-16T18:45:20Z

This looks good to me, r=me after mine is merged and rebased.

compiler-errors · 2023-03-16T19:05:49Z

@bors r=spastorino

bors · 2023-03-16T19:05:52Z

📌 Commit 8d922eb has been approved by spastorino

It is now in the queue for this repository.

…-body, r=spastorino Install projection from RPITIT to default trait method opaque correctly 1. For new lowering strategy `-Zlower-impl-trait-in-trait-to-assoc-ty`, install the correct default trait method projection predicates (RPITIT -> opaque). This makes default trait body tests pass! 2. Fix two WF-checking bugs -- first, we want to make sure that we're always looking for an opaque type in `check_return_position_impl_trait_in_trait_bounds`. That's because the RPITIT projections are normalized to opaques during wfcheck. Second, fix RPITIT's param-envs by not adding the projection predicates that we install on trait methods to make default RPITITs work -- I left a comment why. 3. Also, just a small drive-by for `rustc_on_unimplemented`. Not sure if it affects any tests, but can't hurt. r? `@spastorino,` based off of rust-lang#109140

…-body, r=spastorino Install projection from RPITIT to default trait method opaque correctly 1. For new lowering strategy `-Zlower-impl-trait-in-trait-to-assoc-ty`, install the correct default trait method projection predicates (RPITIT -> opaque). This makes default trait body tests pass! 2. Fix two WF-checking bugs -- first, we want to make sure that we're always looking for an opaque type in `check_return_position_impl_trait_in_trait_bounds`. That's because the RPITIT projections are normalized to opaques during wfcheck. Second, fix RPITIT's param-envs by not adding the projection predicates that we install on trait methods to make default RPITITs work -- I left a comment why. 3. Also, just a small drive-by for `rustc_on_unimplemented`. Not sure if it affects any tests, but can't hurt. r? ``@spastorino,`` based off of rust-lang#109140

…-body, r=spastorino Install projection from RPITIT to default trait method opaque correctly 1. For new lowering strategy `-Zlower-impl-trait-in-trait-to-assoc-ty`, install the correct default trait method projection predicates (RPITIT -> opaque). This makes default trait body tests pass! 2. Fix two WF-checking bugs -- first, we want to make sure that we're always looking for an opaque type in `check_return_position_impl_trait_in_trait_bounds`. That's because the RPITIT projections are normalized to opaques during wfcheck. Second, fix RPITIT's param-envs by not adding the projection predicates that we install on trait methods to make default RPITITs work -- I left a comment why. 3. Also, just a small drive-by for `rustc_on_unimplemented`. Not sure if it affects any tests, but can't hurt. r? ```@spastorino,``` based off of rust-lang#109140

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#108958 (Remove box expressions from HIR) - rust-lang#109044 (Prevent stable `libtest` from supporting `-Zunstable-options`) - rust-lang#109155 (Fix riscv64 fuchsia LLVM target name) - rust-lang#109156 (Fix linker detection for clang with prefix) - rust-lang#109181 (inherit_overflow: adapt pattern to also work with v0 mangling) - rust-lang#109198 (Install projection from RPITIT to default trait method opaque correctly) - rust-lang#109215 (Use sort_by_key instead of sort_by) - rust-lang#109229 (Fix invalid markdown link references) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

…er-errors Add revisions for -Zlower-impl-trait-in-trait-to-assoc-ty fixed tests Needs to go on top of rust-lang#109198 r? `@compiler-errors`

…er-errors Add revisions for -Zlower-impl-trait-in-trait-to-assoc-ty fixed tests Needs to go on top of rust-lang#109198 r? ``@compiler-errors``

spastorino added 6 commits March 15, 2023 12:27

Rename impl_trait_in_trait_parent to impl_trait_in_trait_parent_fn

26c4c1e

Properly implement generics_of for traits

39ffe96

Make impl_trait_in_trait_container consider newly generated RPITITs

39d19ca

ImplTraitPlaceholder -> is_impl_trait_in_trait

e41491f

Feed is_type_alias_impl_trait for RPITITs on the trait side

11f1810

Add revisions to fixed tests in -Zlower-impl-trait-in-trait-to-assoc-ty

c5c4340

rustbot assigned spastorino Mar 16, 2023

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-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Mar 16, 2023

rustbot 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 Mar 16, 2023

compiler-errors added 3 commits March 16, 2023 01:56

Install projection from RPITIT to default trait method opaque correctly

0949da8

Don't install default opaque projection predicates in RPITIT associat…

ff7c3b8

…ed type's param-env

Fix on_unimplemented_note for RPITITs

8d922eb

compiler-errors force-pushed the new-rpitit-default-body branch from e41f976 to 8d922eb Compare March 16, 2023 01:59

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 16, 2023

compiler-errors mentioned this pull request Mar 16, 2023

Add revisions for -Zlower-impl-trait-in-trait-to-assoc-ty fixed tests #109193

Merged

spastorino reviewed Mar 16, 2023

View reviewed changes

compiler-errors mentioned this pull request Mar 16, 2023

Fix generics_of on traits new RPITITs and also migrate from ImplTraitPlaceholder to is_impl_trait_in_trait #109140

Closed

bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 16, 2023

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Mar 16, 2023

matthiaskrgr mentioned this pull request Mar 16, 2023

Rollup of 9 pull requests #109230

Closed

matthiaskrgr mentioned this pull request Mar 17, 2023

Rollup of 8 pull requests #109242

Closed

This was referenced Mar 17, 2023

Rollup of 9 pull requests #109252

Closed

Rollup of 8 pull requests #109253

Merged

bors merged commit 246d989 into rust-lang:master Mar 17, 2023

rustbot added this to the 1.70.0 milestone Mar 17, 2023

compiler-errors deleted the new-rpitit-default-body branch August 11, 2023 20:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Install projection from RPITIT to default trait method opaque correctly #109198

Install projection from RPITIT to default trait method opaque correctly #109198

compiler-errors commented Mar 16, 2023 •

edited

Loading

compiler-errors commented Mar 16, 2023

compiler-errors commented Mar 16, 2023

compiler-errors commented Mar 16, 2023

spastorino Mar 16, 2023

compiler-errors Mar 16, 2023

spastorino Mar 16, 2023

compiler-errors Mar 16, 2023

compiler-errors Mar 16, 2023

compiler-errors Mar 16, 2023 •

edited

Loading

spastorino Mar 16, 2023

spastorino Mar 16, 2023

compiler-errors Mar 16, 2023

spastorino commented Mar 16, 2023

compiler-errors commented Mar 16, 2023

bors commented Mar 16, 2023

Install projection from RPITIT to default trait method opaque correctly #109198

Install projection from RPITIT to default trait method opaque correctly #109198

Conversation

compiler-errors commented Mar 16, 2023 • edited Loading

compiler-errors commented Mar 16, 2023

compiler-errors commented Mar 16, 2023

compiler-errors commented Mar 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

compiler-errors Mar 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spastorino commented Mar 16, 2023

compiler-errors commented Mar 16, 2023

bors commented Mar 16, 2023

compiler-errors commented Mar 16, 2023 •

edited

Loading

compiler-errors Mar 16, 2023 •

edited

Loading