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

Replace MSVC with MinGW GCC #1754

Closed
wants to merge 1 commit into from
Closed

Replace MSVC with MinGW GCC #1754

wants to merge 1 commit into from

Conversation

Xeeynamo
Copy link
Owner

@Xeeynamo Xeeynamo commented Oct 6, 2024

This is mainly done to put an end on the C2133 error and other oddities caused by the Microsoft Visual C++ compiler. It will be much easier to just rely on what GCC, Clang and MW C accepts as right instead of adding a fourth element into the mix that caused us more problems than what was worth.

This is achieved by hinting cmake to use GCC with DCMAKE_C_COMPILER=gcc. The CI agents from GitHub comes with MinGW pre-installed, so let's take advantage of that.

This unlocks #1743, #1701 and any issue we were bound to encounter later on.

@Xeeynamo Xeeynamo force-pushed the win-lose branch 12 times, most recently from 792ef32 to 2de4f09 Compare October 6, 2024 13:51
@Xeeynamo Xeeynamo changed the title Get rid of the MSVC compiler Replace MSVC with MinGW GCC Oct 6, 2024
@Xeeynamo Xeeynamo marked this pull request as ready for review October 6, 2024 14:04
@Xeeynamo Xeeynamo mentioned this pull request Oct 6, 2024
Copy link
Collaborator

@sozud sozud left a comment

Choose a reason for hiding this comment

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

I'm not going to block this if the majority of contributors want this but my opinion is we should keep MSVC:

It's the native compiler on the platform the majority of people use.

It has a visual debugger that as far as I know doesn't really have an equivalent anywhere else.

What it accepts is pretty different from gcc and clang and not testing against it means more and more incompatibilities are going to build up over time in the codebase.

long doesn't seem to have a different size size on gcc/windows, so we are going to get more portability issues https://stackoverflow.com/questions/384502/what-is-the-bit-size-of-long-on-64-bit-windows

I don't remember the specific details but certain kinds of programming errors get detected at runtime by MSVC that are different from sanitize-address etc.

@Xeeynamo
Copy link
Owner Author

Xeeynamo commented Oct 6, 2024

It's the native compiler on the platform the majority of people use.

My counter-argument on this is that it is a problem exclusive to the Visual Studio IDE and the tooling that comes with it. This already drag us down from great initiatives such as #995 . I do not want MSVC alone to stall the innovation of this project.

It has a visual debugger that as far as I know doesn't really have an equivalent anywhere else.

Any GDB front-end will work. It is also possible to generate a PDB file and still use the Visual Studio debugger. If I recall correctly Visual Studio also supports g++ as a C compiler and non-SLN projects via CMake, but I did not test that.

What it accepts is pretty different from gcc and clang and not testing against it means more and more incompatibilities are going to build up over time in the codebase.

It would drive us further and further from MSVC, that is true. But we can still generate a Windows build via MinGW. And we can return back to MSVC if we find another solution, and if we find value on that.

long doesn't seem to have a different size size on gcc/windows, so we are going to get more portability issues https://stackoverflow.com/questions/384502/what-is-the-bit-size-of-long-on-64-bit-windows

This problem is unrelated to this PR though. We've been using the 32-bit u_long on 64-bit builds since inception.

@Xeeynamo
Copy link
Owner Author

Xeeynamo commented Oct 9, 2024

Closing this. If we decide it is worth stripping out MSVC support we can re-open it.

@Xeeynamo Xeeynamo closed this Oct 9, 2024
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.

2 participants