ci: windows 2017 C++17 testing

[henryiii]

Pulling out the non-annotations changes from #2427

[bstaletic]

Why was this removed?

[henryiii]

This was just to have a macOS FindPython run, but 3.9dev can also do FindPython without making another macOS run.

[henryiii]

Don't seem to be able to inject a single flag (or flags properly) to MSVC's build...

[YannickJadoul]

Must be something with GHA, though. I just tried, and I can perfectly run cmake .. -DPYTHON_EXECUTABLE=C:\Python38\python.exe -DCMAKE_CXX_FLAGS=/permissive- :-/

Something with thing being escaped?

[henryiii]

It is definitely messing with escaping.

[henryiii]

@YannickJadoul Any idea why I'm getting C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\VC\Tools\MSVC\14.16.27023\include\xlocale(319): error C2220: warning treated as error - no 'object' file generated ?

[YannickJadoul]

@YannickJadoul Any idea why I'm getting C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\VC\Tools\MSVC\14.16.27023\include\xlocale(319): error C2220: warning treated as error - no 'object' file generated ?

No idea; didn't think we were elevating warnings to errors?
The actual error seems to be the line before, though: warning C4530: C++ exception handler used, but unwind semantics are not enabled. Specify /EHsc

But I'm a bit confused on whether and why VS 2017 would have different defaults to VS 2019.

[bstaletic]

So, by default, exception handling is utterly borked on MSVC.

Default exception handling behavior

The compiler always generates code that supports asynchronous structured exception handling (SEH). By default (that is, if no /EHsc, /EHs, or /EHa option is specified), the compiler supports SEH handlers in the native C++ catch(...) clause. However, it also generates code that only partially supports C++ exceptions. The default exception unwinding code doesn't destroy automatic C++ objects outside of try blocks that go out of scope because of an exception. Resource leaks and undefined behavior may result when a C++ exception is thrown.

https://docs.microsoft.com/en-us/cpp/build/reference/eh-exception-handling-model?view=vs-2019

/EHsc means "please follow the standard".

[YannickJadoul]

So, by default, exception handling is utterly borked on MSVC.

...

/EHsc means "please follow the standard".

Fair enough. But why do the VS 2019 not have this problem?

Oh, maybe it's related to us asking VS 2017 "please follow the standard" with /permissive-?

[bstaletic]

I guess that's a possible explanation.

[bstaletic]

By the way, why don't we merge #2431 into this PR?

[YannickJadoul]

By the way, why don't we merge #2431 into this PR?

Happy with that. That's why I requested a review from @henryiii there :-) If this is necessary, maybe we should also add /EHsc to the flags?

[henryiii]

Let's see what we need to add, then we can combine the branches

[YannickJadoul]

OK! Feel free to steal or redirect my PR in whichever way works best for you :-)

[henryiii]

Found it, setting the flags manually here overrides the default:
https://gitlab.kitware.com/cmake/cmake/blob/v3.18.2/Modules/Platform/Windows-MSVC.cmake#L229

So it loses /EHsc if you add any flags.

[henryiii]

This would help the situation if added in the future: https://gitlab.kitware.com/cmake/cmake/-/issues/20610

[henryiii]

I've added the commit to #2431 and will go from there.