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

format safety keywords on static items #6204

Merged
merged 1 commit into from
Jun 21, 2024

Conversation

ytmimi
Copy link
Contributor

@ytmimi ytmimi commented Jun 21, 2024

This includes both ast::StaticItem and ast::StaticForeignItem. safety was added to both ast::StaticItem and ast::SaticForeignItem in rust-lang/rust#124482.

This includes both `ast::StaticItem` and `ast::StaticForeignItem`.
`safety` was added to both `ast::StaticItem` and `ast::SaticForeignItem`
in rust-lang/rust#124482.

#![feature(unsafe_extern_blocks)]

safe static TEST1: i32;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure that formatting the safety here is necessary per this comment https://github.com/rust-lang/rust/pull/124482/files#r1608953719. For now it parses, so I don't think there's an issue, though I'm happy to revert the changes to StaticParts if we don't think this is necessary.

Copy link
Member

Choose a reason for hiding this comment

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

Let's keep it, since it doesn't hurt.


#![feature(unsafe_extern_blocks)]

safe static TEST1: i32;
Copy link
Member

Choose a reason for hiding this comment

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

Let's keep it, since it doesn't hurt.

@calebcartwright calebcartwright merged commit 30cdc2b into rust-lang:master Jun 21, 2024
26 checks passed
@calebcartwright
Copy link
Member

probably warrants an addition in the release notes too

@ytmimi
Copy link
Contributor Author

ytmimi commented Jun 21, 2024

@calebcartwright I'll update the CHANGELOG PR #6004 in just a bit to include this

Edit: The CHANGELOG PR was updated!

@@ -1989,11 +1994,13 @@ fn rewrite_static(
static_parts: &StaticParts<'_>,
offset: Indent,
) -> Option<String> {
println!("rewriting static");

Choose a reason for hiding this comment

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

You left in a debug print, it seems.

Copy link
Member

Choose a reason for hiding this comment

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

I am surprised that this did not make any of the tests fail...?

Copy link
Member

Choose a reason for hiding this comment

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

I am surprised that this did not make any of the tests fail...?

same (#6210 (comment))

we'll do a postmortem of sorts to review and hopefully identify some preventative measures to prevent a recurrence

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants