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

TAIT decision on "may define implies must define" #117861

Closed
traviscross opened this issue Nov 13, 2023 · 12 comments · Fixed by #113169
Closed

TAIT decision on "may define implies must define" #117861

traviscross opened this issue Nov 13, 2023 · 12 comments · Fixed by #113169
Labels
A-impl-trait Area: impl Trait. Universally / existentially quantified anonymous types with static dispatch. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` finished-final-comment-period The final comment period is finished for this PR / Issue. T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@traviscross
Copy link
Contributor

traviscross commented Nov 13, 2023

Nominating for T-lang to decide whether to adopt the following restriction for the stabilization of TAIT:

At least until the new trait solver is stabilized, any item that is allowed to define the hidden type of some opaque type must define the hidden type of that opaque type.

If adopted, we would call this the "may define implies must define" restriction.

For the purposes of this question, please set aside how we know whether an item is allowed to define the hidden type; this question is invariant to the details of that.

This restriction leaves space for the new trait solver. That solver will consider more uses of the opaque type to be defining and it will implement lazy normalization. Given this, items that may define the hidden type but do not present the potential for differences between the old and new solver that we would prefer to avoid so as to ease the stabilization of the new trait solver.

With this restriction, TAIT becomes roughly equivalent to RPIT with respect to many of the relevant challenges.

The effect of this restriction would be to reject the following code:

#![feature(type_alias_impl_trait)]

type Tait = impl Sized;
fn define() -> Tait {} // Defines the hidden type.
fn passthrough(x: Tait) -> Tait { // Allowed to define but does not.
    // ~^ ERROR Any item that may define a hidden type must define that hidden type.
    x
}

Please set aside how we know that define and passthrough are both allowed to define the hidden type of the opaque. There are many ways that could be notated, and that question is orthogonal to this restriction.

Whether or not this restriction could be lifted later is a question of what is possible under the new trait solver and of what other rules we might decide to adopt that may make this more or less possible. Adopting this restriction now does not in and of itself close the door to possibly permitting this later.

This rule was described and proposed in the 2023-11-08 T-lang Mini-TAIT design meeting (minutes): rust-lang/lang-team#233.

Related

@traviscross traviscross added T-lang Relevant to the language team, which will review and decide on the PR/issue. A-impl-trait Area: impl Trait. Universally / existentially quantified anonymous types with static dispatch. F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` I-lang-nominated Nominated for discussion during a lang team meeting. labels Nov 13, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Nov 13, 2023
@traviscross traviscross removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Nov 13, 2023
@lcnr
Copy link
Contributor

lcnr commented Feb 1, 2024

This issue has been lang nominated for 3 months and is adding uncertainty to the new trait solver work and blocking refactorings of the old solver. I currently do not know how to sensibly support non-defining uses of opaque types inside of their defining scope.

As an alternative to needing to fully decide this as @rust-lang/lang, it would also work to delegate officially delegate this decision to t-types instead. Discussing why this restriction exists and how or whether it could be avoided seems very much like the responsibility of the types team after all.

@oli-obk oli-obk added T-types Relevant to the types team, which will review and decide on the PR/issue. and removed T-lang Relevant to the language team, which will review and decide on the PR/issue. I-lang-nominated Nominated for discussion during a lang team meeting. labels Mar 7, 2024
@oli-obk
Copy link
Contributor

oli-obk commented Mar 7, 2024

I'm going to overrule any language concerns due to critical implementation concerns, which thoroughly puts this into T-types territory. T-lang can still review this and give advice or opinions when they get to this issue.

@oli-obk oli-obk added the I-types-nominated Nominated for discussion during a types team meeting. label Mar 7, 2024
@oli-obk

This comment was marked as off-topic.

@oli-obk oli-obk removed the I-types-nominated Nominated for discussion during a types team meeting. label May 27, 2024
@oli-obk
Copy link
Contributor

oli-obk commented May 27, 2024

@rfcbot merge

@rfcbot
Copy link

rfcbot commented May 27, 2024

Team member @oli-obk has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels May 27, 2024
@jackh726
Copy link
Member

What's the decision here? To adopt to rule as stated in the OP?

@oli-obk
Copy link
Contributor

oli-obk commented May 27, 2024

Yes.

@lcnr
Copy link
Contributor

lcnr commented May 27, 2024

in case somebody wants to look at the cases where non-defining uses in the defining scope cause issues1:

Footnotes

  1. i mostly opened these issues to avoid forgetting about them myself, they lack a lot of background info

@nikomatsakis
Copy link
Contributor

nikomatsakis commented May 29, 2024 via email

@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels May 29, 2024
@rfcbot
Copy link

rfcbot commented May 29, 2024

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Jun 8, 2024
@rfcbot
Copy link

rfcbot commented Jun 8, 2024

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@rfcbot rfcbot added the to-announce Announce this issue on triage meeting label Jun 8, 2024
@oli-obk oli-obk closed this as completed Jun 10, 2024
@oli-obk
Copy link
Contributor

oli-obk commented Jun 10, 2024

Actually, need to finish the impl that actually enforces this

@oli-obk oli-obk reopened this Jun 10, 2024
@bors bors closed this as completed in 02c7a59 Jun 12, 2024
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-impl-trait Area: impl Trait. Universally / existentially quantified anonymous types with static dispatch. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` finished-final-comment-period The final comment period is finished for this PR / Issue. T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants