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

find a way to permit impls of CommonTypeWith where the LHS and RHS type overlap #1077

Closed
zygoloid opened this issue Feb 10, 2022 · 6 comments
Labels
leads question A question for the leads team

Comments

@zygoloid
Copy link
Contributor

zygoloid commented Feb 10, 2022

Per the current design, CommonTypeWith has an impl of this form:

final impl [T:! Type] T as CommonTypeWith(T) where .Result = T {}

Given this final impl, it's unclear how to write a parameterized impl of CommonTypeWith for two parameterized types that overlap. For example:

// Error: would specialize the above `final impl`.
impl [U:! Type, T:! CommonTypeWith(U)] Vec(T) as CommonTypeWith(Vec(U)) where .Result = Vec(T.Result) {}

The error here is unnecessary: there's no actual coherence problem here because in the case where T == U, the computed Result is Vec(T).

It's not clear how to resolve this. Some options:

  • Switch from final impl to a final associated constant.
  • Provide some way for the author of this impl to say that it's prioritized below the final impl.
  • Provide some way for the author of the final impl to say that it is considered more specialized than any other impl, rather than disallowing impls that overlap with it.
  • Add support for type inequality constraints so that the new impl could exclude the T == U case.
  • Allow a final impl to be specialized if the specialization is identical.
@josh11b
Copy link
Contributor

josh11b commented Feb 23, 2022

I don't think CommonTypeWith is a strong argument for this option, since the only reason final associated constants would help is that final associated constants would be likely to support "allow a final thing to be specialized if the specialization is identical."

  • Provide some way for the author of this impl to say that it's prioritized below the final impl.
  • Provide some way for the author of the final impl to say that it is considered more specialized than any other impl, rather than disallowing impls that overlap with it.

I think if we wanted to go in either of these two directions, I'd rather a final impl always be prioritized over other impls and give an error if that means some impl will never match. It seems like it would be better to have fewer options if we can get away with it, unless you have some use case when you would not want this behavior. The main counter argument is if you want to document in the source that an impl isn't going to apply in cases that you might otherwise expect it to.

I'm not excited about this, since we currently don't have a way of proving that two types are not equal.

  • Allow a final impl to be specialized if the specialization is identical.

I'd be fine with this option. It only resolves a relatively narrow set of cases similar to the CommonTypeWith situation, though, since we while we can compare associated constants to see if they are equal, I don't think we can expect to do that with functions.

@chandlerc
Copy link
Contributor

  • Allow a final impl to be specialized if the specialization is identical.

I'd be fine with this option. It only resolves a relatively narrow set of cases similar to the CommonTypeWith situation, though, since we while we can compare associated constants to see if they are equal, I don't think we can expect to do that with functions.

While I agree, this also specifically addresses the only downside to making the final impl be prioritized -- it restricts to the case where there is no need to document the exclusion.

What if we start here to see whether this ends up covering the cases where this is an issue? We can always revisit prioritizing the final in all cases if further use cases emerge.

@github-actions
Copy link

github-actions bot commented Jul 9, 2022

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please comment or remove the inactive label. The long term label can also be added for issues which are expected to take time.
This issue is labeled inactive because the last activity was over 90 days ago.

@github-actions github-actions bot added the inactive Issues and PRs which have been inactive for at least 90 days. label Jul 9, 2022
@jonmeow jonmeow added the leads question A question for the leads team label Aug 10, 2022
@github-actions github-actions bot removed the inactive Issues and PRs which have been inactive for at least 90 days. label Aug 11, 2022
@github-actions
Copy link

github-actions bot commented Nov 9, 2022

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please comment or remove the inactive label. The long term label can also be added for issues which are expected to take time.
This issue is labeled inactive because the last activity was over 90 days ago.

@github-actions github-actions bot added the inactive Issues and PRs which have been inactive for at least 90 days. label Nov 9, 2022
@josh11b
Copy link
Contributor

josh11b commented Nov 10, 2022

As far as I am aware, we would still like an answer to this question. An answer was proposed in #1077 (comment) but never accepted.

@josh11b josh11b removed the inactive Issues and PRs which have been inactive for at least 90 days. label Nov 10, 2022
@chandlerc
Copy link
Contributor

Yep, leads are happy with the resolution here: #1077 (comment)

zygoloid pushed a commit that referenced this issue Jun 2, 2023
Allow an `impl` to overlap with a `final impl` if they agree on the overlap. Agreement is defined as all values comparing equal, and functions never comparing equal.  Implements the decision in question-for-leads issue #1077.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
leads question A question for the leads team
Projects
None yet
Development

No branches or pull requests

4 participants