Dropping MSVC 2015 #3722

rwgk · 2022-02-12T00:55:25Z

Description

This PR drops support for MSVC 2015, and systematically removes workarounds for MSVC 2015.

Some workarounds marked as needed for MSVC 2015 are actually also needed for some other compilers, even including an old version of clang, and the newest version of MSVC. The #ifdefs for these workarounds were made as specific as possible, based on CI failures.

The only remaining AppVeyor build was moved from MSVC 2015 to 2017.

Suggested changelog entry:

Support for MSVC 2015 was dropped. Workarounds for MSVC 2015 were systematically removed.

Upgrade guide:

The minimum version for MSVC is now 2017.

rwgk · 2022-02-12T01:25:20Z

Reference point:

59 successful (we don't want that number to be lower for the rest of this PR)
3 failing (2 GHA, 1 appveyor)

henryiii · 2022-02-13T03:41:15Z

docs/upgrade.rst

-* MSVC >= 2015u3
+* MSVC >= 2017


This is under pybind 2.1 - this was still true in 2.1, we are not editing an old release ;). We should add it under 2.10 at the top if you want.

I have the same nit here.

Oops — thanks for catching this! I was too much in lawn-mower mode when going through here.

Reverted.

I added mention for the upgrade guide to the PR description.

henryiii · 2022-02-13T03:46:01Z

include/pybind11/detail/common.h

-#if !defined(_MSC_VER)
+#if defined(PYBIND11_CPP20) || !defined(_MSC_VER)                                                 \
+    || _MSC_VER > 1930 /* Increment as needed, let MS know please. */
+//     https://docs.microsoft.com/en-us/cpp/preprocessor/predefined-macros


A little confused, what's the current MSVC number, 1930 or 1931? This isn't something we have to increment every MSVC release, is it? If it doesn't work now, we should assume we don't know when it will start working. Once it does, then we can set this number. Only put real, known limits, not "I hope they fix this" numbers. I"m guessing it is fine in C++20 mode, though - we test on MSVC C++20?

Changed as suggested.

Current is 1930.

We have 3 jobs total with C++20:

Clang 10 x64

GCC 10 x64

MSVC 2019 x86

I think I overreacted. I was a pretty unhappy to discover that even MSVC 2019 and 2022 ICE without this workaround. Thinking: heck, if we didn't mask it here for any MSVC, it may have gotten fixed in all those years. But at second thought, my idea to force us to increment the version with every MSVC release — until there is finally a fix — will backfire badly, because it'll force people to update pybind11, or patch locally, to be able to use a new MSVC release.

Another idea is to gate on _MSC_VER >= 1940, with the idea to catch ICEs for the next major update of MSVC, but who knows, really, what the correct number will be? — Staying away from it.

Yes. Do we know it works on C++20? The old version simply always used the workaround, the new one seems to think MSVC will be happy with this on C++20 mode. Is it?

We don't know before or after my change. The old version was guilty (needs workaround) until proven innocent, which has the drawback of covering up flaws even for new compilers. The new is innocent until proven guilty. I'm trying it out now. My first attempt seems to only kind-of work:

Run cmake -S . -B . -DPYBIND11_WERROR=ON -DDOWNLOAD_CATCH=ON -DDOWNLOAD_EIGEN=ON -DCMAKE_CXX_STANDARD=11 -DCMAKE_CXX_STANDARD=20 -- Building for: Visual Studio 17 2022

If it doesn't get too messy we can fold it in here.

eigen is acting up:

test_eigen.cpp D:\a\pybind11\pybind11\_deps\eigen-src\Eigen\src/Core/Matrix.h([46](https://github.com/pybind/pybind11/runs/5179409281?check_suite_focus=true#step:11:46),40): error C2220: the following warning is treated as an error [D:\a\pybind11\pybind11\tests\pybind11_tests.vcxproj] D:\a\pybind11\pybind11\_deps\eigen-src\Eigen\src/Core/PlainObjectBase.h(98): message : see reference to class template instantiation 'Eigen::internal::traits<Derived>' being compiled [D:\a\pybind11\pybind11\tests\pybind11_tests.vcxproj] with [ Derived=Eigen::Matrix<double,-1,-1,0,-1,-1> ] D:\a\pybind11\pybind11\_deps\eigen-src\Eigen\src/Core/Matrix.h(180): message : see reference to class template instantiation 'Eigen::PlainObjectBase<Eigen::Matrix<double,-1,-1,0,-1,-1>>' being compiled [D:\a\pybind11\pybind11\tests\pybind11_tests.vcxproj] D:\a\pybind11\pybind11\tests\test_eigen.cpp(39): message : see reference to class template instantiation 'Eigen::Matrix<double,-1,-1,0,-1,-1>' being compiled [D:\a\pybind11\pybind11\tests\pybind11_tests.vcxproj] D:\a\pybind11\pybind11\_deps\eigen-src\Eigen\src/Core/Matrix.h(46,40): warning C[50](https://github.com/pybind/pybind11/runs/5179409281?check_suite_focus=true#step:11:50)54: operator '&': deprecated between enumerations of different types [D:\a\pybind11\pybind11\tests\pybind11_tests.vcxproj] D:\a\pybind11\pybind11\_deps\eigen-src\Eigen\src/Core/Matrix.h(46,40): message : to simplify migration, consider the temporary use of /Wv:18 flag with the version of the compiler with which you used to build without warnings [D:\a\pybind11\pybind11\tests\pybind11_tests.vcxproj] D:\a\pybind11\pybind11\_deps\eigen-src\Eigen\src/Core/TriangularMatrix.h(211,29): warning C5054: operator '&': deprecated between enumerations of different types [D:\a\pybind11\pybind11\tests\pybind11_tests.vcxproj] D:\a\pybind11\pybind11\_deps\eigen-src\Eigen\src/Core/TriangularMatrix.h(211,29): message : to simplify migration, consider the temporary use of /Wv:18 flag with the version of the compiler with which you used to build without warnings [D:\a\pybind11\pybind11\tests\pybind11_tests.vcxproj] D:\a\pybind11\pybind11\tests\test_eigen.cpp(110): message : see reference to class template instantiation 'Eigen::TriangularView<const MatrixType,1>' being compiled [D:\a\pybind11\pybind11\tests\pybind11_tests.vcxproj] with [ MatrixType=Eigen::Matrix<double,-1,-1,1,-1,-1> ] D:\a\pybind11\pybind11\_deps\eigen-src\Eigen\src/Core/TriangularMatrix.h(212,29): warning C5054: operator '&': deprecated between enumerations of different types [D:\a\pybind11\pybind11\tests\pybind11_tests.vcxproj] D:\a\pybind11\pybind11\_deps\eigen-src\Eigen\src/Core/TriangularMatrix.h(212,29): message : to simplify migration, consider the temporary use of /Wv:18 flag with the version of the compiler with which you used to build without warnings [D:\a\pybind11\pybind11\tests\pybind11_tests.vcxproj] D:\a\pybind11\pybind11\_deps\eigen-src\Eigen\src/Core/TriangularMatrix.h(213,29): warning C5054: operator '&': deprecated between enumerations of different types [D:\a\pybind11\pybind11\tests\pybind11_tests.vcxproj] D:\a\pybind11\pybind11\_deps\eigen-src\Eigen\src/Core/TriangularMatrix.h(213,29): message : to simplify migration, consider the temporary use of /Wv:18 flag with the version of the compiler with which you used to build without warnings [D:\a\pybind11\pybind11\tests\pybind11_tests.vcxproj] D:\a\pybind11\pybind11\_deps\eigen-src\Eigen\src/Core/TriangularMatrix.h(214,29): warning C5054: operator '&': deprecated between enumerations of different types [D:\a\pybind11\pybind11\tests\pybind11_tests.vcxproj] D:\a\pybind11\pybind11\_deps\eigen-src\Eigen\src/Core/TriangularMatrix.h(214,29): message : to simplify migration, consider the temporary use of /Wv:18 flag with the version of the compiler with which you used to build without warnings [D:\a\pybind11\pybind11\tests\pybind11_tests.vcxproj] D:\a\pybind11\pybind11\include\pybind11\cast.h(1014,1): error C27[52](https://github.com/pybind/pybind11/runs/5179409281?check_suite_focus=true#step:11:52): 'pybind11::detail::type_caster<Eigen::Ref<Eigen::Matrix<double,-1,-1,0,-1,-1>,0,Eigen::OuterStride<-1>>,void>': more than one partial specialization matches the template argument list [D:\a\pybind11\pybind11\tests\pybind11_tests.vcxproj]

The old version had a harmless workaround that might not be needed. The new version has a probability of breaking things that would have worked. We try to guess the safest path unless we test it to see it work. If we can test it and it works in C++20 mode, great! Otherwise err on the side of caution.

A nice developer way to disable workarounds might be nice, but all at once would be too much. Maybe a standard searchable comment?

Otherwise err on the side of caution.

Idk, I always cared about helping new compilers getting better. It diminishes the benefit of beta testing-compilers if we cover up anyway.

I'm adding a MSVC 2022 C++20 GHA under PR #3732. The same commit is included here (will rebase after #3732 is merged). It proves that this version works for MSVC 2022 C++20:

#if defined(PYBIND11_CPP20) || !defined(_MSC_VER) // Sadly, all MSVC versions incl. 2022 need this.

Note that the 1 CI failure here was a known flake.

include/pybind11/detail/common.h

henryiii

More workarounds removed than I thought, nice! Couple of minor things.

rwgk · 2022-02-14T00:03:35Z

@henryiii Could you please take another look?

…e pre-commit.

git checkout d078658 include/pybind11/attr.h include/pybind11/detail/common.h include/pybind11/functional.h -------------------- CI pybind#4160 errors observed: pybind@2a26873 https://github.com/pybind/pybind11/runs/5168332130?check_suite_focus=true $ grep ' error C' *.txt | sed 's/2022-02-12[^ ]*//' | sed 's/^[0-9][0-9]*//' | sed 's/^.*\.txt: //' | sort | uniqD:\a\pybind11\pybind11\include\pybind11\cast.h(1364,1): error C2752: 'pybind11::detail::type_caster<Eigen::Ref<Eigen::Vector3f,0,pybind11::EigenDStride>,void>': more than one partial specialization matches the template argument list [D:\a\pybind11\pybind11\build\tests\pybind11_tests.vcxproj] d:\a\pybind11\pybind11\include\pybind11\detail/common.h(1023): error C2737: 'pybind11::overload_cast': 'constexpr' object must be initialized [D:\a\pybind11\pybind11\build\tests\cross_module_gil_utils.vcxproj] d:\a\pybind11\pybind11\include\pybind11\detail/common.h(1023): error C2737: 'pybind11::overload_cast': 'constexpr' object must be initialized [D:\a\pybind11\pybind11\build\tests\pybind11_cross_module_tests.vcxproj] d:\a\pybind11\pybind11\include\pybind11\detail/common.h(1023): error C2737: 'pybind11::overload_cast': 'constexpr' object must be initialized [D:\a\pybind11\pybind11\build\tests\pybind11_tests.vcxproj] d:\a\pybind11\pybind11\include\pybind11\detail/common.h(1023): error C2737: 'pybind11::overload_cast': 'constexpr' object must be initialized [D:\a\pybind11\pybind11\build\tests\test_embed\external_module.vcxproj] D:\a\pybind11\pybind11\include\pybind11\detail/common.h(624): fatal error C1001: Internal compiler error. [D:\a\pybind11\pybind11\build\tests\pybind11_tests.vcxproj] D:\a\pybind11\pybind11\include\pybind11\detail/common.h(624): fatal error C1001: Internal compiler error. [D:\a\pybind11\pybind11\tests\pybind11_tests.vcxproj] $ grep ': error C2737' *.txt | sed 's/^.*MSVC//' | sed 's/___.*//' | sort | uniq _2017 $ grep ': error C2752' *.txt 3______3.8_____MSVC_2019_____x86_-DCMAKE_CXX_STANDARD=17.txt:2022-02-12T16:12:45.9921122Z D:\a\pybind11\pybind11\include\pybind11\cast.h(1364,1): error C2752: 'pybind11::detail::type_caster<Eigen::Ref<Eigen::Vector3f,0,pybind11::EigenDStride>,void>': more than one partial specialization matches the template argument list [D:\a\pybind11\pybind11\build\tests\pybind11_tests.vcxproj] $ grep ': fatal error C1001:' *.txt 10______pypy-3.8-v7.3.7_____windows-2022_____x64.txt:2022-02-12T16:12:56.3163683Z D:\a\pybind11\pybind11\include\pybind11\detail/common.h(624): fatal error C1001: Internal compiler error. [D:\a\pybind11\pybind11\tests\pybind11_tests.vcxproj] 1______3.6_____MSVC_2019_____x86.txt:2022-02-12T16:12:47.6774625Z D:\a\pybind11\pybind11\include\pybind11\detail/common.h(624): fatal error C1001: Internal compiler error. [D:\a\pybind11\pybind11\build\tests\pybind11_tests.vcxproj] 16______3.6_____windows-latest_____x64_-DPYBIND11_FINDPYTHON=ON.txt:2022-02-12T16:12:27.0556151Z D:\a\pybind11\pybind11\include\pybind11\detail/common.h(624): fatal error C1001: Internal compiler error. [D:\a\pybind11\pybind11\tests\pybind11_tests.vcxproj] 17______3.9_____windows-2019_____x64.txt:2022-02-12T16:12:30.3822566Z D:\a\pybind11\pybind11\include\pybind11\detail/common.h(624): fatal error C1001: Internal compiler error. [D:\a\pybind11\pybind11\tests\pybind11_tests.vcxproj] 2______3.7_____MSVC_2019_____x86.txt:2022-02-12T16:12:38.7018911Z D:\a\pybind11\pybind11\include\pybind11\detail/common.h(624): fatal error C1001: Internal compiler error. [D:\a\pybind11\pybind11\build\tests\pybind11_tests.vcxproj] 6______3.6_____windows-2022_____x64.txt:2022-02-12T16:12:00.4513642Z D:\a\pybind11\pybind11\include\pybind11\detail/common.h(624): fatal error C1001: Internal compiler error. [D:\a\pybind11\pybind11\tests\pybind11_tests.vcxproj] 7______3.9_____windows-2022_____x64.txt:2022-02-12T16:11:43.6306160Z D:\a\pybind11\pybind11\include\pybind11\detail/common.h(624): fatal error C1001: Internal compiler error. [D:\a\pybind11\pybind11\tests\pybind11_tests.vcxproj] 8______3.10_____windows-2022_____x64.txt:2022-02-12T16:11:49.9589644Z D:\a\pybind11\pybind11\include\pybind11\detail/common.h(624): fatal error C1001: Internal compiler error. [D:\a\pybind11\pybind11\tests\pybind11_tests.vcxproj] 9______pypy-3.7-v7.3.7_____windows-2022_____x64.txt:2022-02-12T16:11:53.7912112Z D:\a\pybind11\pybind11\include\pybind11\detail/common.h(624): fatal error C1001: Internal compiler error. [D:\a\pybind11\pybind11\tests\pybind11_tests.vcxproj]

rwgk · 2022-02-14T19:36:16Z

Merging this now, to start work on the smart_holder branch, catching up with the clang-format changes (PR #3713), the Python drops (PRs #3688 & #3719), and this PR.

rwgk marked this pull request as ready for review February 13, 2022 01:45

rwgk requested a review from henryiii as a code owner February 13, 2022 01:45

rwgk requested a review from Skylion007 February 13, 2022 01:46

Skylion007 approved these changes Feb 13, 2022

View reviewed changes

henryiii reviewed Feb 13, 2022

View reviewed changes

include/pybind11/detail/common.h Show resolved Hide resolved

henryiii reviewed Feb 13, 2022

View reviewed changes

rwgk mentioned this pull request Feb 14, 2022

Adding MSVC 2022 C++20 GitHub Action #3732

Merged

Ralf W. Grosse-Kunstleve added 17 commits February 14, 2022 09:29

Changing _MSC_VER guard to < 1910 (dropping MSVC 2015).

0321897

Removing MSVC 2015 from ci.yml, and .appveyor.yml entirely.

a0d14ea

Bringing back .appveyor.yml from master.

8c14771

appveyor Visual Studio 2017

8423316

1st manual pass, builds & tests with unix_clang, before pre-commit.

7a09e61

After clang-format (via pre-commit).

dcf2ded

Manual pass looking for "2015", builds & tests with unix_clang, befor…

637b27f

…e pre-commit.

common.h: is_template_base_of

8bd480a

Re-applying 4 changes from 2a26873 that work universally.

e8334e2

overload_cast = {}; only for MSVC 2017 and Clang 5

f6480bd

Refining condition for using is_template_base_of workaround.

baa474c

Undoing MSVC 2015 workaround in test_constants_and_functions.cpp

4858b88

CentOS7: silence_unused_warnings

f072f4a

Tweaks in response to reviews.

cff378c

Adding windows-2022 C++20

ceac490

Trying another way of adding windows-2022 C++20

cb0d114

rwgk force-pushed the msvc_2015_drop branch from 98a6a5c to cb0d114 Compare February 14, 2022 17:32

rwgk merged commit a97e9d8 into pybind:master Feb 14, 2022

rwgk deleted the msvc_2015_drop branch February 14, 2022 19:36

github-actions bot added the needs changelog Possibly needs a changelog entry label Feb 14, 2022

This was referenced Feb 15, 2022

[smart_holder] git merge master after clang-format etc #3735

Closed

[smart_holder] Curated git merge master after clang-format etc #3736

Merged

MSVC C++20 test_eigen #3741

Merged

henryiii removed the needs changelog Possibly needs a changelog entry label Mar 29, 2022

rwgk mentioned this pull request Feb 10, 2023

FWD pybind11 google/pybind11clif#3722

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dropping MSVC 2015 #3722

Dropping MSVC 2015 #3722

rwgk commented Feb 12, 2022 •

edited

Loading

rwgk commented Feb 12, 2022

henryiii Feb 13, 2022 •

edited

Loading

Skylion007 Feb 13, 2022

rwgk Feb 13, 2022

henryiii Feb 13, 2022

rwgk Feb 13, 2022

henryiii Feb 14, 2022

rwgk Feb 14, 2022

rwgk Feb 14, 2022

henryiii Feb 14, 2022

henryiii Feb 14, 2022

rwgk Feb 14, 2022

rwgk Feb 14, 2022

henryiii left a comment

rwgk commented Feb 14, 2022

rwgk commented Feb 14, 2022

Dropping MSVC 2015 #3722

Dropping MSVC 2015 #3722

Conversation

rwgk commented Feb 12, 2022 • edited Loading

Description

Suggested changelog entry:

rwgk commented Feb 12, 2022

henryiii Feb 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

henryiii left a comment

Choose a reason for hiding this comment

rwgk commented Feb 14, 2022

rwgk commented Feb 14, 2022

rwgk commented Feb 12, 2022 •

edited

Loading

henryiii Feb 13, 2022 •

edited

Loading