boostorg / move

Boost.org move module
http://boost.org/libs/move
Boost Software License 1.0
19 stars 55 forks source link

New nothrow move traits are incomplete #35

Closed palebedev closed 3 years ago

palebedev commented 3 years ago

The following fails to compile with boost 1.75, and recent clang+libc++ in C++17+:

#include <boost/container/small_vector.hpp>

#include <type_traits>

struct S
{
    // All non-trivial, but noexcept
    S() noexcept {}
    S(const S&) noexcept {}
    S(S&&) noexcept {}
    S& operator=(const S&) noexcept { return *this; }
    S& operator=(S&&) noexcept { return *this; }
};

static_assert(std::is_nothrow_move_constructible_v<boost::container::small_vector<S,2>>);

This fails due to small_vector's move constructor noexcept specification being boost::container::dtl::is_nothrow_move_assignable<value_type>::value which defers to BOOST_MOVE_IS_NOTHROW_MOVE_ASSIGNABLE(T).

It appears that a recent update in 9a77e69b only added definitions of BOOST_MOVE_HAS_NOTHROW_MOVE_{ASSIGN,CONSTRUCT} for MSVC++, but not for other compilers including clang, which causes them to use a simplified and imprecise emulation based on triviality.

After adding

#     define BOOST_MOVE_HAS_NOTHROW_MOVE_ASSIGN(T) (__is_assignable(T, T &&) && __is_nothrow_assignable(T, T &&))

after BOOST_MOVE_HAS_NOTHROW_ASSIGN and

#     define BOOST_MOVE_HAS_NOTHROW_MOVE_CONSTRUCT(T) (__is_constructible(T, T &&) && __is_nothrow_constructible(T, T &&))

after BOOST_MOVE_HAS_NOTHROW_COPY it still doesn't work.

Turns out, the way BOOST_MOVE_HAS_TRAIT is defined for Clang is wrong. As stated in https://clang.llvm.org/docs/LanguageExtensions.html#type-trait-primitives __has_feature (which is what __has_extension essentially is) only detects a listed subset of traits, not including is_assignable and others which Boost.Move tests. A universal test exists for Clang 6+ via !__is_identifier(__trait), but since __is_identifier itself appeared earlier and AppleClang presumably has a different versioning scheme, I opted to replace

#     define BOOST_MOVE_HAS_TRAIT(T) __has_extension(T)

with

#     ifdef __is_identifier
#       define BOOST_MOVE_HAS_TRAIT(T) (__has_extension(T) || !__is_identifier(__##T))
#     else
#       define BOOST_MOVE_HAS_TRAIT(T) __has_extension(T)
#     endif

and this finally fixed this for me.

This fixes it only for clang, other compilers would need something similar too.

igaztanaga commented 3 years ago

Many thanks for the issue. I've added intrinsics for both clang and GCC.