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

fix errors from MSVC '/permissive-' mode (fixes #6230) #6232

Conversation

Zhaojun-Liu
Copy link
Contributor

@Zhaojun-Liu Zhaojun-Liu commented Dec 12, 2023

Fixes #6230

Add stdexcept header file to fix error C2039: 'invalid_argument': is not a member of 'std' under option /permissive-.

Add stdexcept include to fix error C2039: 'invalid_argument': is not a member of 'std'.
@jameslamb jameslamb added the fix label Dec 12, 2023
@jameslamb jameslamb changed the title Add stdexcept header file fix errors from MSVC '/permissive-' mode (fixes #6230) Dec 12, 2023
Copy link
Collaborator

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

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

Thanks very much for the fix!

I think we should start passing /permissive- when running tests with MSVC here, to catch more such things earlier. Are you interested in a follow-up pull request to add testing like that? I'd be happy to help with the contribution process.

@Zhaojun-Liu
Copy link
Contributor Author

Zhaojun-Liu commented Dec 13, 2023

Thanks very much for the fix!

I think we should start passing /permissive- when running tests with MSVC here, to catch more such things earlier. Are you interested in a follow-up pull request to add testing like that? I'd be happy to help with the contribution process.

I'd love to, so how should I do next? @jameslamb

@jameslamb
Copy link
Collaborator

I'd love to

Thanks!

Let's try using the Windows cpp_tests CI job that runs on Azure DevOps for this purpose.

Here's an example build log where you can see it's using MSVC from Visual Studio 16 2019:

https://dev.azure.com/lightgbm-ci/lightgbm-ci/_build/results?buildId=15541&view=logs&j=1f4df553-f999-5fff-c6fe-71123c872ae2&t=5236b51f-5710-5c9e-d20e-7548daf49042

Somewhere in this block, add /permissive- to the the compiler flags:

LightGBM/CMakeLists.txt

Lines 643 to 656 in 522f0f0

if(MSVC)
set(
CompilerFlags
CMAKE_CXX_FLAGS
CMAKE_CXX_FLAGS_DEBUG
CMAKE_CXX_FLAGS_RELEASE
CMAKE_C_FLAGS
CMAKE_C_FLAGS_DEBUG
CMAKE_C_FLAGS_RELEASE
)
foreach(CompilerFlag ${CompilerFlags})
string(REPLACE "/MD" "/MT" ${CompilerFlag} "${${CompilerFlag}}")
endforeach()
endif()

Open a draft pull request so we can see what happens in CI, and we can help from there.

@jameslamb jameslamb merged commit 6fc8052 into microsoft:master Dec 13, 2023
41 checks passed
@Zhaojun-Liu
Copy link
Contributor Author

Zhaojun-Liu commented Dec 13, 2023

@jameslamb I submitted a draft pull request, I'm not sure if it's correct, you can check #6234.

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