Skip to content

Commit

Permalink
Make String::IsOneByteRepresentationUnderneath static
Browse files Browse the repository at this point in the history
and non-recursive in order to let Clang inline it.
Bonus: Drop IsTwoByteRepresentationUnderneath, which was dead code
except for one test, and is semantically redundant.

Bug: chromium:910573
Change-Id: I86f1c312e93ab875b4b42101ac65ddc94b1c9518
Reviewed-on: https://chromium-review.googlesource.com/c/1369086
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58171}
  • Loading branch information
jakobkummerow authored and Commit Bot committed Dec 11, 2018
1 parent ab5d90d commit 63d6b75
Show file tree
Hide file tree
Showing 10 changed files with 32 additions and 45 deletions.
2 changes: 1 addition & 1 deletion src/builtins/builtins-string.cc
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ V8_WARN_UNUSED_RESULT static Object* ConvertCase(
// character is also ASCII. This is currently the case, but it
// might break in the future if we implement more context and locale
// dependent upper/lower conversions.
if (s->IsOneByteRepresentationUnderneath()) {
if (String::IsOneByteRepresentationUnderneath(*s)) {
// Same length as input.
Handle<SeqOneByteString> result =
isolate->factory()->NewRawOneByteString(length).ToHandleChecked();
Expand Down
2 changes: 1 addition & 1 deletion src/conversions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ class StringToIntHelper {

bool IsOneByte() const {
return raw_one_byte_subject_ != nullptr ||
subject_->IsOneByteRepresentationUnderneath();
String::IsOneByteRepresentationUnderneath(*subject_);
}

Vector<const uint8_t> GetOneByteVector() {
Expand Down
4 changes: 2 additions & 2 deletions src/json-stringifier.cc
Original file line number Diff line number Diff line change
Expand Up @@ -901,14 +901,14 @@ void JsonStringifier::SerializeDeferredKey(bool deferred_comma,
void JsonStringifier::SerializeString(Handle<String> object) {
object = String::Flatten(isolate_, object);
if (builder_.CurrentEncoding() == String::ONE_BYTE_ENCODING) {
if (object->IsOneByteRepresentationUnderneath()) {
if (String::IsOneByteRepresentationUnderneath(*object)) {
SerializeString_<uint8_t, uint8_t>(object);
} else {
builder_.ChangeEncoding();
SerializeString(object);
}
} else {
if (object->IsOneByteRepresentationUnderneath()) {
if (String::IsOneByteRepresentationUnderneath(*object)) {
SerializeString_<uint8_t, uc16>(object);
} else {
SerializeString_<uc16, uc16>(object);
Expand Down
2 changes: 1 addition & 1 deletion src/objects.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16725,7 +16725,7 @@ MaybeHandle<String> EscapeRegExpSource(Isolate* isolate,
Handle<String> source) {
DCHECK(source->IsFlat());
if (source->length() == 0) return isolate->factory()->query_colon_string();
bool one_byte = source->IsOneByteRepresentationUnderneath();
bool one_byte = String::IsOneByteRepresentationUnderneath(*source);
int escapes = one_byte ? CountRequiredEscapes<uint8_t>(source)
: CountRequiredEscapes<uc16>(source);
if (escapes == 0) return source;
Expand Down
41 changes: 14 additions & 27 deletions src/objects/string-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -162,33 +162,20 @@ bool String::IsTwoByteRepresentation() const {
return (type & kStringEncodingMask) == kTwoByteStringTag;
}

bool String::IsOneByteRepresentationUnderneath() {
uint32_t type = map()->instance_type();
STATIC_ASSERT(kIsIndirectStringTag != 0);
STATIC_ASSERT((kIsIndirectStringMask & kStringEncodingMask) == 0);
DCHECK(IsFlat());
switch (type & (kIsIndirectStringMask | kStringEncodingMask)) {
case kOneByteStringTag:
return true;
case kTwoByteStringTag:
return false;
default: // Cons, sliced, thin, strings need to go deeper.
return GetUnderlying()->IsOneByteRepresentationUnderneath();
}
}

bool String::IsTwoByteRepresentationUnderneath() {
uint32_t type = map()->instance_type();
STATIC_ASSERT(kIsIndirectStringTag != 0);
STATIC_ASSERT((kIsIndirectStringMask & kStringEncodingMask) == 0);
DCHECK(IsFlat());
switch (type & (kIsIndirectStringMask | kStringEncodingMask)) {
case kOneByteStringTag:
return false;
case kTwoByteStringTag:
return true;
default: // Cons, sliced, thin, strings need to go deeper.
return GetUnderlying()->IsTwoByteRepresentationUnderneath();
bool String::IsOneByteRepresentationUnderneath(String string) {
while (true) {
uint32_t type = string.map()->instance_type();
STATIC_ASSERT(kIsIndirectStringTag != 0);
STATIC_ASSERT((kIsIndirectStringMask & kStringEncodingMask) == 0);
DCHECK(string.IsFlat());
switch (type & (kIsIndirectStringMask | kStringEncodingMask)) {
case kOneByteStringTag:
return true;
case kTwoByteStringTag:
return false;
default: // Cons, sliced, thin, strings need to go deeper.
string = string.GetUnderlying();
}
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/objects/string.h
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,9 @@ class String : public Name {

// Cons and slices have an encoding flag that may not represent the actual
// encoding of the underlying string. This is taken into account here.
// Requires: this->IsFlat()
inline bool IsOneByteRepresentationUnderneath();
inline bool IsTwoByteRepresentationUnderneath();
// This function is static because that helps it get inlined.
// Requires: string.IsFlat()
static inline bool IsOneByteRepresentationUnderneath(String string);

// NOTE: this should be considered only a hint. False negatives are
// possible.
Expand Down
6 changes: 3 additions & 3 deletions src/regexp/jsregexp.cc
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ int RegExpImpl::IrregexpPrepare(Isolate* isolate, Handle<JSRegExp> regexp,
DCHECK(subject->IsFlat());

// Check representation of the underlying storage.
bool is_one_byte = subject->IsOneByteRepresentationUnderneath();
bool is_one_byte = String::IsOneByteRepresentationUnderneath(*subject);
if (!EnsureCompiledIrregexp(isolate, regexp, subject, is_one_byte)) return -1;

#ifdef V8_INTERPRETED_REGEXP
Expand All @@ -436,7 +436,7 @@ int RegExpImpl::IrregexpExecRaw(Isolate* isolate, Handle<JSRegExp> regexp,
DCHECK_LE(index, subject->length());
DCHECK(subject->IsFlat());

bool is_one_byte = subject->IsOneByteRepresentationUnderneath();
bool is_one_byte = String::IsOneByteRepresentationUnderneath(*subject);

#ifndef V8_INTERPRETED_REGEXP
DCHECK(output_size >= (IrregexpNumberOfCaptures(*irregexp) + 1) * 2);
Expand Down Expand Up @@ -472,7 +472,7 @@ int RegExpImpl::IrregexpExecRaw(Isolate* isolate, Handle<JSRegExp> regexp,
// being internal and external, and even between being Latin1 and UC16,
// but the characters are always the same).
IrregexpPrepare(isolate, regexp, subject);
is_one_byte = subject->IsOneByteRepresentationUnderneath();
is_one_byte = String::IsOneByteRepresentationUnderneath(*subject);
} while (true);
UNREACHABLE();
#else // V8_INTERPRETED_REGEXP
Expand Down
5 changes: 3 additions & 2 deletions src/regexp/regexp-macro-assembler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ int NativeRegExpMacroAssembler::CheckStackGuardState(
HandleScope handles(isolate);
Handle<Code> code_handle(re_code, isolate);
Handle<String> subject_handle(String::cast(ObjectPtr(*subject)), isolate);
bool is_one_byte = subject_handle->IsOneByteRepresentationUnderneath();
bool is_one_byte = String::IsOneByteRepresentationUnderneath(*subject_handle);

StackLimitCheck check(isolate);
bool js_has_overflowed = check.JsHasOverflowed();
Expand Down Expand Up @@ -192,7 +192,8 @@ int NativeRegExpMacroAssembler::CheckStackGuardState(
// If we continue, we need to update the subject string addresses.
if (return_value == 0) {
// String encoding might have changed.
if (subject_handle->IsOneByteRepresentationUnderneath() != is_one_byte) {
if (String::IsOneByteRepresentationUnderneath(*subject_handle) !=
is_one_byte) {
// If we changed between an LATIN1 and an UC16 string, the specialized
// code cannot be used, and we need to restart regexp matching from
// scratch (including, potentially, compiling a new version of the code).
Expand Down
4 changes: 2 additions & 2 deletions src/uri.cc
Original file line number Diff line number Diff line change
Expand Up @@ -493,15 +493,15 @@ static MaybeHandle<String> EscapePrivate(Isolate* isolate,
MaybeHandle<String> Uri::Escape(Isolate* isolate, Handle<String> string) {
Handle<String> result;
string = String::Flatten(isolate, string);
return string->IsOneByteRepresentationUnderneath()
return String::IsOneByteRepresentationUnderneath(*string)
? EscapePrivate<uint8_t>(isolate, string)
: EscapePrivate<uc16>(isolate, string);
}

MaybeHandle<String> Uri::Unescape(Isolate* isolate, Handle<String> string) {
Handle<String> result;
string = String::Flatten(isolate, string);
return string->IsOneByteRepresentationUnderneath()
return String::IsOneByteRepresentationUnderneath(*string)
? UnescapePrivate<uint8_t>(isolate, string)
: UnescapePrivate<uc16>(isolate, string);
}
Expand Down
5 changes: 2 additions & 3 deletions test/cctest/test-strings.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1832,9 +1832,8 @@ TEST(Regress876759) {
CHECK(grandparent->IsOneByteRepresentation());
CHECK(parent->IsTwoByteRepresentation());
CHECK(sliced->IsTwoByteRepresentation());
// The *Underneath versions return the correct representation.
CHECK(sliced->IsOneByteRepresentationUnderneath());
CHECK(!sliced->IsTwoByteRepresentationUnderneath());
// The *Underneath version returns the correct representation.
CHECK(String::IsOneByteRepresentationUnderneath(*sliced));
}

} // namespace test_strings
Expand Down

0 comments on commit 63d6b75

Please sign in to comment.