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

version: detects clang as MSVC and GCC #1446

Conversation

gotschmarcel-ni
Copy link
Contributor

@gotschmarcel-ni gotschmarcel-ni commented Jan 25, 2023

sol uses __GNUC__ to detect GCC and _MSC_VER to detect MSVC. The problem is that clang defines both of them.

sol uses __GNUC__ to detect GCC and _MSC_VER to detect MSVC. The problem
is that clang defines both of them.
@ThePhD
Copy link
Owner

ThePhD commented Jul 18, 2023

This is fine. We want this to happen: you can know if it's a mixed-clang by checking SOL_IS_ON(SOL_COMPILER_VCXX) with SOL_IS_ON(SOL_COMPILER_CLANG), same with GCC.

Albeit, we should probably improve the GCC detection to be a lot more specific to GCC so it doesn't fire on Clang.

@ThePhD ThePhD self-assigned this Jul 18, 2023
@ThePhD ThePhD added Maybe.Bug...? Bug.Well Shit Here we go again... labels Jul 18, 2023
@ThePhD ThePhD added this to the Bugs milestone Jul 18, 2023
@ThePhD
Copy link
Owner

ThePhD commented Jul 18, 2023

Ah, no. It's fine as is. If there's any place where we should specifically be checking for Clang first before GCC, that might help push me in one direction or another.

@ThePhD ThePhD closed this Jul 18, 2023
@gotschmarcel-ni
Copy link
Contributor Author

I don't remember the exact error right now, but for us this was causing compile errors.

@gotschmarcel-ni
Copy link
Contributor Author

Just compiled again w/o our local patch and we get the following error. Note besides, we use -Werror

sol-v3.3.0/include/sol/stack_check_get_qualified.hpp:35:32: error: unknown warning group '-Wmaybe-uninitialized', ignored [-Werror,-Wunknown-warning-option]
#pragma GCC diagnostic ignored "-Wmaybe-uninitialized"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants