Skip to content

checker: disallow constant modification on unsafe {} - #23588

Merged
medvednikov merged 1 commit into
vlang:masterfrom
felipensp:disallow_constant_modify
Jan 28, 2025
Merged

medvednikov merged 1 commit into
vlang:masterfrom
felipensp:disallow_constant_modify

Conversation

@felipensp

Copy link
Copy Markdown
Member

Fix #23582

@huly-for-github

Copy link
Copy Markdown

Connected to Huly®: V_0.6-22014

@JalonSolov

Copy link
Copy Markdown
Collaborator

Really? unsafe is supposed to be an escape hatch to do whatever you need to do. As I said in the issue, if you use it unwisely, that is your own fault, not V's.

@felipensp

felipensp commented Jan 26, 2025 •

Copy link
Copy Markdown
Member Author

Really? unsafe is supposed to be an escape hatch to do whatever you need to do. As I said in the issue, if you use it unwisely, that is your own fault, not V's.

So call it const is pretty inconsistent. It should be mut.

@JalonSolov

Copy link
Copy Markdown
Collaborator

No, it is completely const unless you "color outside the lines" by using unsafe.

Using unsafe is telling the compiler "Yes, I know this is bad, but it is something I really need to do, so stop complaining about it."

@JalonSolov

Copy link
Copy Markdown
Collaborator

@medvednikov What do you think?

@felipensp
felipensp marked this pull request as ready for review January 27, 2025 16:47
@medvednikov
medvednikov merged commit 15e3f62 into vlang:master Jan 28, 2025
@medvednikov

Copy link
Copy Markdown
Member

I agree with the PR. It's too much power, even for unsafe.

@medvednikov

Copy link
Copy Markdown
Member

Can be changed to a __global if it's absolutely necessary.

@jorgeluismireles

Copy link
Copy Markdown
Contributor

Can be changed to a __global if it's absolutely necessary.

Or building with preference -translated as I see in the commit.

@felipensp
felipensp deleted the disallow_constant_modify branch January 29, 2025 15:13
@felipensp

Copy link
Copy Markdown
Member Author

Can be changed to a __global if it's absolutely necessary.

Or building with preference -translated as I see in the commit.

Yes. -translated builds are not affected. 👍🏻

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.

unsafe can modify pub const strings and arrays in other modules: is not good.

4 participants