Use custom definitions for negation and bool_constant

[jcelerier]

Instead of relying on sometimes missing C++17 definitions

As requested per PR#735

[dean0x7d]

Replace this line with the following to fix the MSVC internal compiler error:

template <typename T> struct negation : bool_constant<!T::value> { };

The other compilers can get away with just the alias, but I don't think it's worth splitting up the implementation over it.

[jagerman]

In general, I'm a little hesitant to bother worrying about significantly incomplete C++17 support in compilers (or the associated stl, in this case); we're talking about experimental support for a not-yet-released C++ standard.

In this case, the workaround is pretty simple (and actually slightly reduces code), so it seems acceptable; but there's a reason we do the C++17 travis-ci testing on a gcc/stdlibc++ 7 snapshot and clang/libc++ 4.0 release: those are the first to support a reasonable set of C++17 features. (I believe AppleClang/libc++ is based on LLVM 3.9).

So I guess what I'm saying is: "doesn't work in unreleased standard mode on a non-cutting edge compiler" really doesn't seem an appropriate PR justification for anything more than a trivial change.

[dean0x7d]

@jagerman I agree completely.

[jcelerier]

In general, I'm a little hesitant to bother worrying about significantly incomplete C++17 support in compilers

I still think that the right way for this is to rely on feature-test macros (here's a list) : it does not add much code and you can be certain that what you want to use is there.

[jagerman]

Unfortunately MSVC doesn't support them, so it would still need the MSVC version checks, but yes, I agree that that's a better approach.

Regarding this PR: part of the reason I like the using std::negation and similarly for bool_constant isn't that it buys us anything in terms of efficiency, but rather that it makes our intention instantly clear: what we want is C++17's std::negation, but if that isn't available, we provide our own implementation. (And similarly for C++14 features like enable_if_t). With this PR, yes, we're still doing that, but it's slightly less obvious that our intention here is to backport a C++17 feature.

[jagerman]

As such I think I'd prefer this PR if, instead of removing the "using", it replaced the condition with

#if defined(__cpp_lib_logical_traits) || defined(_MSC_VER)

(Then you also don't need the MSVC struct fix).

[jagerman]

(Or you could accomplish the same with a simple comment to that effect; I'll let others weigh in).

[dean0x7d]

I thought removing the condition would simplify things since the std implementation really doesn't buy us anything in this case. But ultimately I don't have strong feelings either way and I'll defer to you and @jcelerier.

I will say the SD-6 feature-test macros look great and I wasn't aware that they were semi-standardized. I'm definitely in favor of using them for C++17 features going forward since it looks like GCC, clang and Intel/EDG support them.

[dean0x7d]

Merged, thanks @jcelerier!

[jcelerier]

thanks to you ! we can finally get back to work :p