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

Introduce AliasKind::Inherent for inherent associated types #109410

Merged
merged 4 commits into from
May 8, 2023

Conversation

fmease
Copy link
Member

@fmease fmease commented Mar 20, 2023

Allows us to check (possibly generic) inherent associated types for well-formedness.
Type inference now also works properly.

Follow-up to #105961. Supersedes #108430.
Fixes #106722.
Fixes #108957.
Fixes #109768.
Fixes #109789.
Fixes #109790.

Not to be merged before #108860 (AliasKind::Weak).

CC @jackh726
r? @compiler-errors

@rustbot label T-types F-inherent_associated_types

@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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Mar 20, 2023
@rustbot
Copy link
Collaborator

rustbot commented Mar 20, 2023

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@rustbot rustbot added F-inherent_associated_types `#![feature(inherent_associated_types)]` T-types Relevant to the types team, which will review and decide on the PR/issue. labels Mar 20, 2023
Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

first round of comments, need to think more about some things like wf and implied bounds

compiler/rustc_middle/src/ty/sty.rs Show resolved Hide resolved
compiler/rustc_hir_analysis/src/coherence/orphan.rs Outdated Show resolved Hide resolved
compiler/rustc_hir_analysis/src/collect/item_bounds.rs Outdated Show resolved Hide resolved
@@ -395,6 +395,8 @@ pub(super) fn trait_explicit_predicates_and_bounds(
gather_explicit_predicates_of(tcx, def_id.to_def_id())
}

// FIXME(fmease): Needs investigation: Do & can we need to handle inherent associated types more
// ‘precisely’ here? E.g. to break cycles.
Copy link
Member

Choose a reason for hiding this comment

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

Would be nice to link to an issue or known-bug ui test that explains when we get cycles here.

@@ -196,6 +196,7 @@ fn insert_required_predicates_to_be_wf<'tcx>(
}
}

// FIXME(fmease): Do we need to handle inherent projections similarly here?
Copy link
Member

Choose a reason for hiding this comment

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

Probably? it would be nice to create a test exercising this behavior... maybe something like:

struct Foo;

impl Foo {
  type IAT<'a, T> = &'a T;
}

struct Bar<'a, T>(Foo::IAT<'a, T>);

Unless I'm getting this mixed up with other implicit outlives code 🙃

Copy link
Member Author

Choose a reason for hiding this comment

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

Right now I have a negative behavior test for type IAT<'a, T> = &'a T; (i.e. no implied bounds).

compiler/rustc_trait_selection/src/traits/project.rs Outdated Show resolved Hide resolved
compiler/rustc_trait_selection/src/traits/project.rs Outdated Show resolved Hide resolved
compiler/rustc_trait_selection/src/traits/project.rs Outdated Show resolved Hide resolved
compiler/rustc_trait_selection/src/traits/project.rs Outdated Show resolved Hide resolved
src/librustdoc/clean/mod.rs Outdated Show resolved Hide resolved
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@fmease

This comment was marked as resolved.

@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 21, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 21, 2023
…-in-self-ty, r=compiler-errors

Use region-erased self type during IAT selection

Split off from rust-lang#109410 as discussed.
Fixes rust-lang#109299.

Re UI test: I use a reproducer of rust-lang#109299 that contains a name resolution error instead of reproducer [`regionck-2.rs`](https://github.com/rust-lang/rust/blob/fc7ed4af165c27ab5914b93251194f826920cc65/tests/ui/associated-inherent-types/regionck-2.rs) (as found in the `AliasKind::Inherent` PR) since it would (incorrectly) pass typeck in this PR due to the lack of regionck and I'd rather not make *that* a regression test (with or without `known-bug`).

`@rustbot` label F-inherent_associated_types

r? `@compiler-errors`
@compiler-errors
Copy link
Member

compiler-errors commented Mar 22, 2023

Noting so I don't forget -- @fmease can you change these FIXME(fmease) to FIXME(inherent-associated-types)? It's best to be able to search feature-related FIXMEs by their gate name, so that they can, for example, be revisited during stabilization, or if someone just wants to explore what still needs to be answered for IATs.

Of course, the ones that are really notes-to-self are fine, but ones that are follow-ups for the actual feature itself could be changed. I didn't look too hard so if they're all just notes-to-self that's fine too.

@fmease
Copy link
Member Author

fmease commented Mar 22, 2023

can you change these FIXME(fmease) to FIXME(inherent-associated-types)

👍

I tagged most of the FIXMEs with my name to signal that I want to see them solved in this PR. However, there are definitely several of those that should instead be inherent_associated_types.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 22, 2023
…-in-self-ty, r=compiler-errors

Use region-erased self type during IAT selection

Split off from rust-lang#109410 as discussed.
Fixes rust-lang#109299.

Re UI test: I use a reproducer of rust-lang#109299 that contains a name resolution error instead of reproducer [`regionck-2.rs`](https://github.com/rust-lang/rust/blob/fc7ed4af165c27ab5914b93251194f826920cc65/tests/ui/associated-inherent-types/regionck-2.rs) (as found in the `AliasKind::Inherent` PR) since it would (incorrectly) pass typeck in this PR due to the lack of regionck and I'd rather not make *that* a regression test (with or without `known-bug`).

``@rustbot`` label F-inherent_associated_types

r? ``@compiler-errors``
@bors
Copy link
Contributor

bors commented Mar 24, 2023

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

@rustbot rustbot added the A-rustdoc-json Area: Rustdoc JSON backend label Mar 29, 2023
@rustbot
Copy link
Collaborator

rustbot commented Mar 29, 2023

Some changes occurred in src/librustdoc/clean/types.rs

cc @camelid

rustdoc-json-types is a public (although nightly-only) API. If possible, consider changing src/librustdoc/json/conversions.rs; otherwise, make sure you bump the FORMAT_VERSION constant.

cc @CraftSpider, @aDotInTheVoid, @Enselic, @obi1kenobi

@compiler-errors
Copy link
Member

@bors r+

This is fine for now, given that this is probably not the final approach and we've identified all of the existing limitations with FIXMEs that should be reviewed sometime between now and stabilization (which I assume is far off in the future lol).

@bors
Copy link
Contributor

bors commented May 8, 2023

📌 Commit cd6dec3 has been approved by compiler-errors

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 May 8, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request May 8, 2023
…mpiler-errors

Rollup of 6 pull requests

Successful merges:

 - rust-lang#104070 (Prevent aborting guard from aborting the process in a forced unwind)
 - rust-lang#109410 (Introduce `AliasKind::Inherent` for inherent associated types)
 - rust-lang#111004 (Migrate `mir_transform` to translatable diagnostics)
 - rust-lang#111118 (Suggest struct when we get colon in fileds in enum)
 - rust-lang#111170 (Diagnostic args are still args if they're documented)
 - rust-lang#111354 (Fix miscompilation when calling default methods on `Future`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 29ac429 into rust-lang:master May 8, 2023
@rustbot rustbot added this to the 1.71.0 milestone May 8, 2023
@fmease fmease deleted the iat-alias-kind-inherent branch May 9, 2023 00:08
aDotInTheVoid added a commit to rust-lang/rustdoc-types that referenced this pull request May 13, 2023
Enselic added a commit to cargo-public-api/cargo-public-api that referenced this pull request May 13, 2023
The bump adds rustdoc JSON support for inherent associated types [1].

Note that that is only supported in unstable Rust, so it is too early
for us to add a regression test.

[1]: rust-lang/rust#109410
Enselic added a commit to cargo-public-api/cargo-public-api that referenced this pull request May 13, 2023
The bump adds rustdoc JSON support for inherent associated types [1].

Note that that is only supported in unstable Rust, so it is too early
for us to add a regression test.

[1]: rust-lang/rust#109410
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 14, 2023
…illaumeGomez

rustdoc-json: Add tests for `#![feature(inherent_associated_types)]`

Follow up to rust-lang#109410, CC `@fmease`

r? `@GuillaumeGomez`
Enselic added a commit to cargo-public-api/cargo-public-api that referenced this pull request May 14, 2023
The bump adds rustdoc JSON support for inherent associated types [1].

Note that that is only supported in unstable Rust, so it is too early
for us to add a regression test.

[1]: rust-lang/rust#109410
Enselic added a commit to cargo-public-api/cargo-public-api that referenced this pull request May 14, 2023
The bump adds rustdoc JSON support for inherent associated types [1].

Note that that is only supported in unstable Rust, so it is too early
for us to add a regression test.

[1]: rust-lang/rust#109410
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request May 18, 2023
…s, r=compiler-errors

Exclude inherent projections from some alias type `match`es

Updating (hopefully) all remaining `match`es which I overlooked to update when adding `AliasKind::Inherent` in rust-lang#109410.

Fixes rust-lang#111399.
Sadly the regression test is a clippy test instead of a rustc one as I don't know of another way to test that a trait bound like `Ty::InhProj: Trait` doesn't cause a crash without reaching a cycle error first (this is getting old ^^').

`@rustbot` label F-inherent_associated_types
r? `@compiler-errors`
flip1995 pushed a commit to flip1995/rust that referenced this pull request May 20, 2023
…compiler-errors

Introduce `AliasKind::Inherent` for inherent associated types

Allows us to check (possibly generic) inherent associated types for well-formedness.
Type inference now also works properly.

Follow-up to rust-lang#105961. Supersedes rust-lang#108430.
Fixes rust-lang#106722.
Fixes rust-lang#108957.
Fixes rust-lang#109768.
Fixes rust-lang#109789.
Fixes rust-lang#109790.

~Not to be merged before rust-lang#108860 (`AliasKind::Weak`).~

CC `@jackh726`
r? `@compiler-errors`

`@rustbot` label T-types F-inherent_associated_types
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-json Area: Rustdoc JSON backend F-inherent_associated_types `#![feature(inherent_associated_types)]` 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
7 participants