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

Filter out RPITITs when checking for missing associated types #108869

Closed
wants to merge 2 commits into from

Conversation

spastorino
Copy link
Member

@spastorino spastorino commented Mar 7, 2023

This goes on top of other already approved PRs not merged yet. Going to rebase when they are merged. The only useful commit is the last one faa15d2

r? @compiler-errors

The included test is a copy of tests/ui/impl-trait/in-trait/issue-102140.rs which fails with the -Zlower-impl-trait-in-trait-to-assoc-ty currently.

Current master does the following ...

error[E0191]: the value of the associated type `` (from trait `MyTrait`) must be specified
  --> tests/ui/impl-trait/in-trait/new-lowering-strategy/trait-object-impl-trait.rs:23:10
   |
10 |     fn foo(&self) -> impl Marker
   |                      ----------- `` defined here
...
23 | impl dyn MyTrait {
   |          ^^^^^^^ help: specify the associated type: `MyTrait< = Type>`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0191`.

And this PR behaves correctly, same output as current RPITIT code ...

gh-spastorino@dev-desktop-us-1:~/rust4$ rustc +rust4-stage1 tests/ui/impl-trait/in-trait/new-lowering-strategy/trait-object-impl-trait.rs
error[E0277]: the trait bound `&dyn MyTrait: MyTrait` is not satisfied
  --> tests/ui/impl-trait/in-trait/new-lowering-strategy/trait-object-impl-trait.rs:23:22
   |
23 |         MyTrait::foo(&self)
   |         ------------ ^^^^^ the trait `MyTrait` is not implemented for `&dyn MyTrait`
   |         |
   |         required by a bound introduced by this call
   |
help: consider removing the leading `&`-reference
   |
23 -         MyTrait::foo(&self)
23 +         MyTrait::foo(self)
   |

error[E0277]: the trait bound `&dyn MyTrait: MyTrait` is not satisfied
  --> tests/ui/impl-trait/in-trait/new-lowering-strategy/trait-object-impl-trait.rs:23:9
   |
23 |         MyTrait::foo(&self)
   |         ^^^^^^^^^^^^^^^^^^^ the trait `MyTrait` is not implemented for `&dyn MyTrait`
   |
   = help: the trait `MyTrait` is implemented for `Outer`

error[E0277]: the trait bound `&dyn MyTrait: MyTrait` is not satisfied
  --> tests/ui/impl-trait/in-trait/new-lowering-strategy/trait-object-impl-trait.rs:23:9
   |
23 |         MyTrait::foo(&self)
   |         ^^^^^^^^^^^^ the trait `MyTrait` is not implemented for `&dyn MyTrait`
   |
   = help: the trait `MyTrait` is implemented for `Outer`

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0277`.

@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. labels Mar 7, 2023
@spastorino
Copy link
Member Author

Added another commit ef950b8

That commit included test is a copy of tests/ui/async-await/in-trait/object-safety.rs which fails with the -Zlower-impl-trait-in-trait-to-assoc-ty currently.

Current master does the following ...

warning: the feature `async_fn_in_trait` is incomplete and may not be safe to use and/or cause compiler crashes
 --> tests/ui/async-await/in-trait/object-safety.rs:3:12
  |
3 | #![feature(async_fn_in_trait)]
  |            ^^^^^^^^^^^^^^^^^
  |
  = note: see issue #91611 <https://github.com/rust-lang/rust/issues/91611> for more information
  = note: `#[warn(incomplete_features)]` on by default

error[E0038]: the trait `Foo` cannot be made into an object
  --> tests/ui/async-await/in-trait/object-safety.rs:11:12
   |
11 |     let x: &dyn Foo = todo!();
   |            ^^^^^^^^ `Foo` cannot be made into an object
   |
note: for a trait to be "object safe" it needs to allow building a vtable to allow the call to be resolvable dynamically; for more information visit <https://doc.rust-lang.org/reference/items/traits.html#object-safety>
  --> tests/ui/async-await/in-trait/object-safety.rs:7:14
   |
6  | trait Foo {
   |       --- this trait cannot be made into an object...
7  |     async fn foo(&self);
   |              ^^^       ^ ...because it contains the generic associated type ``
   |              |
   |              ...because method `foo` is `async`
   = help: consider moving `foo` to another trait
   = help: consider moving `` to another trait

error: aborting due to previous error; 1 warning emitted

For more information about this error, try `rustc --explain E0038`.

And this PR behaves correctly, same output as current RPITIT code ...

warning: the feature `async_fn_in_trait` is incomplete and may not be safe to use and/or cause compiler crashes
 --> tests/ui/async-await/in-trait/object-safety.rs:3:12
  |
3 | #![feature(async_fn_in_trait)]
  |            ^^^^^^^^^^^^^^^^^
  |
  = note: see issue #91611 <https://github.com/rust-lang/rust/issues/91611> for more information
  = note: `#[warn(incomplete_features)]` on by default

error[E0038]: the trait `Foo` cannot be made into an object
  --> tests/ui/async-await/in-trait/object-safety.rs:11:12
   |
11 |     let x: &dyn Foo = todo!();
   |            ^^^^^^^^ `Foo` cannot be made into an object
   |
note: for a trait to be "object safe" it needs to allow building a vtable to allow the call to be resolvable dynamically; for more information visit <https://doc.rust-lang.org/reference/items/traits.html#object-safety>
  --> tests/ui/async-await/in-trait/object-safety.rs:7:14
   |
6  | trait Foo {
   |       --- this trait cannot be made into an object...
7  |     async fn foo(&self);
   |              ^^^ ...because method `foo` is `async`
   = help: consider moving `foo` to another trait

error: aborting due to previous error; 1 warning emitted

For more information about this error, try `rustc --explain E0038`.

@spastorino spastorino force-pushed the new-rpitit-5 branch 2 times, most recently from 98077b2 to 6763e57 Compare March 7, 2023 20:28
@spastorino spastorino marked this pull request as ready for review March 7, 2023 20:29
@spastorino
Copy link
Member Author

https://github.com/rust-lang/rust/blob/171aa24e8faeca0bb0a478f32f84b032c1fdda37/tests/ui/impl-trait/in-trait/object-safety.rs

This test needs a revision.

I didn't properly investigate this test but it still not working even with this 2 commits. So I'd leave it for a followup.

@spastorino
Copy link
Member Author

Getting thread 'rustc' panicked at 'called Option::unwrap()on aNone value', compiler/rustc_middle/src/ty/context.rs:2436:55 from CI. For tests/ui/impl-trait/in-trait/issue-102140.rs#next which is weird to me given that I was able to bless it locally https://github.com/rust-lang/rust/pull/108869/files#diff-66bc1ebad57c8b27e859819ec19c1c7a319b2d02c796bfb0864fa62ff2fd1ca3

@compiler-errors
Copy link
Member

Possibly debug assertions. Drop the impl trait test and just do the async test, squash, then r=me.

@spastorino
Copy link
Member Author

spastorino commented Mar 7, 2023

Possibly debug assertions. Drop the impl trait test and just do the async test, squash, then r=me.

Ohh, this is likely due to something that is already fixed in #108700. Maybe it's better if we wait for it?.

@compiler-errors
Copy link
Member

Up to you.

@spastorino
Copy link
Member Author

I'll wait for it, rebase and r=you if that's fine after I see everything green.

@compiler-errors
Copy link
Member

compiler-errors commented Mar 7, 2023

yeah, either do what I said in #108869 (comment), or rebase + bless. r=me in that case, as long as the test stderrs are equal with/without the -Z flag (and also please check if the impl-trait/in-trait/object-safety.rs test passes). If the stderr has changed, re-request review from me or ping me or something.

@spastorino
Copy link
Member Author

Placing it again on top of existing unmerged PRs, to see if CI is happy.

@spastorino
Copy link
Member Author

@compiler-errors, this one is ready, rebased and ready for a final review.

@compiler-errors
Copy link
Member

closed this one since you said you just wanted to land the other pr together as one stack

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 14, 2023
…r-errors

Fix object safety checks for new RPITITs

This one goes on top of rust-lang#108869

r? `@compiler-errors`
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants