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

Ban references to Self in trait object substs for projection predicates too. #100500

Merged
merged 3 commits into from
Aug 20, 2022

Conversation

cjgillot
Copy link
Contributor

Fixes #100484
Fixes #100485

r? @lcnr

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 13, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 13, 2022
// Like for trait refs, verify that `dummy_self` did not leak inside default type
// parameters.
let references_self = b.projection_ty.substs.iter().skip(1).any(|arg| {
if let ty::GenericArgKind::Type(ty) = arg.unpack() {
Copy link
Contributor

Choose a reason for hiding this comment

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

this feels incorrect for const generics. What happens if you have a generic constant referencing self? (using feature(generic_const_exprs))

a generic constant not using self would be even worse 😅 because it still has Self in its generic arguments even if unused.

Copy link
Contributor

Choose a reason for hiding this comment

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

were there any problems with using arg.walk instead of ty.walk. Also, the generic arg itself should get returned as the first item of walk, so the explicit ty == dummy_self check shouldn't be necessary

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you have a test in mind for this?

Copy link
Contributor

Choose a reason for hiding this comment

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

use feature(generic_const_exprs) and have a Trait<{ N + 1 } in it, or even Trait<{ 1 + 2 }>.

can look into making a full example if you want, am done for today 😅

@lcnr
Copy link
Contributor

lcnr commented Aug 18, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Aug 18, 2022

📌 Commit 72acd94 has been approved by lcnr

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 Aug 18, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 20, 2022
…lcnr

Ban references to `Self` in trait object substs for projection predicates too.

Fixes rust-lang#100484
Fixes rust-lang#100485

r? `@lcnr`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 20, 2022
…lcnr

Ban references to `Self` in trait object substs for projection predicates too.

Fixes rust-lang#100484
Fixes rust-lang#100485

r? ``@lcnr``
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 20, 2022
…lcnr

Ban references to `Self` in trait object substs for projection predicates too.

Fixes rust-lang#100484
Fixes rust-lang#100485

r? ```@lcnr```
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 20, 2022
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#97963 (net listen backlog set to negative on Linux.)
 - rust-lang#99935 (Reenable disabled early syntax gates as future-incompatibility lints)
 - rust-lang#100129 (add miri-test-libstd support to libstd)
 - rust-lang#100500 (Ban references to `Self` in trait object substs for projection predicates too.)
 - rust-lang#100636 (Revert "Revert "Allow dynamic linking for iOS/tvOS targets."")
 - rust-lang#100718 ([rustdoc] Fix item info display)
 - rust-lang#100769 (Suggest adding a reference to a trait assoc item)
 - rust-lang#100777 (elaborate how revisions work with FileCheck stuff in src/test/codegen)
 - rust-lang#100796 (Refactor: remove unnecessary string searchings)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 33a4029 into rust-lang:master Aug 20, 2022
@rustbot rustbot added this to the 1.65.0 milestone Aug 20, 2022
@cjgillot cjgillot deleted the verify-self-predicate branch August 21, 2022 08:59
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.
Projects
None yet
5 participants