[CXX-2625] More type traits + invoke() #1042

vector-of-bool · 2023-10-19T19:42:13Z

(CXX-2625) This changeset adds many additional type traits from C++14 and beyond, especially for easier "SFINAE trickery". Many are C++14/17 traits, some are from library fundamentals, and some are neither.

This changeset does not fully replace any existing dependencies, but will make it easier to do so going forward, as well as cleaning up some of our own metaprogramming trickery.

eramongodb · 2023-10-23T17:18:17Z

Some type traits in this PR uses alias templates, e.g.:

template <typename Type, typename... Traits>
using requires_t = enable_if_t<conjunction<Traits...>::value, Type>;

Additionally, some constraints are expressed as return types (the traditional std::enable_if pattern):

template <typename T>
bsoncxx::stdx::requires_t<void, bsoncxx::stdx::is_alike<T, int>> //
only_int(T) {} // Example A

Instead, I suggest (1) going all-in on defining type traits using named structs whenever possible (including the implementation of _t variants) instead of alias templates, as is already being done for some type traits (e.g. bsoncxx::stdx::conjunction):

template <typename Type, typename... Traits>
struct requires : std::enable_if<conjunction<Traits...>::value, Type> {};

template <typename Type, typename... Traits>
using requires_t = typename requires<Type, Traits...>::type;

And (2) moving the constraints into the template parameter list instead of the return type (as is already being done for some ctors, e.g. bsoncxx::document::value::value()):

template <typename T, bsoncxx::stdx::requires_t<void, bsoncxx::stdx::is_alike<T, int>>* = nullptr>
void only_int(T) {} // Example B

Both suggestions are to improve the quality of error messages. The currently proposed changes emit the following error message on constraint failure for the only_int example (note: no meaningful information about the failed constraint is visible in the message):

error: no matching function for call to 'only_int'
    only_int(0.0);
    ^~~~~~~~
note: candidate template ignored: substitution failure [with T = double]: no type named 'type' in 'std::enable_if<false>'
only_int(T) {} // Example A
^

Suggestion 1 ensures the named type trait is embedded in the error message, rather than its obscure substituted implementation details (suggestion applied to both requires and is_alike for this example):

error: no matching function for call to 'only_int'
    only_int(0.0);
    ^~~~~~~~
note: candidate template ignored: substitution failure [with T = double]: no type named 'type' in 'bsoncxx::stdx::requires<void, bsoncxx::stdx::is_alike<double, int>>'
only_int(T) {} // Example A
^

Suggestion 2 avoids the noisy mess that obscures the return type in the function declaration (+ the noisy repetition of the constraint when the return type happens to be on the same line as the function name), while still preserving clean SFINAE semantics that is consistent across all functions (including ctors), and also being friendlier to ClangFormat even for complex constraints (no //-injection workarounds required):

error: no matching function for call to 'only_int'
    only_int(0.0);
    ^~~~~~~~
note: candidate template ignored: substitution failure [with T = double]: no type named 'type' in 'bsoncxx::stdx::requires<void, bsoncxx::stdx::is_alike<double, int>>'
void only_int(T) {} // Example B
     ^

Additionally, a suggestion 3: despite the convenience of requires_t, I am partial to using typename std::enable_if<cond..., type>::type despite its verbosity due to the quality of error messages by compilers that special-case substition failure messages for std::enable_if. e.g. given (suggestions 1 and 2 are applied here; note the clean ClangFormat results):

template <typename T,
          bsoncxx::stdx::requires_t<void,
                                    bsoncxx::stdx::is_alike<T, double>,
                                    bsoncxx::stdx::is_alike<T, int>>* = nullptr>
void impossible(T) {}

The error message looks as follows (the entire constraint is emitted as-is):

error: no matching function for call to 'impossible'
    impossible(0.0);
    ^~~~~~~~~~
note: candidate template ignored: substitution failure [with T = double]: no type named 'type' in 'bsoncxx::stdx::requires<void, bsoncxx::stdx::is_alike<double, double>, bsoncxx::stdx::is_alike<double, int>>'
void impossible(T) {}
     ^

Whereas if using explicit std::enable_if:

template <typename T,
          typename std::enable_if<bsoncxx::stdx::is_alike<T, double>::value &&
                                  bsoncxx::stdx::is_alike<T, int>::value>::type* = nullptr>
void impossible(T) {}

The error message looks as follows (more meaningful and specific reason for constraint failure):

error: no matching function for call to 'impossible'
    impossible(0.0);
    ^~~~~~~~~~
note: candidate template ignored: requirement 'bsoncxx::stdx::is_alike<double, int>::value' was not satisfied [with T = double]
void impossible(T) {}
     ^

kevinAlbs · 2023-10-31T13:36:46Z

src/bsoncxx/test/CMakeLists.txt

@@ -22,6 +22,7 @@ else()
    bsoncxx_add_library(bsoncxx_testing "bsoncxx-testing" STATIC)
 endif()

+file(GLOB src_bsoncxx_test_DIST_cpps CONFIGURE_DEPENDS RELATIVE ${CMAKE_CURRENT_SOURCE_DIR} *.cpp)


Remove unused src_bsoncxx_test_DIST_cpps?

Not yet addressed.

src/bsoncxx/test/CMakeLists.txt

src/bsoncxx/test/type_traits.test.cpp

src/bsoncxx/include/bsoncxx/v_noabi/bsoncxx/stdx/type_traits.hpp

src/bsoncxx/include/bsoncxx/v_noabi/bsoncxx/types/bson_value/view.hpp

requires_t for more constraint visibility

vector-of-bool · 2023-11-02T21:12:12Z

@eramongodb The avoidance of instantiating class templates is intentional since it explodes can explode compilation time. I was also unsatisfied with the opaque diagnostic messages on constraint failure, so I've added some changes that will produce better error messages on almost all compilers, and often better than the compilers' special-cases for enable_if.

Most oddities in the placement of the SFINAE constraints are specifically to appease VS2015, which has horribly broken SFINAE behavior. When we drop VS2015 support, we will be safe to relocate the constraints to the template parameter lists in (almost?) all contexts (without needing to add the typename=void disambiguators).

kevinAlbs · 2023-11-03T18:22:17Z

src/bsoncxx/test/CMakeLists.txt

@@ -22,6 +22,7 @@ else()
    bsoncxx_add_library(bsoncxx_testing "bsoncxx-testing" STATIC)
 endif()

+file(GLOB src_bsoncxx_test_DIST_cpps CONFIGURE_DEPENDS RELATIVE ${CMAKE_CURRENT_SOURCE_DIR} *.cpp)


Not yet addressed.

eramongodb

I find it difficult to review what traits are implemented in such a manner to avoid VS 2015 compatibility issues vs. what are not, but the intent/context is understood.

src/bsoncxx/include/bsoncxx/v_noabi/bsoncxx/stdx/type_traits.hpp

eramongodb · 2023-11-06T17:46:06Z

src/bsoncxx/include/bsoncxx/v_noabi/bsoncxx/document/value.hpp

@@ -84,8 +86,7 @@ class BSONCXX_API value {
    /// @param t
    ///   A user-defined object to serialize into a BSON object.
    ///
-    template <typename T,
-              typename std::enable_if<!std::is_same<T, typename array::view>::value, int>::type = 0>
+    template <typename T, _traits::requires_not_t<int, std::is_same<T, array::view>> = 0>


Concern: unlike all other functions in this PR that use SFINAE, this ctor is not declared with BSONCXX_INLINE or BSONCXX_PRIVATE despite being a member of a BSONCXX_API-declared class, meaning instantiations are part of the public ABI. Although this does not yet pose an issue (given it's still a member of the unstable ABI namespace), this might be something we want to address before/during its migration into the stable ABI namespace.

Side-note: we may want to establish a style/coding guideline for the CXX Driver libraries that ABI-stability of template instantiations are not supported even for members of the stable ABI namespace (or classes in that namespace) to avoid the complexity of maintaining ABI-stability of symbols that depend on our traits/SFINAE utilities.

src/bsoncxx/include/bsoncxx/v_noabi/bsoncxx/stdx/type_traits.hpp

eramongodb · 2023-11-06T21:29:13Z

src/bsoncxx/include/bsoncxx/v_noabi/bsoncxx/stdx/type_traits.hpp

+
+namespace bsoncxx {
+
+BSONCXX_INLINE_NAMESPACE_BEGIN


Suggested change

BSONCXX_INLINE_NAMESPACE_BEGIN

inline namespace v_noabi {

Consistency with changes in #1039.

Could that macro be removed completely, or is it too much part of the public API?

It was considered, but I think it's been part of the API for too long to risk its sudden removal. I intend to document them as deprecated as part of CXX-2745 work.

vector-of-bool added 10 commits October 16, 2023 13:48

Bring additional type_traits

9b87cd6

invoke() and friends

9c4b2c3

Trying to fix VS2015

90e4b49

Tweak signature of requires_t

9be7bc7

Minor formatting

dbdd785

More traits usage tweaks

7354ee3

Tweak invoke() to be a callable

30e690a

Incorrect requires_t

e48891f

Merge branch 'master' into more-type_traits

aa8f847

Formatting fixes for lint

f4f60c3

vector-of-bool requested review from kevinAlbs, adriandole and eramongodb October 20, 2023 18:06

Missing dist listing

86e7665

Merge branch 'master' into more-type_traits

effc1fa

kevinAlbs reviewed Nov 2, 2023

View reviewed changes

vector-of-bool added 8 commits November 2, 2023 12:36

Rework namespace shape for type_traits to be private. Refactor of

8db1259

requires_t for more constraint visibility

Missed some traits usages

10f8740

VS 2015 Can't handle our scale

03c898b

Missed a namespace

0c2e0f5

Better diagnostics on non-VS2015

84b9177

tt_detail->detail

d88f258

Add back a not_view alias template

74d92cc

Error message fix

7977ccd

Bump MSVC requirement for better requires_t

cab0029

vector-of-bool requested a review from kevinAlbs November 3, 2023 17:56

kevinAlbs approved these changes Nov 3, 2023

View reviewed changes

eramongodb removed their request for review November 6, 2023 15:49

eramongodb self-requested a review November 6, 2023 15:49

eramongodb requested changes Nov 6, 2023

View reviewed changes

vector-of-bool added 2 commits November 6, 2023 14:15

Rename namespaces to just use "detail" for type traits

73acf08

Unused glob

7a7b651

vector-of-bool requested a review from eramongodb November 6, 2023 21:27

eramongodb approved these changes Nov 6, 2023

View reviewed changes

eramongodb reviewed Nov 6, 2023

View reviewed changes

vector-of-bool added 3 commits November 6, 2023 16:38

VS2015 parser is broken here

d6cf410

Declare the inline namespace directly

b8108a5

Merge branch 'master' into more-type_traits

d0d5ba4

vector-of-bool removed the request for review from adriandole November 7, 2023 19:37

vector-of-bool merged commit ed06ab8 into mongodb:master Nov 7, 2023

eramongodb mentioned this pull request Nov 7, 2023

Use consistent pattern for type_traits header include directives #1054

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[CXX-2625] More type traits + invoke() #1042

[CXX-2625] More type traits + invoke() #1042

Uh oh!

vector-of-bool commented Oct 19, 2023 •

edited

Loading

Uh oh!

eramongodb commented Oct 23, 2023 •

edited

Loading

Uh oh!

kevinAlbs Oct 31, 2023

Uh oh!

kevinAlbs Nov 3, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vector-of-bool commented Nov 2, 2023

Uh oh!

kevinAlbs Nov 3, 2023

Uh oh!

eramongodb left a comment

Uh oh!

Uh oh!

eramongodb Nov 6, 2023

Uh oh!

Uh oh!

eramongodb Nov 6, 2023

Uh oh!

vector-of-bool Nov 6, 2023

Uh oh!

eramongodb Nov 7, 2023

Uh oh!

Uh oh!

[CXX-2625] More type traits + invoke() #1042

[CXX-2625] More type traits + invoke() #1042

Uh oh!

Conversation

vector-of-bool commented Oct 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eramongodb commented Oct 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kevinAlbs Oct 31, 2023

Choose a reason for hiding this comment

Uh oh!

kevinAlbs Nov 3, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vector-of-bool commented Nov 2, 2023

Uh oh!

kevinAlbs Nov 3, 2023

Choose a reason for hiding this comment

Uh oh!

eramongodb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

eramongodb Nov 6, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

eramongodb Nov 6, 2023

Choose a reason for hiding this comment

Uh oh!

vector-of-bool Nov 6, 2023

Choose a reason for hiding this comment

Uh oh!

eramongodb Nov 7, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vector-of-bool commented Oct 19, 2023 •

edited

Loading

eramongodb commented Oct 23, 2023 •

edited

Loading