Skip to content
Permalink
Browse files
Drop StringImpl::createFromLiteral()
https://bugs.webkit.org/show_bug.cgi?id=239792

Reviewed by Darin Adler.

Drop StringImpl::createFromLiteral().

Call sites that have an ASCIILiteral can now simply call StringImpl::create(ASCIILiteral).
Call sites that have raw characters can call the existing StringImpl::createWithoutCopying().

This simplifies our API a bit.

Also inline part of the createWithoutCopying() functions so that the 0-length check is
inline. This allows the compiler to optimize the check out when the length is known at
compile time (which is often the case with literals).

* Tools/TestWebKitAPI/Tests/WTF/StringImpl.cpp:
(TestWebKitAPI::TEST):
* Tools/TestWebKitAPI/Tests/WTF/StringView.cpp:
(TestWebKitAPI::TEST):
* Source/JavaScriptCore/API/JSScriptRef.cpp:
* Source/JavaScriptCore/Scripts/wkbuiltins/builtins_templates.py:
* Source/JavaScriptCore/builtins/BuiltinExecutables.cpp:
(JSC::BuiltinExecutables::BuiltinExecutables):
* Source/JavaScriptCore/runtime/IntlObject.cpp:
(JSC::availableUnits):
* Source/WTF/wtf/text/StringImpl.cpp:
(WTF::StringImpl::createWithoutCopyingNonEmpty):
(WTF::StringImpl::createFromLiteral): Deleted.
(WTF::StringImpl::createWithoutCopying): Deleted.
* Source/WTF/wtf/text/StringImpl.h:
(WTF::StringImpl::create):
(WTF::StringImpl::createWithoutCopying):
(WTF::StringImpl::createFromLiteral): Deleted.
* Source/WTF/wtf/text/WTFString.h:
(WTF::String::String):

Canonical link: https://commits.webkit.org/250127@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@293621 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
cdumez committed Apr 29, 2022
1 parent 652c0e3 commit 85ec4b9a8cd237f968b01c4533d42a1fbb38681f
Show file tree
Hide file tree
Showing 9 changed files with 62 additions and 93 deletions.
@@ -92,7 +92,7 @@ JSScriptRef JSScriptCreateReferencingImmortalASCIIText(JSContextGroupRef context
startingLineNumber = std::max(1, startingLineNumber);

auto sourceURL = urlString ? URL({ }, urlString->string()) : URL();
auto result = OpaqueJSScript::create(vm, SourceOrigin { sourceURL }, sourceURL.string(), startingLineNumber, String(StringImpl::createFromLiteral(source, length)));
auto result = OpaqueJSScript::create(vm, SourceOrigin { sourceURL }, sourceURL.string(), startingLineNumber, String(StringImpl::createWithoutCopying(source, length)));

ParserError error;
if (!parseScript(vm, SourceCode(result.copyRef()), error)) {
@@ -132,7 +132,7 @@ class BuiltinsGeneratorTemplates:
explicit ${objectName}BuiltinsWrapper(JSC::VM& vm)
: m_vm(vm)
${macroPrefix}_FOREACH_${objectMacro}_BUILTIN_FUNCTION_NAME(INITIALIZE_BUILTIN_NAMES)
#define INITIALIZE_BUILTIN_SOURCE_MEMBERS(name, functionName, overriddenName, length) , m_##name##Source(JSC::makeSource(StringImpl::createFromLiteral(s_##name, length), { }))
#define INITIALIZE_BUILTIN_SOURCE_MEMBERS(name, functionName, overriddenName, length) , m_##name##Source(JSC::makeSource(StringImpl::createWithoutCopying(s_##name, length), { }))
${macroPrefix}_FOREACH_${objectMacro}_BUILTIN_CODE(INITIALIZE_BUILTIN_SOURCE_MEMBERS)
#undef INITIALIZE_BUILTIN_SOURCE_MEMBERS
{
@@ -36,7 +36,7 @@ namespace JSC {

BuiltinExecutables::BuiltinExecutables(VM& vm)
: m_vm(vm)
, m_combinedSourceProvider(StringSourceProvider::create(StringImpl::createFromLiteral(s_JSCCombinedCode, s_JSCCombinedCodeLength), { }, String()))
, m_combinedSourceProvider(StringSourceProvider::create(StringImpl::createWithoutCopying(s_JSCCombinedCode, s_JSCCombinedCodeLength), { }, String()))
{
}

@@ -1899,7 +1899,7 @@ static JSArray* availableUnits(JSGlobalObject* globalObject)

int32_t index = 0;
for (const MeasureUnit& unit : simpleUnits) {
result->putDirectIndex(globalObject, index++, jsString(vm, StringImpl::createFromLiteral(unit.subType)));
result->putDirectIndex(globalObject, index++, jsString(vm, StringImpl::create(unit.subType)));
RETURN_IF_EXCEPTION(scope, { });
}
return result;
@@ -151,29 +151,15 @@ void StringImpl::destroy(StringImpl* stringImpl)
StringImplMalloc::free(stringImpl);
}

Ref<StringImpl> StringImpl::createFromLiteral(const char* characters, unsigned length)
Ref<StringImpl> StringImpl::createWithoutCopyingNonEmpty(const UChar* characters, unsigned length)
{
ASSERT_WITH_MESSAGE(length, "Use StringImpl::empty() to create an empty string");
ASSERT(charactersAreAllASCII(reinterpret_cast<const LChar*>(characters), length));
return adoptRef(*new StringImpl(reinterpret_cast<const LChar*>(characters), length, ConstructWithoutCopying));
}

Ref<StringImpl> StringImpl::createFromLiteral(const char* characters)
{
return createFromLiteral(characters, strlen(characters));
}

Ref<StringImpl> StringImpl::createWithoutCopying(const UChar* characters, unsigned length)
{
if (!length)
return *empty();
ASSERT(length);
return adoptRef(*new StringImpl(characters, length, ConstructWithoutCopying));
}

Ref<StringImpl> StringImpl::createWithoutCopying(const LChar* characters, unsigned length)
Ref<StringImpl> StringImpl::createWithoutCopyingNonEmpty(const LChar* characters, unsigned length)
{
if (!length)
return *empty();
ASSERT(length);
return adoptRef(*new StringImpl(characters, length, ConstructWithoutCopying));
}

@@ -247,14 +247,11 @@ class StringImpl : private StringImplShape {

static Ref<StringImpl> createSubstringSharingImpl(StringImpl&, unsigned offset, unsigned length);

// FIXME: Replace calls to these overloads of createFromLiteral to createWithoutCopying instead.
WTF_EXPORT_PRIVATE static Ref<StringImpl> createFromLiteral(const char*, unsigned length);
WTF_EXPORT_PRIVATE static Ref<StringImpl> createFromLiteral(const char*);
static Ref<StringImpl> createFromLiteral(ASCIILiteral);

WTF_EXPORT_PRIVATE static Ref<StringImpl> createWithoutCopying(const UChar*, unsigned length);
WTF_EXPORT_PRIVATE static Ref<StringImpl> createWithoutCopying(const LChar*, unsigned length);
static Ref<StringImpl> createWithoutCopying(const char* characters, unsigned length) { return createWithoutCopying(reinterpret_cast<const LChar*>(characters), length); }
ALWAYS_INLINE static Ref<StringImpl> create(ASCIILiteral literal) { return createWithoutCopying(literal.characters8(), literal.length()); }

static Ref<StringImpl> createWithoutCopying(const UChar* characters, unsigned length) { return length ? createWithoutCopyingNonEmpty(characters, length) : Ref { *empty() }; }
static Ref<StringImpl> createWithoutCopying(const LChar* characters, unsigned length) { return length ? createWithoutCopyingNonEmpty(characters, length) : Ref { *empty() }; }
ALWAYS_INLINE static Ref<StringImpl> createWithoutCopying(const char* characters, unsigned length) { return createWithoutCopying(reinterpret_cast<const LChar*>(characters), length); }
WTF_EXPORT_PRIVATE static Ref<StringImpl> createUninitialized(unsigned length, LChar*&);
WTF_EXPORT_PRIVATE static Ref<StringImpl> createUninitialized(unsigned length, UChar*&);
template<typename CharacterType> static RefPtr<StringImpl> tryCreateUninitialized(unsigned length, CharacterType*&);
@@ -324,6 +321,9 @@ class StringImpl : private StringImplShape {
private:
static WTF_EXPORT_PRIVATE UTF8ConversionError utf8Impl(const UChar* characters, unsigned length, char*& buffer, size_t bufferSize, ConversionMode);

WTF_EXPORT_PRIVATE static Ref<StringImpl> createWithoutCopyingNonEmpty(const LChar*, unsigned length);
WTF_EXPORT_PRIVATE static Ref<StringImpl> createWithoutCopyingNonEmpty(const UChar*, unsigned length);

// The high bits of 'hash' are always empty, but we prefer to store our flags
// in the low bits because it makes them slightly more efficient to access.
// So, we shift left and right when setting and getting our hash code.
@@ -994,12 +994,6 @@ ALWAYS_INLINE Ref<StringImpl> StringImpl::createSubstringSharingImpl(StringImpl&
return adoptRef(*new (NotNull, stringImpl) StringImpl(rep.m_data16 + offset, length, *ownerRep));
}

inline Ref<StringImpl> StringImpl::createFromLiteral(ASCIILiteral literal)
{
auto length = literal.length();
return length ? createFromLiteral(literal.characters(), length) : Ref { *StringImpl::empty() };
}

template<typename CharacterType> ALWAYS_INLINE RefPtr<StringImpl> StringImpl::tryCreateUninitialized(unsigned length, CharacterType*& output)
{
if (!length) {
@@ -451,7 +451,7 @@ inline String::String(StaticStringImpl* string)
}

inline String::String(ASCIILiteral characters)
: m_impl(characters.isNull() ? nullptr : RefPtr<StringImpl> { StringImpl::createFromLiteral(characters) })
: m_impl(characters.isNull() ? nullptr : RefPtr<StringImpl> { StringImpl::create(characters) })
{
}

0 comments on commit 85ec4b9

Please sign in to comment.