Skip to content

feat: adopt C++20 defaulted comparisons and spaceship operator #1761

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dave-365 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: adopt C++20 defaulted comparisons and spaceship operator #1761

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions napi-inl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -604,10 +604,12 @@ bool Maybe<T>::operator==(const Maybe& other) const {
(!IsJust() || Unwrap() == other.Unwrap());
}

#if !defined(NAPI_HAS_CPP20_COMPARISONS)
template <class T>
bool Maybe<T>::operator!=(const Maybe& other) const {
return !operator==(other);
}
#endif

template <class T>
Maybe<T>::Maybe() : _has_value(false) {}
Expand Down Expand Up @@ -790,9 +792,11 @@ inline bool Value::operator==(const Value& other) const {
return StrictEquals(other);
}

#if !defined(NAPI_HAS_CPP20_COMPARISONS)
inline bool Value::operator!=(const Value& other) const {
return !this->operator==(other);
}
#endif

inline bool Value::StrictEquals(const Value& other) const {
bool result;
Expand Down Expand Up @@ -1961,10 +1965,12 @@ inline bool Object::const_iterator::operator==(
return _index == other._index;
}

#if !defined(NAPI_HAS_CPP20_COMPARISONS)
inline bool Object::const_iterator::operator!=(
const const_iterator& other) const {
return _index != other._index;
}
#endif

inline const std::pair<Value, Object::PropertyLValue<Value>>
Object::const_iterator::operator*() const {
Expand Down Expand Up @@ -1998,9 +2004,11 @@ inline bool Object::iterator::operator==(const iterator& other) const {
return _index == other._index;
}

#if !defined(NAPI_HAS_CPP20_COMPARISONS)
inline bool Object::iterator::operator!=(const iterator& other) const {
return _index != other._index;
}
#endif

inline std::pair<Value, Object::PropertyLValue<Value>>
Object::iterator::operator*() {
Expand Down Expand Up @@ -3746,10 +3754,12 @@ inline bool Reference<T>::operator==(const Reference<T>& other) const {
return this->Value().StrictEquals(other.Value());
}

#if !defined(NAPI_HAS_CPP20_COMPARISONS)
template <typename T>
inline bool Reference<T>::operator!=(const Reference<T>& other) const {
return !this->operator==(other);
}
#endif

template <typename T>
inline Napi::Env Reference<T>::Env() const {
Expand Down
35 changes: 35 additions & 0 deletions napi.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,17 @@
#include <utility>
#include <vector>

#if (defined(__cpp_impl_three_way_comparison) && \
__cpp_impl_three_way_comparison >= 201907L) || \
(__cplusplus >= 202002L) || (defined(_MSVC_LANG) && _MSVC_LANG >= 202002L)
#if defined(__has_include)
#if __has_include(<compare>)
#include <compare>
#define NAPI_HAS_CPP20_COMPARISONS 1
#endif
#endif
#endif

// VS2015 RTM has bugs with constexpr, so require min of VS2015 Update 3 (known
// good version)
#if !defined(_MSC_VER) || _MSC_FULL_VER >= 190024210
Expand Down Expand Up @@ -292,7 +303,18 @@ class Maybe {
bool UnwrapTo(T* out) const;

bool operator==(const Maybe& other) const;
#if !defined(NAPI_HAS_CPP20_COMPARISONS)
bool operator!=(const Maybe& other) const;
#else
template <typename U = T>
constexpr auto operator<=>(const Maybe<U>& other) const
-> std::compare_three_way_result_t<U> {
if (_has_value && other._has_value) {
return _value <=> other._value;
}
return _has_value <=> other._has_value;
}
#endif

private:
Maybe();
Expand Down Expand Up @@ -361,9 +383,14 @@ class BasicEnv {
// 'Napi::Env' and 'Napi::Env')
//
// ... occurs when comparing foo.Env() == bar.Env() or foo.Env() == nullptr
#if defined(NAPI_HAS_CPP20_COMPARISONS)
bool operator==(const BasicEnv& other) const = default;
auto operator<=>(const BasicEnv& other) const = default;
#else
bool operator==(const BasicEnv& other) const {
return _env == other._env;
}
#endif
bool operator==(std::nullptr_t /*other*/) const {
return _env == nullptr;
}
Expand Down Expand Up @@ -505,8 +532,10 @@ class Value {
/// Tests if this value strictly equals another value.
bool operator==(const Value& other) const;

#if !defined(NAPI_HAS_CPP20_COMPARISONS)
/// Tests if this value does not strictly equal another value.
bool operator!=(const Value& other) const;
#endif

/// Tests if this value strictly equals another value.
bool StrictEquals(const Value& other) const;
Expand Down Expand Up @@ -1258,7 +1287,9 @@ class Object::const_iterator {

inline bool operator==(const const_iterator& other) const;

#if !defined(NAPI_HAS_CPP20_COMPARISONS)
inline bool operator!=(const const_iterator& other) const;
#endif

inline const std::pair<Value, Object::PropertyLValue<Value>> operator*()
const;
Expand All @@ -1282,7 +1313,9 @@ class Object::iterator {

inline bool operator==(const iterator& other) const;

#if !defined(NAPI_HAS_CPP20_COMPARISONS)
inline bool operator!=(const iterator& other) const;
#endif

inline std::pair<Value, Object::PropertyLValue<Value>> operator*();

Expand Down Expand Up @@ -1834,7 +1867,9 @@ class Reference {

operator napi_ref() const;
bool operator==(const Reference<T>& other) const;
#if !defined(NAPI_HAS_CPP20_COMPARISONS)
bool operator!=(const Reference<T>& other) const;
#endif

Napi::Env Env() const;
bool IsEmpty() const;
Expand Down
37 changes: 37 additions & 0 deletions test/basic_types/value.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,41 @@ static Value ValueReturnsCorrectEnv(const CallbackInfo& info) {
return Boolean::New(info.Env(), testValue.Env() == info.Env());
}

static Value TestEnvComparisons(const CallbackInfo& info) {
Env env = info.Env();
Env env2 = info.Env();
bool result = (env == env2) && !(env != env2) && (env != nullptr) &&
!(env == nullptr) && (nullptr != env) && !(nullptr == env);
#if defined(NAPI_HAS_CPP20_COMPARISONS)
result = result && ((env <=> env2) == 0);
#endif
return Boolean::New(info.Env(), result);
}

static Value TestMaybeComparisons(const CallbackInfo& info) {
Maybe<int> m_none1 = Nothing<int>();
Maybe<int> m_none2 = Nothing<int>();
Maybe<int> m_one = Just<int>(1);
Maybe<int> m_two = Just<int>(2);

bool ok = (m_none1 == m_none2) && (m_none1 != m_one) && (m_one == m_one) &&
(m_one != m_two);
#if defined(NAPI_HAS_CPP20_COMPARISONS)
ok = ok && (m_none1 < m_one) && (m_one > m_none1) && (m_one < m_two) &&
(m_two > m_one) && (m_one <= m_two) && (m_two >= m_one) &&
(m_one <= m_one) && (m_one >= m_one) && ((m_none1 <=> m_none2) == 0) &&
((m_none1 <=> m_one) < 0) && ((m_one <=> m_none1) > 0) &&
((m_one <=> m_two) < 0) && ((m_two <=> m_one) > 0);

Maybe<double> md_none = Nothing<double>();
Maybe<double> md_one = Just<double>(1.5);
Maybe<double> md_two = Just<double>(2.5);
ok = ok && (md_none < md_one) && (md_one < md_two) &&
((md_one <=> md_two) < 0);
#endif
return Boolean::New(info.Env(), ok);
}

static Value EmptyValueReturnNullPtrOnCast(const CallbackInfo& info) {
Value emptyValue;
bool isNullPtr = static_cast<napi_value>(emptyValue) == nullptr;
Expand Down Expand Up @@ -164,6 +199,8 @@ Object InitBasicTypesValue(Env env) {

exports["assertValueReturnsCorrectEnv"] =
Function::New(env, ValueReturnsCorrectEnv);
exports["assertEnvComparisons"] = Function::New(env, TestEnvComparisons);
exports["assertMaybeComparisons"] = Function::New(env, TestMaybeComparisons);

exports["assertEmptyValReturnNullPtrOnCast"] =
Function::New(env, EmptyValueReturnNullPtrOnCast);
Expand Down
2 changes: 2 additions & 0 deletions test/basic_types/value.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ function test (binding) {
assertValueStrictlyEqual(value);
assertValueStrictlyNonEqual(value);
assertValueReturnsCorrectEnv(value);
assert(value.assertEnvComparisons());
assert(value.assertMaybeComparisons());
assertEmptyValueNullPtrOnCast(value);
assertNonEmptyReturnValOnCast(value);

Expand Down
15 changes: 15 additions & 0 deletions test/binding.gyp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,21 @@
'sources': ['>@(build_sources)'],
'defines': ['NODE_ADDON_API_ENABLE_TYPE_CHECK_ON_AS']
},
{
'target_name': 'binding_cpp20',
'dependencies': ['../node_addon_api.gyp:node_addon_api_except'],
'sources': ['>@(build_sources)'],
'defines': ['NODE_ADDON_API_ENABLE_TYPE_CHECK_ON_AS'],
'cflags_cc': ['-std=c++20'],
'msvs_settings': {
'VCCLCompilerTool': {
'AdditionalOptions': ['/std:c++20']
}
},
'xcode_settings': {
'CLANG_CXX_LANGUAGE_STANDARD': 'c++20'
}
},
{
'target_name': 'binding_except_all',
'dependencies': ['../node_addon_api.gyp:node_addon_api_except_all'],
Expand Down
14 changes: 10 additions & 4 deletions test/common/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,12 +161,15 @@ exports.whichBuildType = whichBuildType;

exports.runTest = async function (test, buildType, buildPathRoot = process.env.BUILD_PATH || '') {
buildType = buildType || await whichBuildType();
const fs = require('fs');
const bindings = [
path.join(buildPathRoot, `../build/${buildType}/binding.node`),
path.join(buildPathRoot, `../build/${buildType}/binding_noexcept.node`),
path.join(buildPathRoot, `../build/${buildType}/binding_noexcept_maybe.node`),
path.join(buildPathRoot, `../build/${buildType}/binding_custom_namespace.node`)
].map(it => require.resolve(it));
path.join(buildPathRoot, `../build/${buildType}/binding_custom_namespace.node`),
path.join(buildPathRoot, `../build/${buildType}/binding_cpp20.node`)
].filter(it => fs.existsSync(path.resolve(__dirname, it)))
.map(it => require.resolve(it));

for (const item of bindings) {
await Promise.resolve(test(require(item), { bindingPath: item }))
Expand All @@ -177,12 +180,15 @@ exports.runTest = async function (test, buildType, buildPathRoot = process.env.B
exports.runTestWithBindingPath = async function (test, buildType, buildPathRoot = process.env.BUILD_PATH || '') {
buildType = buildType || await whichBuildType();

const fs = require('fs');
const bindings = [
path.join(buildPathRoot, `../build/${buildType}/binding.node`),
path.join(buildPathRoot, `../build/${buildType}/binding_noexcept.node`),
path.join(buildPathRoot, `../build/${buildType}/binding_noexcept_maybe.node`),
path.join(buildPathRoot, `../build/${buildType}/binding_custom_namespace.node`)
].map(it => require.resolve(it));
path.join(buildPathRoot, `../build/${buildType}/binding_custom_namespace.node`),
path.join(buildPathRoot, `../build/${buildType}/binding_cpp20.node`)
].filter(it => fs.existsSync(path.resolve(__dirname, it)))
.map(it => require.resolve(it));

for (const item of bindings) {
await test(item);
Expand Down
22 changes: 22 additions & 0 deletions test/maybe/check.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,28 @@ void TestMaybeOperatorOverload(const CallbackInfo& info) {

assert(fn_a.Call({}) == fn_a.Call({}));
assert(fn_a.Call({}) != fn_b.Call({}));
#if defined(NAPI_HAS_CPP20_COMPARISONS)
Maybe<int> m_none1 = Nothing<int>();
Maybe<int> m_none2 = Nothing<int>();
Maybe<int> m_one = Just<int>(1);
Maybe<int> m_two = Just<int>(2);

assert(m_none1 == m_none2);
assert(m_none1 != m_one);
assert(m_none1 < m_one);
assert(m_one > m_none1);
assert(m_one < m_two);
assert(m_two > m_one);
assert(m_one <= m_two);
assert(m_two >= m_one);
assert(m_one <= m_one);
assert(m_one >= m_one);
assert((m_none1 <=> m_none2) == 0);
assert((m_none1 <=> m_one) < 0);
assert((m_one <=> m_none1) > 0);
assert((m_one <=> m_two) < 0);
assert((m_two <=> m_one) > 0);
#endif
}

void NormalJsCallback(const CallbackInfo& info) {
Expand Down