-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Change symbol mangling on Windows to avoid conflicts #5518
Conversation
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 it might be better to use String.build here, because otherwise this will allocate a lot of memory in the codegen phase. Previously it didn't because there was no memory allocation in the block.
I made a comment, but feel free to merge this. We can improve the performance later, specially because now there's no windows support at all. |
3c845b4
to
0c89dbe
Compare
Changed the fix according to the suggestion to use |
I guess that could happen... ¯_(ツ)_/¯ @Vexatos I meant using String.build do |io|
name.each_char do |char|
case char
...
end
end
end |
Ah, yeah, sorry, that would probably be a lot faster than gsub. |
0c89dbe
to
0f2436e
Compare
0f2436e
to
7737f70
Compare
Also not mangling |
Do we need to mangle specifically for Windows anymore? |
I just tried without the special mangling for Windows and the object file ends up being invalid (when executing the |
Can we get this cherry-picked for 0.24.2? Would be a great help for the ongoing windows efforts. @bcardiff |
Previously, certain symbols produced conflicts on windows, since
**
was mangled into..
which was the same as what+
was mangled into, producing undefined behaviour (In this case, it seems that+
was being executed instead of**
). This change mangles non-alphanumerical characters into codes with their (upper-case) hex values (e.g.<
is mangled into.3C.
), which will avoid conflicts and, in case of issues, still show which symbol used to be there, at the cost of being rather verbose.