-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
const blocks as a mod item
#149174
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
base: main
Are you sure you want to change the base?
const blocks as a mod item
#149174
Conversation
7ec6ad3 to
0b65188
Compare
This comment has been minimized.
This comment has been minimized.
0b65188 to
cd57dbd
Compare
This comment has been minimized.
This comment has been minimized.
cd57dbd to
2133ba3
Compare
This comment has been minimized.
This comment has been minimized.
2133ba3 to
c9880cd
Compare
This comment has been minimized.
This comment has been minimized.
c9880cd to
cb1f138
Compare
This comment has been minimized.
This comment has been minimized.
cb1f138 to
ccad7e5
Compare
|
Some changes occurred in src/tools/rustfmt cc @rust-lang/rustfmt |
|
rustbot has assigned @jdonszelmann. Use |
ccad7e5 to
c62fe64
Compare
|
then I think it might be better not to. |
|
the const block will get its own defid, so with it we effectively allocate two |
|
@rustbot author |
That's not much of an improvement over
|
16b9a8a to
0654a7a
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
0654a7a to
fb3cacc
Compare
|
Changed the expansion to @rustbot ready |
| pub enum AllowConstBlockItems { | ||
| Yes, | ||
| No, | ||
| DoesNotMatter, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you just document what does not matter means? e.g. "in the parsing context it doesn't matter so we default to no"
|
r=me after that |
|
@rustbot author |
| shape | ||
| }; | ||
| let rhs = rewrite_assign_rhs_expr(context, &lhs, ex, shape, rhs_kind, rhs_tactics)?; | ||
| let rhs: String = rewrite_assign_rhs_expr(context, &lhs, ex, shape, rhs_kind, rhs_tactics)?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was this change necessary? If not please revert it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert all of these changes, they don't look like they're necessary for const block formatting. It's best not to make refactors like this to subtrees that are maintained outside of r-l/rust
| let context = &self.get_context(); | ||
| let offset = self.block_indent; | ||
| self.push_rewrite( | ||
| item.span, | ||
| block | ||
| .rewrite( | ||
| context, | ||
| Shape::legacy( | ||
| context.budget(offset.block_indent), | ||
| offset.block_only(), | ||
| ), | ||
| ) | ||
| .map(|rhs| { | ||
| recover_comment_removed(format!("const {rhs}"), span, context) | ||
| }), | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you're going to need to call rewrite_block() directly here instead of block.rewrite() so that you can pass along attributes from the Item. I wan to avoid inner attributes getting removed like they were in rust-lang/rustfmt#6158.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a test case similar to the one in rust-lang/rustfmt#6158 that has inner attributes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to make some rustfmt changes
Tracking issue: #149226
This adds support for writing
const { ... }as an item in a module. In the current implementation, this is a unique AST item that gets lowered toconst _: () = const { ... };in HIR.rustfmt support included.
TODO:
pub const { ... }does not make sense (see Add warn-by-default lint for visibility onconst _declarations #147136). Reject it. Should this be rejected by the parser or smth?_ident).