mapbox / variant

C++11/C++14 Variant
BSD 3-Clause "New" or "Revised" License
371 stars 100 forks source link

revert MSVC non-work-around for std::is_nothrow_destructible #90

Closed lightmare closed 8 years ago

lightmare commented 8 years ago

Going back to using std::is_nothrow_destructible. It probably won't fix the original issue -- errors stating wrong noexcept on virtual destructors coming from plugins (https://github.com/mapnik/mapnik/issues/3277#issuecomment-177471394, https://github.com/mapbox/windows-builds/issues/76). However I think having std trait will be a better starting point for further investigation than some ugly hack. I'll try to devise some tests to check how it behaves with recursive_wrapper and virtual destructors.

With this, mapnik at least doesn't error out immediately with cannot delete objects that are not pointers -- I got two builds aborted after an hour (https://ci.appveyor.com/project/lightmare/mapnik/build/%237.test-variant-87/messages), unfortunately they didn't get to compile plugins.

lightmare commented 8 years ago

I'm giving up, it's time to declare the destructor noexcept(true)

https://ci.appveyor.com/project/lightmare/variant/build/job/jae1h7b357ebq6te

..\test\t\noexcept.cpp(109): FAILED:
  CHECK( (mapbox::util::detail::is_nothrow_destructible<variant<int, float>>::value) == true )
with expansion:
  false == true

^ that one should have the same result as std::is_nothrow_destructible<std::tuple<int, float>>, and if that is false...

btw: any idea why failing CHECK_NOFAIL aren't in the output?

lightmare commented 8 years ago

I'm closing this pull request as its pointless, and suggest reverting https://github.com/mapbox/variant/commit/24391a9ea3e4b060c7a62debf6eb4e3515b9a5e6 completely and declaring ~variant() noexcept // (true). All the trouble is not worth it (allowing alternatives with throwing destructors).