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

style-guide: Document formatting of as casts (mostly like a binary operator) #114394

Merged
merged 1 commit into from
Sep 21, 2023

Conversation

joshtriplett
Copy link
Member

as casts currently get formatted like a binary operator, except that
the second line can stack several as casts rather than breaking them
each onto their own line. Document this.

As far as I can tell (cc @calebcartwright for verification), this is not a 2024 edition change, it just documents current behavior.

@joshtriplett joshtriplett added the T-style Relevant to the style team, which will review and decide on the PR/issue. label Aug 3, 2023
@rustbot
Copy link
Collaborator

rustbot commented Aug 3, 2023

r? @compiler-errors

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 3, 2023
@rustbot
Copy link
Collaborator

rustbot commented Aug 3, 2023

Some changes occurred in src/doc/style-guide

cc @rust-lang/style

@calebcartwright
Copy link
Member

I'll take a closer look at the proposed textual changes, but we do already have a dedicated section for casts (however light it may be) and I'd suggest we consolidate under a single one

https://github.com/rust-lang/rust/blob/46e156c34e3e59a9f7049375818ca1fd816e3ca3/src/doc/style-guide/src/expressions.md#casts-as

@joshtriplett
Copy link
Member Author

@calebcartwright I completely missed that! I'll consolidate them.

I don't think this is a change, just documenting existing behavior.

… operator)

`as` casts currently get formatted like a binary operator, except that
the second line can stack several `as` casts rather than breaking them
each onto their own line. Merge `as` into a subsection of the binary
operators section, and then go into detail on the one difference between
`as` formatting and binary operator formatting.
@calebcartwright
Copy link
Member

@ytmimi - on the off chance you have bandwidth this week to double check rustfmt's current behavior matches the more descriptive style guide text could you do so and confirm here? this one's been waiting on me for a while but I've got some other things above it on my todo list and probably won't be able to give it much attention this week.

My initial reaction/mental model aligns with Josh that this aligns with current behavior, but with the increased emphasis on formatting stability and avoiding style guide vs. rustfmt discrepancies I want to check some edge cases with deep nesting, comments, and probably embedded within some bin expressions with paren wrapping too, solely out of an abundance of caution

@ytmimi
Copy link
Contributor

ytmimi commented Aug 28, 2023

@calebcartwright This one got lost in my inbox, but I'll put some time aside this week to validate rustfmt's current behavior. I'm almost positive that rustfmt doesn't handle comments in casts very well (at least not until we land rust-lang/rustfmt#5296).

I also found this report rust-lang/rustfmt#3528, which suggests rustfmt isn't wrapping casts correctly.

I'll report back once I've spent some more time looking into those edge cases.

@calebcartwright
Copy link
Member

Thanks for looking into it @ytmimi! My inclination continues to be that this updated text to the style guide is correct insofar as describing the desired behavior in a non-breaking way, and the existing (potential) issues in rustfmt aren't affected.

@calebcartwright
Copy link
Member

@bors r+ rollup=always

@bors
Copy link
Contributor

bors commented Sep 20, 2023

📌 Commit 42fa7df has been approved by calebcartwright

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 20, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 20, 2023
…iaskrgr

Rollup of 4 pull requests

Successful merges:

 - rust-lang#114394 (style-guide: Document formatting of `as` casts (mostly like a binary operator))
 - rust-lang#115990 (Allow anyone to set llvm-fixed-upstream)
 - rust-lang#116008 (Rename BoxMeUp to PanicPayload.)
 - rust-lang#116011 (Update browser-ui-test version to 0.16.10)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 21, 2023
…iaskrgr

Rollup of 4 pull requests

Successful merges:

 - rust-lang#114394 (style-guide: Document formatting of `as` casts (mostly like a binary operator))
 - rust-lang#115990 (Allow anyone to set llvm-fixed-upstream)
 - rust-lang#116008 (Rename BoxMeUp to PanicPayload.)
 - rust-lang#116011 (Update browser-ui-test version to 0.16.10)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 47277ab into rust-lang:master Sep 21, 2023
11 checks passed
@rustbot rustbot added this to the 1.74.0 milestone Sep 21, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Sep 21, 2023
Rollup merge of rust-lang#114394 - joshtriplett:style-guide-as, r=calebcartwright

style-guide: Document formatting of `as` casts (mostly like a binary operator)

`as` casts currently get formatted like a binary operator, except that
the second line can stack several `as` casts rather than breaking them
each onto their own line. Document this.

As far as I can tell (cc `@calebcartwright` for verification), this is not a 2024 edition change, it just documents current behavior.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-style Relevant to the style team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants