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.

Sign up for GitHub

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

Jump to bottom

Deny associated type bindings within associated type bindings #102338

Merged
merged 1 commit into from
Sep 28, 2022
Merged

Deny associated type bindings within associated type bindings #102338

merged 1 commit into from
Sep 28, 2022

Conversation

compiler-errors
Copy link
Member

Fixes #102335

This was made worse by #100865, which unified the way we generate substs for GATs and non-generic associated types. However, the issue was not caused by #100865, evidenced by the test I added for GATs:

trait T {
    type A: S<C<(), i32 = ()> = ()>;
    //~^ ERROR associated type bindings are not allowed here
}

trait Q {}

trait S {
    type C<T>: Q;
}

fn main() {}

^ which passes on beta (where GATs are stable) and presumably ever since GATs support was added to create_substs_for_associated_item in astconv.

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 27, 2022
@rust-highfive
Copy link
Collaborator

r? @jackh726

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 27, 2022
@compiler-errors
Copy link
Member Author

r? @cjgillot since you reviewed #100865, feel free to re-roll though

@rust-highfive rust-highfive assigned cjgillot and unassigned jackh726 Sep 27, 2022
@fmease
Copy link
Member

fmease commented Sep 27, 2022

which passes on beta (where GATs are stable)

Does this mean we should backport this PR to beta?

@compiler-errors
Copy link
Member Author

Probably.

@compiler-errors compiler-errors added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Sep 27, 2022
@bors
Copy link
Contributor

bors commented Sep 27, 2022

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

@compiler-errors compiler-errors force-pushed the assoc-ty-binding-in-assoc-ty-binding branch from 5ee0422 to ca2e0bb Compare September 27, 2022 17:41
cjgillot
cjgillot reviewed Sep 27, 2022
View reviewed changes
compiler/rustc_hir_analysis/src/astconv/mod.rs
);

let assoc_bindings = self.create_assoc_bindings_for_generic_args(item_segment.args());
if let Some(b) = assoc_bindings.first() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I seems wasteful to convert all bindings, just to err on the first one later. This could as well be implemented as
if let Some(b) = item_segment.args().bindings.first()
Probably better as a follow-up, as there are several instances of this pattern in the file.

@cjgillot
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Sep 27, 2022

📌 Commit ca2e0bb has been approved by cjgillot

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 Sep 27, 2022
@JohnTitor JohnTitor mentioned this pull request Sep 28, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 28, 2022
@bors
Auto merge of rust-lang#102388 - JohnTitor:rollup-mbyw6fl, r=JohnTitor
837bf37 
Rollup of 8 pull requests

Successful merges:

 - rust-lang#100747 (Add long description and test for E0311)
 - rust-lang#102232 (Stabilize bench_black_box)
 - rust-lang#102288 (Suggest unwrapping `???<T>` if a method cannot be found on it but is present on `T`.)
 - rust-lang#102338 (Deny associated type bindings within associated type bindings)
 - rust-lang#102347 (Unescaping cleanups)
 - rust-lang#102348 (Tweak `FulfillProcessor`.)
 - rust-lang#102378 (Use already resolved `self_ty` in `confirm_fn_pointer_candidate`)
 - rust-lang#102380 (rustdoc: remove redundant mobile `.source > .sidebar` CSS)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b263b7e into rust-lang:master Sep 28, 2022
@rustbot rustbot added this to the 1.66.0 milestone Sep 28, 2022
@matthiaskrgr
Copy link
Member

The test crashes in rustdoc apparently 😅 #102467

@apiraino
Copy link
Contributor

Beta backport accepted as per compiler team on Zulip

@rustbot label +beta-accepted

@rustbot rustbot added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Sep 29, 2022
@compiler-errors compiler-errors mentioned this pull request Sep 29, 2022
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 30, 2022
@matthiaskrgr
Rollup merge of rust-lang#102492 - compiler-errors:simplify-deny-asso…
842a7d3 
…c-bindings, r=cjgillot

Don't lower assoc bindings just to deny them

Some clean-up: rust-lang#102338 (comment)
@cuviper cuviper mentioned this pull request Oct 4, 2022
Merged
@cuviper cuviper removed this from the 1.66.0 milestone Oct 4, 2022
@cuviper cuviper added this to the 1.65.0 milestone Oct 4, 2022
@cuviper cuviper removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Oct 4, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 10, 2022
@bors
Auto merge of rust-lang#102676 - cuviper:beta-next, r=cuviper
da7ffa2 
[beta] backports

* Avoid duplicating StorageLive in let-else rust-lang#101894
* Re-add HRTB implied static bug note rust-lang#101924
* Revert "Copy stage0 binaries into stage0-sysroot" rust-lang#101942
* implied_bounds: deal with inference vars rust-lang#102016
* fix ConstProp handling of written_only_inside_own_block_locals rust-lang#102045
* Fix wrongly refactored Lift impl rust-lang#102088
* Fix a typo “pararmeter” in error message rust-lang#102119
* Deny associated type bindings within associated type bindings rust-lang#102338
* Continue migration of CSS themes rust-lang#101934
* Fix search result colors rust-lang#102369
* Fix unwind drop glue for if-then scopes rust-lang#102394
* Revert "Use getentropy when possible on all Apple platforms" rust-lang#102693
* Fix associated type bindings with anon const in GAT position rust-lang#102336
* Revert perf-regression 101620 rust-lang#102064
* `EscapeAscii` is not an `ExactSizeIterator` rust-lang#99880
@compiler-errors compiler-errors deleted the assoc-ty-binding-in-assoc-ty-binding branch November 2, 2022 02:56
Aaron1011 pushed a commit to Aaron1011/rust that referenced this pull request Jan 6, 2023
@JohnTitor
Rollup merge of rust-lang#102338 - compiler-errors:assoc-ty-binding-i…
b9c8fb0 
…n-assoc-ty-binding, r=cjgillot

Deny associated type bindings within associated type bindings

Fixes rust-lang#102335

This was made worse by rust-lang#100865, which unified the way we generate substs for GATs and non-generic associated types. However, the issue was not _caused_ by rust-lang#100865, evidenced by the test I added for GATs:

```rust
trait T {
    type A: S<C<(), i32 = ()> = ()>;
    //~^ ERROR associated type bindings are not allowed here
}

trait Q {}

trait S {
    type C<T>: Q;
}

fn main() {}
```

^ which passes on beta (where GATs are stable) and presumably ever since GATs support was added to `create_substs_for_associated_item` in astconv.
Aaron1011 pushed a commit to Aaron1011/rust that referenced this pull request Jan 6, 2023
@bors
Auto merge of rust-lang#102388 - JohnTitor:rollup-mbyw6fl, r=JohnTitor
cf62e8a 
Rollup of 8 pull requests

Successful merges:

 - rust-lang#100747 (Add long description and test for E0311)
 - rust-lang#102232 (Stabilize bench_black_box)
 - rust-lang#102288 (Suggest unwrapping `???<T>` if a method cannot be found on it but is present on `T`.)
 - rust-lang#102338 (Deny associated type bindings within associated type bindings)
 - rust-lang#102347 (Unescaping cleanups)
 - rust-lang#102348 (Tweak `FulfillProcessor`.)
 - rust-lang#102378 (Use already resolved `self_ty` in `confirm_fn_pointer_candidate`)
 - rust-lang#102380 (rustdoc: remove redundant mobile `.source > .sidebar` CSS)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjgillot cjgillot cjgillot left review comments

Assignees

@cjgillot cjgillot

Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. 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
Milestone
1.65.0
Development

Successfully merging this pull request may close these issues.

Compiler accepts const equality bound involving nonsensical nested equality constraint on associated const
10 participants
@compiler-errors @rust-highfive @fmease @bors @cjgillot @matthiaskrgr @apiraino @cuviper @jackh726 @rustbot