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

Change operator precedence #4075

Merged
merged 9 commits into from
Aug 10, 2024
Merged

Change operator precedence #4075

merged 9 commits into from
Aug 10, 2024

Conversation

josh11b
Copy link
Contributor

@josh11b josh11b commented Jun 22, 2024

Update the operator precedence to achieve a few goals:

  • Form operators into groups which behave similarly
  • Make the group of operators ("top-level operators") that capture everything to the right, like if...then...else, behave similarly to the left, so that rearranging expressions won't change how they group.
  • Add the where operator, used to specify constraints on facet types, to the precedence chart, to define how it interacts with other operators.
  • Make the operator precedence diagram prettier, so that it eventually can be made into a poster that Carbon programmers can hang on their walls.

@josh11b josh11b added proposal A proposal proposal draft Proposal in draft, not ready for review labels Jun 22, 2024
@josh11b josh11b marked this pull request as ready for review June 24, 2024 22:37
@github-actions github-actions bot added proposal rfc Proposal with request-for-comment sent out and removed proposal draft Proposal in draft, not ready for review labels Jun 24, 2024
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.

- It is used in an `impl` declaration to specify the values of associated
constants (such as associated types). In that context, `impl T as I where R`
is interpreted conceptually as `impl T as (I where R)`. It would be nice if
`T as I where R` would mean the same thing in other contexts.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe worth noting here: if we don't get the "would be nice" version, we want it to be invalid rather than meaning (T as I) where R. That is, considered in isolation, we would prefer T as (I where R) > invalid > (T as I) where R.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

proposals/p4075.md Outdated Show resolved Hide resolved
`T as I where R` would mean the same thing in other contexts.
- The `where` operator will frequently be used with the binary `&` operator,
since that is how facet types are combined. It is desirable that
`I & J where R` be interpreted as `(I & J) where R`. This is expected to be
Copy link
Contributor

Choose a reason for hiding this comment

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

Likewise here: considered in isolation, we probably prefer (I & J) where R > invalid > I & (I where R).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

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.

Just wanted to say I'm generally happy with this. Leaving the detail review to @zygoloid. I'm also not concerned if we discover we want to make a further tweak here based on experience, this still seems like a good improvement and bringing the where clause into the picture.

Copy link
Contributor Author

@josh11b josh11b left a comment

Choose a reason for hiding this comment

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

josh11b and others added 3 commits August 9, 2024 15:59
@josh11b
Copy link
Contributor Author

josh11b commented Aug 9, 2024

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.

LGTM and approving for leads.

@chandlerc chandlerc added this pull request to the merge queue Aug 10, 2024
Merged via the queue into carbon-language:trunk with commit c5b5d36 Aug 10, 2024
7 checks passed
@josh11b josh11b deleted the prec branch August 19, 2024 22:51
josh11b added a commit to josh11b/carbon-lang that referenced this pull request Aug 21, 2024
github-merge-queue bot pushed a commit that referenced this pull request Aug 22, 2024
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
Co-authored-by: Geoff Romer <gromer@google.com>
dwblaikie pushed a commit to dwblaikie/carbon-lang that referenced this pull request Aug 23, 2024
)

Co-authored-by: Josh L <josh11b@users.noreply.github.com>
Co-authored-by: Geoff Romer <gromer@google.com>
josh11b added a commit to josh11b/carbon-lang that referenced this pull request Aug 23, 2024
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.

3 participants