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

Fix impl_trait_ty_to_ty substs for -Zlower-impl-trait-in-trait-to-assoc-ty #109244

Closed
wants to merge 1 commit into from

Conversation

spastorino
Copy link
Member

@spastorino spastorino commented Mar 17, 2023

impl_trait_ty_to_ty was assuming that all substs were about lifetimes and this is not true anymore with the new RPITITs synthesized as associated types.

The only relevant commit is bebb98b.

r? @compiler-errors

@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-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Mar 17, 2023
@spastorino
Copy link
Member Author

@rustbot author

I'd like to check this out a bit better but feel free to leave thoughts.

@rustbot 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 17, 2023
@spastorino spastorino force-pushed the new-rpitit-13 branch 2 times, most recently from ed4eecc to 593d6af Compare March 17, 2023 03:35
@rust-log-analyzer

This comment has been minimized.

@spastorino spastorino force-pushed the new-rpitit-13 branch 4 times, most recently from 47991db to bebb98b Compare March 17, 2023 12:51
@spastorino
Copy link
Member Author

@rustbot review

@rustbot 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 17, 2023
@spastorino
Copy link
Member Author

spastorino commented Mar 17, 2023

@compiler-errors I think this commit goes in the right direction but the error that I'm getting in tests/ui/impl-trait/in-trait/opaque-in-impl.rs seems to suggest that something else is missing ...

error: internal compiler error: impl item and trait item have different parameters
  --> tests/ui/impl-trait/in-trait/opaque-in-impl.rs:29:5
   |
29 |     fn bar<T>(&self) -> impl Debug {
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

The code grabs the fn_def_id for both trait and impls but still check_substs_compatible fails when checking assoc types on trait and impl. I'm going to investigate.

@spastorino
Copy link
Member Author

spastorino commented Mar 17, 2023

@rustbot author

found the problem, generics_of is returning ...

params: [
    GenericParamDef { name: "T", def_id: DefId(0:9 ~ opaque_in_impl[513c]::{impl#0}::bar::T), index: 0, pure_wrt_drop: false, kind: Type { has_default: false, synthetic: false } }, 
    GenericParamDef { name: "T", def_id: DefId(0:6 ~ opaque_in_impl[513c]::Bar::bar::T), index: 1, pure_wrt_drop: false, kind: Type { has_default: false, synthetic: false } }
]

Which is clearly wrong. Fixing it ...

@rustbot 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 17, 2023
@spastorino
Copy link
Member Author

I think this can be merged because it's fixes a problem related to impl_trait_ty_to_ty, some new tests pass with this PR and the problem I've found is related to generics_of for the impl associated type which can be addressed in a different PR.

@rustbot review

@rustbot 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 17, 2023
@rustbot
Copy link
Collaborator

rustbot commented Mar 17, 2023

These commits modify the Cargo.lock file. Random changes to Cargo.lock can be introduced when switching branches and rebasing PRs.
This was probably unintentional and should be reverted before this PR is merged.

If this was intentional then you can ignore this comment.

@spastorino spastorino force-pushed the new-rpitit-13 branch 2 times, most recently from b23f2f1 to 8cde7f3 Compare March 17, 2023 18:45
@spastorino
Copy link
Member Author

@compiler-errors this one should be ready too.

@spastorino
Copy link
Member Author

#109277 includes this and other stuff.

@spastorino spastorino closed this Mar 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants