[typeof][scope_exit] VS2017 and option /permissive-
Hello, late last year, VS2017RC gained a compiler option /permissive- which disables some non-conforming behaviour. Source code that was accepted by previous versions of msvc, and still is accepted by msvc 14.1 without this compiler option, is rejected and flagged as an error if /permissive- is engaged. For more on this see https://blogs.msdn.microsoft.com/vcblog/2016/11/16/permissive-switch/. While checking our company's codebase for possible problems related to that I noticed compile errors in Boost.Log and traced the problem back to Boost.Typeof and in particular "libs\typeof\include\boost\typeof\msvc\typeof_impl.hpp". Therein is code emulating the missing __typeof__ keyword by taking advantage of a silent msvc compiler 'bug' as a 'feature'. Parts of this code are duplicated in a msvc-specific workaround in Boost.Scope_Exit. This compiler bug is no longer silent with /permissive- in place but rather a hard error. As it turns out, I'm not the first to be bitten by this: ticket #12900 tells the same story. Many other Boost libraries use Boost.Typeof directly or indirectly and therefore suffer from the same problem (like Boost.Log in my particular case). I've implemented a possible solution and created two pull-requests: - https://github.com/boostorg/typeof/pull/7 - https://github.com/boostorg/scope_exit/pull/3 The patches only affect msvc 14.1. With these patches applied, the test results of the full Boost testsuite are the same as without these patches. I don't know enough about __typeof__ to decide if my emulation is compatible in every aspect, but at least it's good enough to pass the testsuite. There are two other compile failures introduced with /permissive- in place which are compiler bugs which need to be addressed by Microsoft's compiler team. Fortunately these are very specific to just two libraries, Boost.Spirit.Repository (one test case) and Boost.Geometry (two test cases caused by one single function). -- PGP/GPG: 2CCB 3ECB 0954 5CD3 B0DB 6AA0 BA03 56A1 2C4638C5
Daniela Engert wrote: ...
I've implemented a possible solution and created two pull-requests: - https://github.com/boostorg/typeof/pull/7
I think that a more principled solution here would be to check
!defined(BOOST_NO_CXX11_DECLTYPE) at the very start of typeof.hpp (if
BOOST_TYPEOF_EMULATION is not defined) and then use a generic decltype-based
implementation. Something like
-#if defined(__COMO__)
+#if !defined(BOOST_NO_CXX11_DECLTYPE) && !defined(BOOST_TYPEOF_EMULATION)
+# ifndef BOOST_TYPEOF_NATIVE
+# define BOOST_TYPEOF_NATIVE
+# endif
+
+#elif defined(__COMO__)
(which would also require the inclusion of
Am 15.04.2017 um 15:05 schrieb Peter Dimov via Boost:
Daniela Engert wrote: ...
I've implemented a possible solution and created two pull-requests: - https://github.com/boostorg/typeof/pull/7
I think that a more principled solution here would be to check !defined(BOOST_NO_CXX11_DECLTYPE) at the very start of typeof.hpp (if BOOST_TYPEOF_EMULATION is not defined) and then use a generic decltype-based implementation. Something like ... where decltype.hpp is more or less the same as the suggested msvc/typeof_impl_decltype.hpp, but without ensure_obj (shouldn't be needed with decltype) and with remove_cv_ref instead of remove_reference.
Except that we don't have remove_cv_ref in TypeTraits, so remove_cv
.
Thanks for this suggestion, Peter, I was thinking along the same lines as well. But for a start, I rather went for the low-risk path affecting only one compiler. During my experiments to find a fix for my problem I noticed that the decltype-based implemention required a template type alias to work thoughout all Boost libraries. Otherwise the compiler would complain about either a missing 'typename' keyword or a superfluous one. Thus, as long as there is no better way to deal with that the first test would have to check for !defined(BOOST_NO_CXX11_TEMPLATE_ALIASES) as well. I will implement this in the next couple of minutes and start-off another run of the full Boost test-suite to see what's happening... Any comments on that? Ciao Dani -- PGP/GPG: 2CCB 3ECB 0954 5CD3 B0DB 6AA0 BA03 56A1 2C4638C5
Daniela Engert wrote:
Thanks for this suggestion, Peter, I was thinking along the same lines as well. But for a start, I rather went for the low-risk path affecting only one compiler. During my experiments to find a fix for my problem I noticed that the decltype-based implemention required a template type alias to work thoughout all Boost libraries. Otherwise the compiler would complain about either a missing 'typename' keyword or a superfluous one.
Interesting. I didn't think of that but you're right. This is probably the reason qualifiers are stripped via a helper function and not with a type trait in the original implementation.
Am 15.04.2017 um 16:23 schrieb Peter Dimov via Boost:
Interesting. I didn't think of that but you're right. This is probably the reason qualifiers are stripped via a helper function and not with a type trait in the original implementation.
I've reworked my pull-request by incorporating your suggestions with some minor additional tweaks required to make it work. The tests of Boost.Typeof pass with msvc10, msvc12, msvc14, and msvc14.1, and a test run of the full Boost testsuite with msvc14.1 is successful as well. Given the generality of the suggested change, I'd appreciate if someone would test it on gcc and clang, too. Ciao Dani -- PGP/GPG: 2CCB 3ECB 0954 5CD3 B0DB 6AA0 BA03 56A1 2C4638C5
Daniela Engert wrote:
I've reworked my pull-request by incorporating your suggestions with some minor additional tweaks required to make it work. The tests of Boost.Typeof pass with msvc10, msvc12, msvc14, and msvc14.1, and a test run of the full Boost testsuite with msvc14.1 is successful as well. Given the generality of the suggested change, I'd appreciate if someone would test it on gcc and clang, too.
Tests pass for me on Cygwin g++ 6.3.0 (03/11/14/1z), clang++ 3.9.1 (03/11/14/1z), msvc-8.0, 10.0, 11.0, 12.0, 14.1.
+#define BOOST_TYPEOF(expr) boost::type_of::remove_cv_ref_t
The ensure_obj helper shouldn't be necessary here. The above tests also pass
with
+#define BOOST_TYPEOF(expr) boost::type_of::remove_cv_ref_t
Am 15.04.2017 um 23:52 schrieb Peter Dimov via Boost:
Daniela Engert wrote:
Tests pass for me on Cygwin g++ 6.3.0 (03/11/14/1z), clang++ 3.9.1 (03/11/14/1z), msvc-8.0, 10.0, 11.0, 12.0, 14.1.
Great - thanks!
+#define BOOST_TYPEOF(expr) boost::type_of::remove_cv_ref_t
The ensure_obj helper shouldn't be necessary here. The above tests also pass with
I remember seeing failures without that in former test runs, but that might have been caused by other missing pieces.
I've also enabled Travis for typeof, so any future pushes in the PR should be CI-tested.
Thanks a lot, Travis is 'terra incognita' to me.
Tests fail with msvc-14.1 /std:c++latest because of use of binder1st, auto_ptr, unary_function and so on, but that's a separate issue and unrelated to these changes, although we'll probably have to fix it at some point.
Well, all my tests with msvc-14.0 and msvc-14.1 run with /std:c++latest. This engages different, c++17-related code paths which already exhibited an error in one Boost library (fixed now). And test runs with msvc-14.1 have /permissive- attached from now on. Ciao Dani -- PGP/GPG: 2CCB 3ECB 0954 5CD3 B0DB 6AA0 BA03 56A1 2C4638C5
Daniela Engert wrote:
I've reworked my pull-request by incorporating your suggestions with some minor additional tweaks required to make it work.
+#if !defined(BOOST_NO_CXX11_DECLTYPE) && !defined(BOOST_NO_CXX11_TEMPLATE_ALIASES) +# define BOOST_TYPEOF_DECLTYPE +# if !defined(BOOST_TYPEOF_EMULATION) && !defined(BOOST_TYPEOF_NATIVE) +# define BOOST_TYPEOF_NATIVE +# endif +#endif + #if defined(__COMO__)
I'd think that if we hit the decltype case, we no longer need to enter the compiler-specific branches below that. So #if !defined(BOOST_NO_CXX11_DECLTYPE) && !defined(BOOST_NO_CXX11_TEMPLATE_ALIASES) && !defined(BOOST_TYPEOF_EMULATION) # define BOOST_TYPEOF_DECLTYPE # if !defined(BOOST_TYPEOF_NATIVE) # define BOOST_TYPEOF_NATIVE # endif #elif defined(__COMO__) perhaps? Tested as before.
Am 16.04.2017 um 00:11 schrieb Peter Dimov via Boost:
I'd think that if we hit the decltype case, we no longer need to enter the compiler-specific branches below that. So
#if !defined(BOOST_NO_CXX11_DECLTYPE) && !defined(BOOST_NO_CXX11_TEMPLATE_ALIASES) && !defined(BOOST_TYPEOF_EMULATION) # define BOOST_TYPEOF_DECLTYPE # if !defined(BOOST_TYPEOF_NATIVE) # define BOOST_TYPEOF_NATIVE # endif
#elif defined(__COMO__)
perhaps? Tested as before.
True. The pull-request is reworked along that line and I like the result of that much more than my original code. It has the additional benefit that it can be easily rebased or cherry-picked onto master (like I do with my take on Boost 1.64). My tests with msvc pass, and Travis is happy with it as well. Ciao Dani -- PGP/GPG: 2CCB 3ECB 0954 5CD3 B0DB 6AA0 BA03 56A1 2C4638C5
participants (2)
-
Daniela Engert -
Peter Dimov