Skip to content

Switch the Compiler to Clang (with concessions)#23

Closed
fairangelica wants to merge 1 commit intosaturn-xvi:mainfrom
fairangelica:main
Closed

Switch the Compiler to Clang (with concessions)#23
fairangelica wants to merge 1 commit intosaturn-xvi:mainfrom
fairangelica:main

Conversation

@fairangelica
Copy link
Copy Markdown
Collaborator

Heyhey, as per the comment:

Kind Reagards,
Someone who has had enough of this dumpster fire of a compiler and will switch to NixOS and clang.
Update: after one month of pain while trying to switch i will do that laterTM

I tried to switch the compiler from MSVC to Clang, which runs fine but could have some issues that I didn't check/ might have missed. I tested it on VS2022 with C++ Clang Tools for Windows, build option was "Release x64".
Currently, the patched GameClientApp complains with ".text decryption failed" but after hitting Enter a few times it seems to run without any more complains that I encountered.
There are some concessions I had to make to the codebase that I wanted to highlight here:

  • I had to switch the C++ standard from C++20 to C++17, as Visual Studio defaults to compile options that are unknown to 'clang-cl', making it impossible to make the build work (If someone else can find out how to do that it would be nice). That means that I had to configure vcpkg to install fmt and switched all the "std:format" statements to "fmt:format", and had to swap "std::erase" to "std::remove" and "std::remove_if".
  • It would be good to double-check 'window_manager.h' & 'viewmodel_write_blocker.h' as I trial & error removed the ## until Clang was satisfied.

Hope it helps for the journey to the fabled NixOS!

@ThisKwasior
Copy link
Copy Markdown

I've also had issues with preprocessor string concatenation when I tried building the project manually with MinGW/GCC, so just removing it will probably break the game. Although I also had issues with pe.h for some reason and a lot of msvc integer literals.

@fairangelica
Copy link
Copy Markdown
Collaborator Author

I've also had issues with preprocessor string concatenation when I tried building the project manually with MinGW/GCC, so just removing it will probably break the game.

Yep, Clang seemed way more finicky and strict with how it wants to parse them. I just hope that the lack of some of them doesn't make it stray from the intended behavior '^^

Although I also had issues with pe.h for some reason and a lot of msvc integer literals.

Didn't need to modify that actually, seems like VS includes some "magic" to make it work or something, I suspect the Windows SDK but unsure.

@eawd123wadd4354as243df
Copy link
Copy Markdown

Making the build work would probably be easiest using cmake instead of the VS system.
Check #22 (or #24 I guess).

The String Concatenation is correct. The ## joins two tokens, so adding it after a variable tries to make a single token from the variable and the token after. I have no idea why this is valid under MSVC, but the changes you make produce the correct (expected) result.

__builtin_return_address(0) is equivalent to _ReturnAddress() for our use case, as none of the points mentioned under GCC Return Address apply (clang feature works the same as gcc).

@fairangelica
Copy link
Copy Markdown
Collaborator Author

Making the build work would probably be easiest using cmake instead of the VS system. Check #22 (or #24 I guess).

Of course, but I was not knowledgeable enough to port to cmake at the time I worked on this and still have no clue, would be great to get #22 in tandem to this ^^
With MingW64 runtime support, I couldn't get around to it yet, hope this gives a good start with specifically MinGW-LLVM, chose Clang here as it was the easiest to switch to with the VS build system.

The String Concatenation is correct. The ## joins two tokens, so adding it after a variable tries to make a single token from the variable and the token after. I have no idea why this is valid under MSVC, but the changes you make produce the correct (expected) result.

Nice! Thanks for checking. Noticed that MSVC is stupidly permissive and "magically" fixes so many things that would error under Clang.

__builtin_return_address(0) is equivalent to _ReturnAddress() for our use case, as none of the points mentioned under GCC Return Address apply (clang feature works the same as gcc).

Nice ^^, again thanks for checking!

@Hazno-dev
Copy link
Copy Markdown
Member

Of course, but I was not knowledgeable enough to port to cmake at the time I worked on this and still have no clue, would be great to get #22 in tandem to this ^^ With MingW64 runtime support, I couldn't get around to it yet, hope this gives a good start with specifically MinGW-LLVM, chose Clang here as it was the easiest to switch to with the VS build system.

With the little CMake migration I did we should be able to support this and MSVC. I tried to isolate most of the MSVC pipeline into /Platforms/MSVC.cmake (tho there is some msbuild specific stuff for the patcher as its using .net).

@zkxjzmswkwl
Copy link
Copy Markdown

MSVC unions aren't standard. Furthermore, historically, you want to use the same compiler (or at the least, very similar) of the target you're mapping the PE file to.

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