From 9af66ffadd7988ce3b2073a071b29a35060a8f90 Mon Sep 17 00:00:00 2001 From: umuoy1 Date: Sat, 8 Aug 2026 03:03:47 +0800 Subject: [PATCH 1/4] fix: resolve Symbol::For overload ambiguity Signed-off-by: umuoy1 --- doc/symbol.md | 5 ++--- napi-inl.h | 6 ------ napi.h | 3 --- test/symbol.cc | 23 +++++++++++++++++++++++ test/symbol.js | 1 + 5 files changed, 26 insertions(+), 12 deletions(-) diff --git a/doc/symbol.md b/doc/symbol.md index 4fa1ae0da..1269cd7a1 100644 --- a/doc/symbol.md +++ b/doc/symbol.md @@ -49,7 +49,6 @@ Returns a `Napi::Symbol` representing a well-known `Symbol` from the ### For ```cpp -static Napi::Symbol Napi::Symbol::For(napi_env env, const std::string& description); static Napi::Symbol Napi::Symbol::For(napi_env env, std::string_view description); static Napi::Symbol Napi::Symbol::For(napi_env env, const char* description); static Napi::Symbol Napi::Symbol::For(napi_env env, String description); @@ -59,8 +58,8 @@ 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. + - `std::string_view` - represents a UTF-8 string view. `std::string` values + are implicitly convertible to `std::string_view`. - `const char*` - represents a UTF8 string description. - `String` - Node addon API String description. - `napi_value` - Node-API `napi_value` description. diff --git a/napi-inl.h b/napi-inl.h index ec63ffeee..04dde9959 100644 --- a/napi-inl.h +++ b/napi-inl.h @@ -1416,12 +1416,6 @@ inline MaybeOrValue Symbol::WellKnown(napi_env env, #endif } -inline MaybeOrValue Symbol::For(napi_env env, - const std::string& description) { - napi_value descriptionValue = String::New(env, description); - return Symbol::For(env, descriptionValue); -} - inline MaybeOrValue Symbol::For(napi_env env, std::string_view description) { napi_value descriptionValue = String::New(env, description); diff --git a/napi.h b/napi.h index 870a5c290..75d11fb12 100644 --- a/napi.h +++ b/napi.h @@ -825,9 +825,6 @@ class Symbol : public Name { /// Get a public Symbol (e.g. Symbol.iterator). static MaybeOrValue WellKnown(napi_env, const std::string& name); - // Create a symbol in the global registry, UTF-8 Encoded cpp string - static MaybeOrValue For(napi_env env, const std::string& description); - // Create a symbol in the global registry, UTF-8 encoded cpp string view static MaybeOrValue For(napi_env env, std::string_view description); diff --git a/test/symbol.cc b/test/symbol.cc index d978739ff..856da4611 100644 --- a/test/symbol.cc +++ b/test/symbol.cc @@ -5,6 +5,21 @@ #include "test_helper.h" using namespace Napi; +namespace { + +class StringLike { + public: + explicit StringLike(const std::string& value) : _value(value) {} + + operator std::string() const { return _value; } + operator std::string_view() const { return _value; } + + private: + std::string _value; +}; + +} // namespace + Symbol CreateNewSymbolWithNoArgs(const Napi::CallbackInfo&) { return Napi::Symbol(); } @@ -47,6 +62,12 @@ 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 FetchSymbolFromGlobalRegistryWithCKey(const Napi::CallbackInfo& info) { String cppStringKey = info[0].As(); return MaybeUnwrap( @@ -83,6 +104,8 @@ Object InitSymbol(Env env) { Function::New(env, FetchSymbolFromGlobalRegistryWithCppKey); exports["getSymbolFromGlobalRegistryWithStringViewKey"] = Function::New(env, FetchSymbolFromGlobalRegistryWithStringViewKey); + exports["getSymbolFromGlobalRegistryWithStringLikeKey"] = + Function::New(env, FetchSymbolFromGlobalRegistryWithStringLikeKey); exports["testUndefinedSymbolCanBeCreated"] = Function::New(env, TestUndefinedSymbolsCanBeCreated); exports["testNullSymbolCanBeCreated"] = diff --git a/test/symbol.js b/test/symbol.js index baf39c81b..4994698fd 100644 --- a/test/symbol.js +++ b/test/symbol.js @@ -55,6 +55,7 @@ function test (binding) { assertCanCreateOrFetchGlobalSymbols('data', binding.symbol.getSymbolFromGlobalRegistry); assertCanCreateOrFetchGlobalSymbols('CppKey', binding.symbol.getSymbolFromGlobalRegistryWithCppKey); assertCanCreateOrFetchGlobalSymbols('StringViewKey', binding.symbol.getSymbolFromGlobalRegistryWithStringViewKey); + assertCanCreateOrFetchGlobalSymbols('StringLikeKey', binding.symbol.getSymbolFromGlobalRegistryWithStringLikeKey); assertCanCreateOrFetchGlobalSymbols('CKey', binding.symbol.getSymbolFromGlobalRegistryWithCKey); assert(binding.symbol.createNewSymbolWithNoArgs() === undefined); From 2e92c98fe4192115f6cc0661567dd3cc3a2d66f2 Mon Sep 17 00:00:00 2001 From: umuoy1 Date: Sun, 9 Aug 2026 15:51:03 +0800 Subject: [PATCH 2/4] fix: refine Symbol::For overload resolution Signed-off-by: umuoy1 --- doc/symbol.md | 9 +++- napi-inl.h | 12 +++++ napi.h | 45 +++++++++++++++++++ test/symbol.cc | 117 +++++++++++++++++++++++++++++++++++++++++++++---- test/symbol.js | 23 +++++++++- 5 files changed, 195 insertions(+), 11 deletions(-) diff --git a/doc/symbol.md b/doc/symbol.md index 1269cd7a1..8ed71bbaa 100644 --- a/doc/symbol.md +++ b/doc/symbol.md @@ -49,6 +49,7 @@ Returns a `Napi::Symbol` representing a well-known `Symbol` from the ### For ```cpp +static Napi::Symbol Napi::Symbol::For(napi_env env, const std::string& description); static Napi::Symbol Napi::Symbol::For(napi_env env, std::string_view description); static Napi::Symbol Napi::Symbol::For(napi_env env, const char* description); static Napi::Symbol Napi::Symbol::For(napi_env env, String description); @@ -58,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: - - `std::string_view` - represents a UTF-8 string view. `std::string` values - are implicitly convertible to `std::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 04dde9959..d81257518 100644 --- a/napi-inl.h +++ b/napi-inl.h @@ -1416,12 +1416,24 @@ inline MaybeOrValue Symbol::WellKnown(napi_env env, #endif } +inline MaybeOrValue Symbol::For(napi_env env, + const std::string& description) { + napi_value descriptionValue = String::New(env, description); + return Symbol::For(env, descriptionValue); +} + inline MaybeOrValue Symbol::For(napi_env env, std::string_view description) { napi_value descriptionValue = String::New(env, description); 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 75d11fb12..77159638f 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 symbol_for_overload_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: @@ -825,9 +862,17 @@ class Symbol : public Name { /// Get a public Symbol (e.g. Symbol.iterator). static MaybeOrValue WellKnown(napi_env, const std::string& name); + // Create a symbol in the global registry, UTF-8 Encoded cpp string + static MaybeOrValue For(napi_env env, const std::string& description); + // 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 856da4611..9277c3e9d 100644 --- a/test/symbol.cc +++ b/test/symbol.cc @@ -1,21 +1,61 @@ #include #include +#include #include "test_helper.h" using namespace Napi; namespace { -class StringLike { - public: - explicit StringLike(const std::string& value) : _value(value) {} +struct StringLike { + operator std::string() const { return "unexpected-string-key"; } + operator std::string_view() const { return value; } - operator std::string() const { return _value; } - 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; } - private: - std::string _value; + mutable std::string stringValue; + std::string_view viewValue; +}; + +struct ImplicitStringViewLike { + operator std::string_view() const { return value; } + + std::string_view value; +}; + +struct ExplicitStringViewLike { + explicit operator std::string_view() const { return value; } + + std::string_view value; +}; + +struct ImplicitAndExplicitStringViewLike : ImplicitStringViewLike, + ExplicitStringViewLike { + operator std::string() const { return "unexpected-string-key"; } }; } // namespace @@ -64,10 +104,59 @@ Symbol FetchSymbolFromGlobalRegistryWithStringViewKey( Symbol FetchSymbolFromGlobalRegistryWithStringLikeKey( const Napi::CallbackInfo& info) { - StringLike key(info[0].As().Utf8Value()); + 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; + static_cast(key).value = value; + static_cast(key).value = + "unexpected-explicit-string-view-key"; + return MaybeUnwrap(Symbol::For(info.Env(), key)); +} + Symbol FetchSymbolFromGlobalRegistryWithCKey(const Napi::CallbackInfo& info) { String cppStringKey = info[0].As(); return MaybeUnwrap( @@ -106,6 +195,18 @@ Object InitSymbol(Env env) { 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 4994698fd..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,7 +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( + '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); From 500888083c2c097d9256ffc6ff62e82de3d2475e Mon Sep 17 00:00:00 2001 From: umuoy1 Date: Sun, 9 Aug 2026 17:29:02 +0800 Subject: [PATCH 3/4] test: refine Symbol::For conversion coverage Signed-off-by: umuoy1 --- test/symbol.cc | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/test/symbol.cc b/test/symbol.cc index 9277c3e9d..e7c8b7447 100644 --- a/test/symbol.cc +++ b/test/symbol.cc @@ -41,23 +41,20 @@ struct StringReferenceLike { std::string_view viewValue; }; -struct ImplicitStringViewLike { - operator std::string_view() const { return value; } +struct ImplicitAndExplicitStringViewLike { + operator std::string() const { return "unexpected-string-key"; } - std::string_view value; -}; + // 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; } -struct ExplicitStringViewLike { - explicit operator std::string_view() const { return value; } + explicit operator std::string_view() & { + return "unexpected-explicit-string-view-key"; + } std::string_view value; }; -struct ImplicitAndExplicitStringViewLike : ImplicitStringViewLike, - ExplicitStringViewLike { - operator std::string() const { return "unexpected-string-key"; } -}; - } // namespace Symbol CreateNewSymbolWithNoArgs(const Napi::CallbackInfo&) { @@ -150,10 +147,7 @@ Symbol FetchSymbolFromGlobalRegistryWithStringReferenceKey( Symbol FetchSymbolFromGlobalRegistryWithImplicitViewKey( const Napi::CallbackInfo& info) { std::string value = info[0].As().Utf8Value(); - ImplicitAndExplicitStringViewLike key; - static_cast(key).value = value; - static_cast(key).value = - "unexpected-explicit-string-view-key"; + ImplicitAndExplicitStringViewLike key{value}; return MaybeUnwrap(Symbol::For(info.Env(), key)); } From 20ef69513be81d9af504ac0850adbe4771be31b1 Mon Sep 17 00:00:00 2001 From: umuoy1 Date: Mon, 10 Aug 2026 01:13:28 +0800 Subject: [PATCH 4/4] refactor: rename string conversion probe Signed-off-by: umuoy1 --- napi.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/napi.h b/napi.h index 77159638f..e3ef19e5c 100644 --- a/napi.h +++ b/napi.h @@ -791,7 +791,7 @@ class String : public Name { namespace details { // This overload set must mirror the non-template Symbol::For overloads. -struct symbol_for_overload_probe { +struct string_convertible_probe { static void select(const std::string&); static void select(std::string_view); static void select(const char*); @@ -805,7 +805,7 @@ struct has_unambiguous_symbol_for_overload : std::false_type {}; template struct has_unambiguous_symbol_for_overload< T, - std::void_t()))>> + std::void_t()))>> : std::true_type {}; // Enable the template overload only for string-like arguments that have no