Fix to compile with Visual C++ and /Zc:implicitNoexcept-.

[jaykrell]

Otherwise gets: Error C2694 'override': overriding virtual function has less restrictive exception specification than base class virtual member function 'base'

Similar changes are being made e.g.: https://github.com/boostorg/json/pull/636

And proposed here by me: https://github.com/boostorg/iostreams/pull/136

I grant there there could be more of this. These two are just enough for our codebase.

[mclow]

Why would anyone want destructors to not be noexcept by default?

[mclow]

Ah: (from https://docs.microsoft.com/en-us/cpp/build/reference/zc-implicitnoexcept-implicit-exception-specifiers?view=msvc-160)

If the option is disabled by specifying /Zc:implicitNoexcept-, no implicit exception specifiers are generated by the compiler. This behavior is the same as Visual Studio 2013, where destructors and deallocators that did not have exception specifiers could have throw statements.

it's to be compatible with old, broken versions of MSVC.

[jaykrell]

Why is the compiler option available?

We believe we have code that depends on the old behavior. And we do get compilation errors where when we upgrade the compiler. (I didn't arrive here randomly. :) )

I can make up code that depends on it, at least, but not necessarily that matches real world code (imagine there is a higher level catch, that cleans up thread locals or terminates the thread, or something, with very careful management of resources).

Boost should not legislate or stand in the way here? This doesn't make Boost worse? (Unless compilation with old pre-noexcept pre-default compilers?)

[jaykrell]

Honestly I am a bit unsure, but this where we are.

[mclow]

Why is the compiler option available?

I think I answered that. Because an old version of MSVC implemented this (incorrect, or if you prefer a non-pejorative description - nonstandard) behavior.

I can make up code that depends on it, at least, but not necessarily that matches real world code (imagine there is a higher level catch, that cleans up thread locals or terminates the thread, or something, with very careful management of resources).

I'm sure you can; but if you want to write portable code, you should mark those destructors with noexcept(false).

This doesn't make Boost worse? (Unless compilation with old pre-noexcept pre-default compilers?)

Sure it does - in a minor way. Some of our classes will have implicitly generated destructors, (whose exception specification will change from compiler to compiler, because of this MSVC misfeature), while others (such as basic_oaltstringstream if this is landed) will not.

[jaykrell]

Notice how the json motivation kinda came and went. The users eventually fixed their code. Maybe we will too eventually. I just figure, it is fairly harmless to support here. The meaning of this code is not changed -- in fact it is preserved.

[jaykrell]

Our code is really not portable, very Windows-specific, fairly hopelessly so, and likely will remain that way for a very long time. This is not the biggest problem by far. If we really wanted portability in this regard, we'd run some large tool over our code and mark all destructors as noexcept(false), and pretend we got everything (i.e. ignoring third party code).

[mclow]

Notice how the json motivation kinda came and went. The users eventually fixed their code.

I did notice that. But by that time Peter had landed the patch, and so ASIO has this in it.

Maybe we will too eventually. I just figure, it is fairly harmless to support here. The meaning of this code is not changed -- in fact it is preserved.

I'm not saying "No, I won't land this patch", but I'm trying to understand the necessity, and figure out what the best way to resolve this problem.

It may seem that I'm saying "Well, the best way is for you to fix your code and stop using this dumb compiler 'feature'", but that's really not what I'm saying.

[jaykrell]

whose exception specification will change from compiler to compiler

Isn't it the other way around? This tries to make the exception specification always the same?

[mclow]

whose exception specification will change from compiler to compiler

Isn't it the other way around? This tries to make the exception specification always the same?

Not unless we use it everywhere in boost.

[jaykrell]

Not unless we use it everywhere in boost.

Yeah, I kinda admitted that. My PRs only cover what I hit in our code. Selfish. Or a starting point. :) But, "only" for users of this switch, right? Or maybe, it is more complicated than that? I mean: struct { ~A() noexcept(false) { } }; struct HasA { A a; }; // implicit noexcept(false) destructor?

I mean, sprinkling noexcept on all destructors..isn't actually correct, I guess?

[jaykrell]

Right..there is a sort of transitiveness of noexcept(false) https://docs.microsoft.com/en-us/cpp/build/reference/zc-implicitnoexcept-implicit-exception-specifiers: " For user-defined destructors, the implicit exception specifier is noexcept(true) unless a contained member class or base class has a destructor that is not noexcept(true). For compiler-generated special member functions, if any function directly invoked by this function is effectively noexcept(false), the implicit exception specifier is noexcept(false) "

so sprinkling noexcept has to be mindful of the base, members, etc., else could get it wrong.

[mclow]

Looks like "Environment: FLAVOR=Visual Studio 2019, APPVEYOR_BUILD_WORKER_IMAGE=Visual Studio 2019, B2_ADDRESS_MODEL=address-model=64, B2_CXXFLAGS=cxxflags=-permissive-, B2_CXXSTD=latest, B2_TOOLSET=msvc-14.2" didn't like your change, even though all the other ones did.

[jaykrell]

https://ci.appveyor.com/project/jeking3/format-bhjc4/builds/41225712/job/p0rc2y68a1xtdh6q

\boost/format/feed_args.hpp(99): error C2280: 'std::basic_ostream<char,std::char_traits<char>> &std::operator <<<std::char_traits<char>>(std::basic_ostream<char,std::char_traits<char>> &,const wchar_t *)': attempting to reference a deleted function
C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.29.30133\include\ostream(937): note: see declaration of 'std::operator <<'
.\boost/f

libs\format\tools\format_matrix.cpp(273): note: see reference to function template instantiation 'boost::basic_format<char,std::char_traits<char>,std::allocator<char>> &boost::basic_format<char,std::char_traits<char>,std::allocator<char>>::operator %<const wchar_t*>(const T &)' being compiled
        with
        [
            T=const wchar_t *
        ]
.\boost/format/feed_args.hpp(99): error C2088: '<<': illegal for class
    call "bin.v2\standalone\msvc\msvc-14.2\msvc-setup.bat"  >nul

Huh, that is close to what we are using, except we don't use -fpermissive-. I wonder if this is forcing instantation of something invalid?

[jaykrell]

Rude/lazy question: Was CI working before my change? Can we PR a nop/whitespace/comment change and see how it does?

[jaykrell]

If we are talking about the same warnings/errors, this does appear to be a preexisting condition. If having someone/me fix that first is a precondition for accepting my PR, I sympathize.

[jeking3]

@jaykrell CI is working again, if you rebase it'll run everything now.

[jeking3]

@jaykrell CI is working again, if you rebase it'll run everything now.