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

Termination algorithm for impl selection #2687

Merged
merged 11 commits into from
Apr 10, 2023
Merged

Conversation

josh11b
Copy link
Contributor

@josh11b josh11b commented Mar 16, 2023

This proposal replaces the termination algorithm for impl selection. The previous algorithm relied on a recursion limit, which is counter to our goal for predictability.

The replacement is to terminate if any impl declaration depends transitively on itself with a "strict superset" type as one of its parameters.

@josh11b josh11b added proposal A proposal proposal draft Proposal in draft, not ready for review labels Mar 16, 2023
@josh11b josh11b removed the proposal draft Proposal in draft, not ready for review label Mar 25, 2023
@josh11b josh11b marked this pull request as ready for review March 25, 2023 00:20
@github-actions github-actions bot added the proposal rfc Proposal with request-for-comment sent out label Mar 25, 2023
@josh11b josh11b requested review from zygoloid and removed request for KateGregory March 25, 2023 00:21
proposals/p2687.md Outdated Show resolved Hide resolved
proposals/p2687.md Outdated Show resolved Hide resolved
proposals/p2687.md Show resolved Hide resolved
proposals/p2687.md Outdated Show resolved Hide resolved
proposals/p2687.md Outdated Show resolved Hide resolved
proposals/p2687.md Outdated Show resolved Hide resolved
proposals/p2687.md Outdated Show resolved Hide resolved
proposals/p2687.md Outdated Show resolved Hide resolved
proposals/p2687.md Outdated Show resolved Hide resolved
proposals/p2687.md Outdated Show resolved Hide resolved
Copy link
Contributor

@zygoloid zygoloid left a comment

Choose a reason for hiding this comment

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

Looks good to me, taking to leads.

Copy link
Contributor

@chandlerc chandlerc left a comment

Choose a reason for hiding this comment

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

Didn't really dig into the specifics (hyperplanes???) but overall, this is awesome. =D Really happy with the result here and the write up looks great. Minor tweak suggested. I'll leave to @zygoloid to approve.

proposals/p2687.md Outdated Show resolved Hide resolved
Co-authored-by: Chandler Carruth <[email protected]>
@zygoloid zygoloid merged commit 31e9071 into carbon-language:trunk Apr 10, 2023
@josh11b josh11b deleted the term branch April 10, 2023 20:36
josh11b added a commit to josh11b/carbon-lang that referenced this pull request Sep 22, 2023
github-merge-queue bot pushed a commit that referenced this pull request Oct 3, 2023
Continued from part 1: #3231. Second step updating
`docs/design/generics/details.md`. There remains some work to
incorporate proposal #2200.

- The biggest changes are incorporating much of the text of proposals:
  - #2173
  - #2687
- It incorporates changes from proposals:
  - #989
  - #1178
  - #2138
  - #2200
  - #2360
  - #2964
  - #3162
- It also updates the text to reflect the latest thinking from leads
issues:
  - #996
  - #2153 -- most notably deleting the section on `TypeId`.
- Update to rule for prioritization blocks with mixed type structures
from [discussion on
2023-07-18](https://docs.google.com/document/d/1gnJBTfY81fZYvI_QXjwKk1uQHYBNHGqRLI2BS_cYYNQ/edit?resourcekey=0-ql1Q1WvTcDvhycf8LbA9DQ#heading=h.7jxges9ojgy3)
- Adds reference links to proposals, issues, and discussions relevant to
the text.
- Also tries to use more precise language when talking about
implementations, to avoid confusing `impl` declaration and definitions
with the `impls` operator used in `where` clauses, an issue brought up
in
  - #2495 
  - #2483

---------

Co-authored-by: Richard Smith <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal rfc Proposal with request-for-comment sent out proposal A proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants