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

Operator tokens #601

Merged
merged 10 commits into from
Jul 8, 2021
Merged

Operator tokens #601

merged 10 commits into from
Jul 8, 2021

Conversation

zygoloid
Copy link
Contributor

@zygoloid zygoloid commented Jun 25, 2021

Lexical rules for operator tokens.

@zygoloid zygoloid added the proposal A proposal label Jun 25, 2021
@zygoloid zygoloid requested a review from a team June 25, 2021 22:06
@google-cla google-cla bot added the cla: yes PR meets CLA requirements according to bot. label Jun 25, 2021
@zygoloid zygoloid marked this pull request as ready for review June 28, 2021 23:25
@github-actions github-actions bot added the proposal rfc Proposal with request-for-comment sent out label Jun 28, 2021
Copy link
Contributor

@jonmeow jonmeow left a comment

Choose a reason for hiding this comment

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

Generally looks good, but there are a few points I'm asking to clarify.

proposals/p0601.md Outdated Show resolved Hide resolved
proposals/p0601.md Show resolved Hide resolved
proposals/p0601.md Outdated Show resolved Hide resolved
proposals/p0601.md Outdated Show resolved Hide resolved
proposals/p0601.md Show resolved Hide resolved
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.

I know there was a bunch of discussion in the #syntax Discord channel as well here, but a few orthogonal comments in the interim...

Also, to be clear, I quite like the high level direction. I think it is a nicely balanced set of trade-offs. Now we just need the details to be sorted out....

proposals/p0601.md Show resolved Hide resolved
proposals/p0601.md Outdated Show resolved Hide resolved
Copy link
Contributor

@jonmeow jonmeow left a comment

Choose a reason for hiding this comment

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

At this point I think most of my concern is about whether we can make {/} handling match for binary operators without spaces, as noted on #syntax (a preference for commutative behavior where it's equally likely to get in the way of developers).

proposals/p0601.md Outdated Show resolved Hide resolved
proposals/p0601.md Show resolved Hide resolved
proposals/p0601.md Show resolved Hide resolved
proposals/p0601.md Show resolved Hide resolved
proposals/p0601.md Outdated Show resolved Hide resolved
Allow unspaced binary operators to be followed by `{` or `[` for
symmetry with allowing them to be preceded by `}` or `]` and in
anticipation of struct literals that begin with `{`.
@jonmeow
Copy link
Contributor

jonmeow commented Jul 1, 2021

Looks good, thank you. 👍

proposals/p0601.md Outdated Show resolved Hide resolved
proposals/p0601.md Show resolved Hide resolved
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.

I don't think there are any remaining blocking issues here and I'm happy with the wording at this point. I've made a suggestion for potentially adding a bit more of the motivation, but I don't think that should block anything so approving. Submit away!

@chandlerc
Copy link
Contributor

(I also resolved a couple of comment threads that seemed fully addressed. The only remaining threads are mine and as stated, I'm happy with @zygoloid resoling them and landing.)

@github-actions github-actions bot added proposal accepted Decision made, proposal accepted and removed proposal rfc Proposal with request-for-comment sent out labels Jul 3, 2021
@zygoloid zygoloid merged commit 814ba2d into carbon-language:trunk Jul 8, 2021
@zygoloid zygoloid deleted the proposal-operator-tokens branch March 11, 2022 01:01
chandlerc added a commit that referenced this pull request Jun 28, 2022
Proposal: lexical rules for operator tokens.

Co-authored-by: Chandler Carruth <[email protected]>
@zygoloid zygoloid mentioned this pull request Jan 6, 2023
josh11b pushed a commit that referenced this pull request Jun 2, 2023
The [Operators proposal #601](#601) got accepted and but the details were not updated in the design docs. Added `symbolic_tokens.md` file to add the details of the proposal and its discussion.

Closes #1992 

Co-authored-by: Avi Aaron <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes PR meets CLA requirements according to bot. proposal accepted Decision made, proposal accepted proposal A proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants