diff --git a/doc/symbol.md b/doc/symbol.md index 4fa1ae0da..8ed71bbaa 100644 --- a/doc/symbol.md +++ b/doc/symbol.md @@ -59,12 +59,16 @@ static Napi::Symbol Napi::Symbol::For(napi_env env, napi_value description); - `[in] env`: The `napi_env` environment in which to construct the `Napi::Symbol` object. - `[in] description`: The C++ string representing the `Napi::Symbol` in the global registry to retrieve. `description` may be any of: - - `const std::string&` - UTF8 string description. - - `std::string_view` - represents a UTF8 string view. + - `const std::string&` - represents a UTF-8 string. + - `std::string_view` - represents a UTF-8 string view. - `const char*` - represents a UTF8 string description. - `String` - Node addon API String description. - `napi_value` - Node-API `napi_value` description. +String-like arguments implicitly convertible to both `const std::string&` and +`std::string_view` that do not have a unique best match among the non-template +overloads are resolved through `std::string_view`. + Searches in the global registry for existing symbol with the given name. If the symbol already exist it will be returned, otherwise a new symbol will be created in the registry. It's equivalent to Symbol.for() called from JavaScript. [`Napi::Name`]: ./name.md diff --git a/napi-inl.h b/napi-inl.h index ec63ffeee..d81257518 100644 --- a/napi-inl.h +++ b/napi-inl.h @@ -1428,6 +1428,12 @@ inline MaybeOrValue Symbol::For(napi_env env, return Symbol::For(env, descriptionValue); } +template > +inline MaybeOrValue Symbol::For(napi_env env, T&& description) { + std::string_view descriptionView = std::forward(description); + return Symbol::For(env, descriptionView); +} + inline MaybeOrValue Symbol::For(napi_env env, const char* description) { napi_value descriptionValue = String::New(env, description); return Symbol::For(env, descriptionValue); diff --git a/napi.h b/napi.h index 870a5c290..e3ef19e5c 100644 --- a/napi.h +++ b/napi.h @@ -20,6 +20,8 @@ #include #include #include +#include +#include #include // VS2015 RTM has bugs with constexpr, so require min of VS2015 Update 3 (known @@ -786,6 +788,41 @@ class String : public Name { const; ///< Converts a String value to a UTF-16 encoded C++ string. }; +namespace details { + +// This overload set must mirror the non-template Symbol::For overloads. +struct string_convertible_probe { + static void select(const std::string&); + static void select(std::string_view); + static void select(const char*); + static void select(String); + static void select(napi_value); +}; + +template +struct has_unambiguous_symbol_for_overload : std::false_type {}; + +template +struct has_unambiguous_symbol_for_overload< + T, + std::void_t()))>> + : std::true_type {}; + +// Enable the template overload only for string-like arguments that have no +// unique best match among the non-template Symbol::For overloads. +// +// Exclude nullptr because it matches the pointer overloads equally well and +// cannot safely initialize a std::string_view. +template +using enable_if_ambiguous_symbol_for_t = + std::enable_if_t> && + std::is_convertible_v && + std::is_convertible_v && + !has_unambiguous_symbol_for_overload::value, + int>; + +} // namespace details + /// A JavaScript symbol value. class Symbol : public Name { public: @@ -831,6 +868,11 @@ class Symbol : public Name { // Create a symbol in the global registry, UTF-8 encoded cpp string view static MaybeOrValue For(napi_env env, std::string_view description); + // Resolve otherwise ambiguous string-like arguments through the + // std::string_view overload + template = 0> + static MaybeOrValue For(napi_env env, T&& description); + // Create a symbol in the global registry, C style string (null terminated) static MaybeOrValue For(napi_env env, const char* description); diff --git a/test/symbol.cc b/test/symbol.cc index d978739ff..e7c8b7447 100644 --- a/test/symbol.cc +++ b/test/symbol.cc @@ -1,10 +1,62 @@ #include #include +#include #include "test_helper.h" using namespace Napi; +namespace { + +struct StringLike { + operator std::string() const { return "unexpected-string-key"; } + operator std::string_view() const { return value; } + + std::string value; +}; + +struct RvalueStringLike { + operator std::string() && { return "unexpected-rvalue-string-key"; } + operator std::string_view() && { return value; } + + std::string value; +}; + +struct StringOnlyLike { + operator std::string() const { return value; } + + std::string value; +}; + +struct BothBases : std::string, std::string_view {}; + +struct ViewAndNapiString : std::string_view, Napi::String {}; + +struct StringReferenceLike { + operator std::string&() const { return stringValue; } + operator std::string&&() const { return std::move(stringValue); } + operator std::string_view() const { return viewValue; } + + mutable std::string stringValue; + std::string_view viewValue; +}; + +struct ImplicitAndExplicitStringViewLike { + operator std::string() const { return "unexpected-string-key"; } + + // Copy-initialization must ignore the explicit conversion below. + // Direct-initialization would prefer it for a non-const lvalue. + operator std::string_view() const& { return value; } + + explicit operator std::string_view() & { + return "unexpected-explicit-string-view-key"; + } + + std::string_view value; +}; + +} // namespace + Symbol CreateNewSymbolWithNoArgs(const Napi::CallbackInfo&) { return Napi::Symbol(); } @@ -47,6 +99,58 @@ Symbol FetchSymbolFromGlobalRegistryWithStringViewKey( return MaybeUnwrap(Napi::Symbol::For(info.Env(), std::string_view(key))); } +Symbol FetchSymbolFromGlobalRegistryWithStringLikeKey( + const Napi::CallbackInfo& info) { + StringLike key{info[0].As().Utf8Value()}; + return MaybeUnwrap(Napi::Symbol::For(info.Env(), key)); +} + +Symbol FetchSymbolFromGlobalRegistryWithRvalueStringLikeKey( + const Napi::CallbackInfo& info) { + return MaybeUnwrap(Napi::Symbol::For( + info.Env(), RvalueStringLike{info[0].As().Utf8Value()})); +} + +Symbol FetchSymbolFromGlobalRegistryWithStringOnlyLikeKey( + const Napi::CallbackInfo& info) { + StringOnlyLike key{info[0].As().Utf8Value()}; + return MaybeUnwrap(Napi::Symbol::For(info.Env(), key)); +} + +Symbol FetchSymbolFromGlobalRegistryWithBothBasesKey( + const Napi::CallbackInfo& info) { + std::string value = info[0].As().Utf8Value(); + BothBases key; + static_cast(key) = "unexpected-string-key"; + static_cast(key) = value; + return MaybeUnwrap(Symbol::For(info.Env(), key)); +} + +Symbol FetchSymbolFromGlobalRegistryWithViewAndNapiStringKey( + const Napi::CallbackInfo& info) { + Env env = info.Env(); + std::string value = info[0].As().Utf8Value(); + ViewAndNapiString key; + static_cast(key) = value; + static_cast(key) = + Napi::String::New(env, "unexpected-napi-string-key"); + return MaybeUnwrap(Symbol::For(env, key)); +} + +Symbol FetchSymbolFromGlobalRegistryWithStringReferenceKey( + const Napi::CallbackInfo& info) { + std::string value = info[0].As().Utf8Value(); + StringReferenceLike key{"unexpected-string-reference-key", value}; + return MaybeUnwrap(Symbol::For(info.Env(), key)); +} + +Symbol FetchSymbolFromGlobalRegistryWithImplicitViewKey( + const Napi::CallbackInfo& info) { + std::string value = info[0].As().Utf8Value(); + ImplicitAndExplicitStringViewLike key{value}; + return MaybeUnwrap(Symbol::For(info.Env(), key)); +} + Symbol FetchSymbolFromGlobalRegistryWithCKey(const Napi::CallbackInfo& info) { String cppStringKey = info[0].As(); return MaybeUnwrap( @@ -83,6 +187,20 @@ Object InitSymbol(Env env) { Function::New(env, FetchSymbolFromGlobalRegistryWithCppKey); exports["getSymbolFromGlobalRegistryWithStringViewKey"] = Function::New(env, FetchSymbolFromGlobalRegistryWithStringViewKey); + exports["getSymbolFromGlobalRegistryWithStringLikeKey"] = + Function::New(env, FetchSymbolFromGlobalRegistryWithStringLikeKey); + exports["getSymbolFromGlobalRegistryWithRvalueStringLikeKey"] = + Function::New(env, FetchSymbolFromGlobalRegistryWithRvalueStringLikeKey); + exports["getSymbolFromGlobalRegistryWithStringOnlyLikeKey"] = + Function::New(env, FetchSymbolFromGlobalRegistryWithStringOnlyLikeKey); + exports["getSymbolFromGlobalRegistryWithBothBasesKey"] = + Function::New(env, FetchSymbolFromGlobalRegistryWithBothBasesKey); + exports["getSymbolFromGlobalRegistryWithViewAndNapiStringKey"] = + Function::New(env, FetchSymbolFromGlobalRegistryWithViewAndNapiStringKey); + exports["getSymbolFromGlobalRegistryWithStringReferenceKey"] = + Function::New(env, FetchSymbolFromGlobalRegistryWithStringReferenceKey); + exports["getSymbolFromGlobalRegistryWithImplicitViewKey"] = + Function::New(env, FetchSymbolFromGlobalRegistryWithImplicitViewKey); exports["testUndefinedSymbolCanBeCreated"] = Function::New(env, TestUndefinedSymbolsCanBeCreated); exports["testNullSymbolCanBeCreated"] = diff --git a/test/symbol.js b/test/symbol.js index baf39c81b..6ecbb90d3 100644 --- a/test/symbol.js +++ b/test/symbol.js @@ -42,6 +42,7 @@ function test (binding) { const symbTwo = fetchFunction(symbol); assert(symbOne && symbTwo); assert(symbOne === symbTwo); + assert(symbOne === Symbol.for(symbol)); } assertCanCreateSymbol('testing'); @@ -55,6 +56,27 @@ function test (binding) { assertCanCreateOrFetchGlobalSymbols('data', binding.symbol.getSymbolFromGlobalRegistry); assertCanCreateOrFetchGlobalSymbols('CppKey', binding.symbol.getSymbolFromGlobalRegistryWithCppKey); assertCanCreateOrFetchGlobalSymbols('StringViewKey', binding.symbol.getSymbolFromGlobalRegistryWithStringViewKey); + assertCanCreateOrFetchGlobalSymbols( + 'StringLikeKey', + binding.symbol.getSymbolFromGlobalRegistryWithStringLikeKey); + assertCanCreateOrFetchGlobalSymbols( + 'RvalueStringLikeKey', + binding.symbol.getSymbolFromGlobalRegistryWithRvalueStringLikeKey); + assertCanCreateOrFetchGlobalSymbols( + 'StringOnlyLikeKey', + binding.symbol.getSymbolFromGlobalRegistryWithStringOnlyLikeKey); + assertCanCreateOrFetchGlobalSymbols( + 'BothBasesKey', + binding.symbol.getSymbolFromGlobalRegistryWithBothBasesKey); + assertCanCreateOrFetchGlobalSymbols( + 'ViewAndNapiStringKey', + binding.symbol.getSymbolFromGlobalRegistryWithViewAndNapiStringKey); + assertCanCreateOrFetchGlobalSymbols( + 'StringReferenceKey', + binding.symbol.getSymbolFromGlobalRegistryWithStringReferenceKey); + assertCanCreateOrFetchGlobalSymbols( + 'ImplicitViewKey', + binding.symbol.getSymbolFromGlobalRegistryWithImplicitViewKey); assertCanCreateOrFetchGlobalSymbols('CKey', binding.symbol.getSymbolFromGlobalRegistryWithCKey); assert(binding.symbol.createNewSymbolWithNoArgs() === undefined);