Skip to content

Conversation

@casperisfine
Copy link
Contributor

[Feature #20350]

STR_CHILLED now spans on two user flags. If one bit is set it marks a chilled string literal, if it's the other it marks a Symbol#to_s chilled string.

Since it's not possible, and doesn't make much sense to include debug info when --debug-frozen-string-literal is set, we can't include allocation source, but we can safely include the symbol name in the warning message, making it much easier to find the source of the issue.

cc @jhawthorn @eregon

@peterzhu2118 do you know if I should bump the ABI version given some flags were moved around?

@matzbot matzbot requested a review from a team November 12, 2024 10:45
@launchable-app

This comment has been minimized.

@eregon
Copy link
Member

eregon commented Nov 12, 2024

@peterzhu2118 do you know if I should bump the ABI version given some flags were moved around?

Yes, changing the values/definitions of macros is definitely ABI-incompatible, so it should be bumped.

@casperisfine
Copy link
Contributor Author

@eregon would you happen to remember where it's set.

@eregon
Copy link
Member

eregon commented Nov 12, 2024

Copy link
Member

@eregon eregon left a comment

Choose a reason for hiding this comment

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

LGTM

This makes space in RString for two bits for chilled strings.
[Feature #20350]

`STR_CHILLED` now spans on two user flags. If one bit is set it
marks a chilled string literal, if it's the other it marks a
`Symbol#to_s` chilled string.

Since it's not possible, and doesn't make much sense to include
debug info when `--debug-frozen-string-literal` is set, we can't
include allocation source, but we can safely include the symbol
name in the warning message, making it much easier to find the source
of the issue.

Co-Authored-By: Étienne Barrié <[email protected]>
@maximecb maximecb merged commit 6deeec5 into ruby:master Nov 13, 2024
73 checks passed
@maximecb maximecb deleted the shared-to-fl-user-0 branch November 13, 2024 14:20
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.

5 participants