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

CMake compiler flag conditional inverted #84

Closed
SpartanJ opened this issue May 10, 2015 · 2 comments
Closed

CMake compiler flag conditional inverted #84

SpartanJ opened this issue May 10, 2015 · 2 comments
Labels
bug Something isn't working major

Comments

@SpartanJ
Copy link
Owner

Original report by Jacob Howard (Bitbucket: havoc-io, GitHub: havoc-io).


It appears that the compiler flag conditional in CMakeLists.txt is inverted. It currently reads

if (MSVC)
    add_definitions(-Wall)
else ()
    add_definitions(-D_SCL_SECURE_NO_WARNINGS)
endif()

It should actually be the other way around (the -D_SCL_SECURE_NO_WARNINGS flag only makes sense for MSVC)

if (MSVC)
    add_definitions(-D_SCL_SECURE_NO_WARNINGS) 
else ()
    add_definitions(-Wall)
endif()

In fact, it should actually read

if (MSVC)
    add_definitions(-D_SCL_SECURE_NO_WARNINGS) 
else ()
    add_definitions(-Wall -Wno-long-long)
endif()

It looks like what happened is that the conditional was originally in the incorrect order (someone tried to mimic the order of the premake file where the condition is equivalent to NOT MSVC). That didn't work on Windows, so in commit c6cde00 the -Wno-long-long flag was removed to make it barely work. MSVC does understand the -Wall flag, but on Windows this causes all warnings to become active, unlike in GCC/Clang where only a sensible subset become active, so the code builds on Windows, but it spits out a very large number of warnings.

Anyway, all that needs to happen is that the flags should be swapped and the -Wno-long-long flag should probably be added back in to keep consistency with premake.

I apologize for not issuing this as a pull request, but I'm really unfamiliar with Mercurial.

@SpartanJ
Copy link
Owner Author

Original comment by Martín Lucas Golini (Bitbucket: SpartanJ, GitHub: SpartanJ).


Fixed issue #44.

@SpartanJ
Copy link
Owner Author

Original comment by Martín Lucas Golini (Bitbucket: SpartanJ, GitHub: SpartanJ).


Thanks for reporting it!

@SpartanJ SpartanJ added major bug Something isn't working labels Jan 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working major
Projects
None yet
Development

No branches or pull requests

1 participant