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

Semicolons terminate statements #2665

Merged
merged 15 commits into from
Mar 16, 2023

Conversation

jonmeow
Copy link
Contributor

@jonmeow jonmeow commented Mar 9, 2023

Statements, declarations, and definitions will terminate with either a semicolon
(;) or a close curly brace (}). Semicolons are never optional.

For example, with a semicolon, x = x + 2;. With a close curly brace,
for ( ... ) { ... }, or class C { ...}.

This does not affect any approved proposal; rather, it makes an important
assumption explicit.

Based on lead decision #1924

Fixes #2002

@jonmeow jonmeow added proposal A proposal proposal draft Proposal in draft, not ready for review labels Mar 9, 2023
@jonmeow jonmeow changed the title Semicolons Semicolons terminate statements Mar 9, 2023
@jonmeow jonmeow marked this pull request as ready for review March 9, 2023 19:30
@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 Mar 9, 2023
@github-actions github-actions bot requested a review from zygoloid March 9, 2023 19:30
@zygoloid
Copy link
Contributor

zygoloid commented Mar 9, 2023

I think the rule as written is too strong, and that the rule we want is that statements are terminated by either a semicolon or a close brace. Specifically, I don't think we want to require semicolons after for, if, or match statements (which are all terminated by close braces), nor after declaration statements that define classes and similar with a braced body (which you already point out as an exception).

I wonder if it's also worth adding something along these lines to principles/, about only using tokens and not whitespace to determine how to parse, but probably not as part of this proposal -- keeping this narrow and focused seems like the best way to make progress.

@jonmeow
Copy link
Contributor Author

jonmeow commented Mar 9, 2023

I think the rule as written is too strong, and that the rule we want is that statements are terminated by either a semicolon or a close brace. Specifically, I don't think we want to require semicolons after for, if, or match statements (which are all terminated by close braces), nor after declaration statements that define classes and similar with a braced body (which you already point out as an exception).

Adjusted phrasing. Note now I've trimmed the abstract slightly, and shifted more to the "proposal" section (still thin, because I don't think there's that much to say).

I wonder if it's also worth adding something along these lines to principles/, about only using tokens and not whitespace to determine how to parse, but probably not as part of this proposal -- keeping this narrow and focused seems like the best way to make progress.

I think https://github.com/carbon-language/carbon-lang/blob/trunk/docs/project/principles/low_context_sensitivity.md may already address this sufficiently, or put differently, maybe we could adjust that principle rather than adding another if you want it more explicitly addressed. The examples in the principle are more syntactic, but "visual aids" still has a section there and there's the up-front comment "It can be subtle: the contextual clues might be easily missed or mistaken." which could be inferred to apply to whitespace.

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.

Generally looks good to me.

I think it'd be nice to have a slightly stronger case made for the various "optional semicolons" rules having problems and surprises, but non-essential. Comments here are generally suggesting things you could say to make that case more strongly, none seems essential to address.

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

Thanks, this looks good to me, asking other 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.

LG, thanks for writing it all up!

@jonmeow jonmeow merged commit 9a063cc into carbon-language:trunk Mar 16, 2023
@jonmeow jonmeow deleted the proposal-semicolons branch March 16, 2023 21:17
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.

Make proposal for semicolons
3 participants