From 5ff4fb286ce3557a811fbaaaf0600f7cab1a435c Mon Sep 17 00:00:00 2001 From: Florin Malita Date: Thu, 14 Jun 2018 15:16:40 +0000 Subject: Revert "[skjson] Implementation/API tweaks" This reverts commit 03b68421cafa58428edd8ab12e0d2235c7000c3f. Reason for revert: Broke Debian9 builds Original change's description: > [skjson] Implementation/API tweaks > > * move most common accessor methods to the header, for inlining > * drop the lazy type checking semantics in favor of explicit guarded/unguarded > conversions > * revisit the public class hierarchy to better constrain type-bound APIs > * expose public type factories and add tests > * drop the empty-vector optimization -- allocating an external size_t in these > uncommon cases is better than paying for a conditional on every access. > > Change-Id: I24a7c75db3aa8b12c740c77ac7df4af4e3a1dff8 > Reviewed-on: https://skia-review.googlesource.com/134610 > Commit-Queue: Florin Malita > Reviewed-by: Mike Klein TBR=mtklein@google.com,fmalita@chromium.org Change-Id: I2c681ef5c8d5fc15508e58b4b0f6ab9491b7d76f No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://skia-review.googlesource.com/134880 Reviewed-by: Florin Malita Commit-Queue: Florin Malita --- modules/skjson/include/SkJSON.h | 279 +++------------------- modules/skjson/src/SkJSON.cpp | 480 +++++++++++++++++++++++++++----------- modules/skjson/src/SkJSONTest.cpp | 128 ++-------- 3 files changed, 392 insertions(+), 495 deletions(-) (limited to 'modules') diff --git a/modules/skjson/include/SkJSON.h b/modules/skjson/include/SkJSON.h index 8e4231e7a7..4cb312c64f 100644 --- a/modules/skjson/include/SkJSON.h +++ b/modules/skjson/include/SkJSON.h @@ -11,7 +11,6 @@ #include "SkArenaAlloc.h" #include "SkTypes.h" -class SkString; class SkWStream; namespace skjson { @@ -28,26 +27,12 @@ namespace skjson { * * Values are opaque, fixed-size (64 bits), immutable records. * - * They can be converted to facade types for type-specific functionality. + * They can be freely converted to any of the facade types for type-specific functionality. * - * E.g.: + * Note: type checking is lazy/deferred, to facilitate chained property access - e.g. * - * if (v.is()) { - * for (const auto& item : v.as()) { - * if (const NumberValue* n = item) { - * printf("Found number: %f", **n); - * } - * } - * } - * - * if (v.is()) { - * const StringValue* id = v.as()["id"]; - * if (id) { - * printf("Found object ID: %s", id->begin()); - * } else { - * printf("Missing object ID"); - * } - * } + * if (!v.as()["foo"].as()["bar"].is()) + * LOG("found v.foo.bar!"); */ class alignas(8) Value { public: @@ -61,7 +46,7 @@ public: }; /** - * @return The type of this value. + * @return The public type of this record. */ Type getType() const; @@ -69,289 +54,85 @@ public: * @return True if the record matches the facade type T. */ template - bool is() const { return this->getType() == T::kType; } + bool is() const { return T::IsType(this->getType()); } /** - * Unguarded conversion to facade types. + * @return The record cast as facade type T. * - * @return The record cast as facade type T&. + * Note: this is always safe, as proper typing is enforced in the facade methods. */ template const T& as() const { - SkASSERT(this->is()); - return *reinterpret_cast(this); + return *reinterpret_cast(this->is() ? this : &Value::Null()); } /** - * Guarded conversion to facade types. - * - * @return The record cast as facade type T*. + * @return Null value singleton. */ - template - operator const T*() const { - return this->is() ? &this->as() : nullptr; - } - - /** - * @return The string representation of this value. - */ - SkString toString() const; + static const Value& Null(); protected: - /* - Value implementation notes: - - -- fixed 64-bit size - - -- 8-byte aligned - - -- union of: - - bool - int32 - float - char[8] (short string storage) - external payload (tagged) pointer - - -- highest 3 bits reserved for type storage - - */ - enum class Tag : uint8_t { - // We picked kShortString == 0 so that tag 0x00 and stored max_size-size (7-7=0) - // conveniently overlap the '\0' terminator, allowing us to store a 7 character - // C string inline. - kShortString = 0b00000000, // inline payload - kNull = 0b00100000, // no payload - kBool = 0b01000000, // inline payload - kInt = 0b01100000, // inline payload - kFloat = 0b10000000, // inline payload - kString = 0b10100000, // ptr to external storage - kArray = 0b11000000, // ptr to external storage - kObject = 0b11100000, // ptr to external storage - }; - static constexpr uint8_t kTagMask = 0b11100000; - - void init_tagged(Tag); - void init_tagged_pointer(Tag, void*); - - Tag getTag() const { - return static_cast(fData8[kTagOffset] & kTagMask); - } - - // Access the record data as T. - // - // This is also used to access the payload for inline records. Since the record type lives in - // the high bits, sizeof(T) must be less than sizeof(Value) when accessing inline payloads. - // - // E.g. - // - // uint8_t - // ----------------------------------------------------------------------- - // | val8 | val8 | val8 | val8 | val8 | val8 | val8 | TYPE| - // ----------------------------------------------------------------------- - // - // uint32_t - // ----------------------------------------------------------------------- - // | val32 | unused | TYPE| - // ----------------------------------------------------------------------- - // - // T* (64b) - // ----------------------------------------------------------------------- - // | T* (kTypeShift bits) |TYPE| - // ----------------------------------------------------------------------- - // - template - const T* cast() const { - static_assert(sizeof (T) <= sizeof(Value), ""); - static_assert(alignof(T) <= alignof(Value), ""); - return reinterpret_cast(this); - } - - template - T* cast() { return const_cast(const_cast(this)->cast()); } - - // Access the pointer payload. - template - const T* ptr() const { - static_assert(sizeof(uintptr_t) == sizeof(Value) || - sizeof(uintptr_t) * 2 == sizeof(Value), ""); - - return (sizeof(uintptr_t) < sizeof(Value)) - // For 32-bit, pointers are stored unmodified. - ? *this->cast() - // For 64-bit, we use the high bits of the pointer as tag storage. - : reinterpret_cast(*this->cast() & kTagPointerMask); - } - -private: - static constexpr size_t kValueSize = 8; - - uint8_t fData8[kValueSize]; - -#if defined(SK_CPU_LENDIAN) - static constexpr size_t kTagOffset = kValueSize - 1; - - static constexpr uintptr_t kTagPointerMask = - ~(static_cast(kTagMask) << ((sizeof(uintptr_t) - 1) * 8)); -#else - // The current value layout assumes LE and will take some tweaking for BE. - static_assert(false, "Big-endian builds are not supported at this time."); -#endif + uint8_t fData8[8]; }; class NullValue final : public Value { public: - static constexpr Type kType = Type::kNull; - - NullValue(); + static bool IsType(Value::Type t) { return t == Type::kNull; } }; -class BoolValue final : public Value { -public: - static constexpr Type kType = Type::kBool; - - explicit BoolValue(bool); - - bool operator *() const { - SkASSERT(this->getTag() == Tag::kBool); - return *this->cast(); - } -}; - -class NumberValue final : public Value { +template +class PrimitiveValue final : public Value { public: - static constexpr Type kType = Type::kNumber; + static bool IsType(Value::Type t) { return t == vtype; } - explicit NumberValue(int32_t); - explicit NumberValue(float); - - double operator *() const { - SkASSERT(this->getTag() == Tag::kInt || - this->getTag() == Tag::kFloat); - - return this->getTag() == Tag::kInt - ? static_cast(*this->cast()) - : static_cast(*this->cast()); - } + T operator *() const; }; template class VectorValue : public Value { public: - using ValueT = T; - static constexpr Type kType = vtype; + static bool IsType(Value::Type t) { return t == vtype; } - size_t size() const { - SkASSERT(this->getType() == kType); - return *this->ptr(); - } + size_t size() const; - const T* begin() const { - SkASSERT(this->getType() == kType); - const auto* size_ptr = this->ptr(); - return reinterpret_cast(size_ptr + 1); - } - - const T* end() const { - SkASSERT(this->getType() == kType); - const auto* size_ptr = this->ptr(); - return reinterpret_cast(size_ptr + 1) + *size_ptr; - } + const T* begin() const; + const T* end() const; const T& operator[](size_t i) const { - SkASSERT(this->getType() == kType); - SkASSERT(i < this->size()); - - return *(this->begin() + i); + return (i < this->size()) ? *(this->begin() + i) : T::Null(); } }; -class ArrayValue final : public VectorValue { -public: - ArrayValue(const Value* src, size_t size, SkArenaAlloc& alloc); -}; - -class StringValue final : public Value { -public: - static constexpr Type kType = Type::kString; - - StringValue(); - StringValue(const char* src, size_t size, SkArenaAlloc& alloc); - - size_t size() const { - switch (this->getTag()) { - case Tag::kShortString: - return kMaxInlineStringSize - SkToSizeT(this->cast()[kMaxInlineStringSize]); - case Tag::kString: - return this->cast>()->size(); - default: - return 0; - } - } - - const char* begin() const { - return this->getTag() == Tag::kShortString - ? this->cast() - : this->cast>()->begin(); - } - - const char* end() const { - if (this->getTag() == Tag::kShortString) { - const auto* payload = this->cast(); - return payload + kMaxInlineStringSize - SkToSizeT(payload[kMaxInlineStringSize]); - } - return this->cast>()->end(); - } - -private: - static constexpr size_t kMaxInlineStringSize = sizeof(Value) - 1; -}; +using BoolValue = PrimitiveValue; +using NumberValue = PrimitiveValue; +using StringValue = VectorValue; +using ArrayValue = VectorValue; struct Member { StringValue fKey; Value fValue; + + static const Member& Null(); }; class ObjectValue final : public VectorValue { public: - ObjectValue(const Member* src, size_t size, SkArenaAlloc& alloc); - const Value& operator[](const char*) const; - -private: - // Not particularly interesting - hiding for disambiguation. - const Member& operator[](size_t i) const = delete; }; class DOM final : public SkNoncopyable { public: explicit DOM(const char*); - const Value& root() const { return fRoot; } + const Value& root() const { return *fRoot; } void write(SkWStream*) const; private: SkArenaAlloc fAlloc; - Value fRoot; + const Value* fRoot; }; -inline Value::Type Value::getType() const { - switch (this->getTag()) { - case Tag::kNull: return Type::kNull; - case Tag::kBool: return Type::kBool; - case Tag::kInt: return Type::kNumber; - case Tag::kFloat: return Type::kNumber; - case Tag::kShortString: return Type::kString; - case Tag::kString: return Type::kString; - case Tag::kArray: return Type::kArray; - case Tag::kObject: return Type::kObject; - } - - SkASSERT(false); // unreachable - return Type::kNull; -} - } // namespace skjson #endif // SkJSON_DEFINED diff --git a/modules/skjson/src/SkJSON.cpp b/modules/skjson/src/SkJSON.cpp index 05bf04d8f0..8af85ba695 100644 --- a/modules/skjson/src/SkJSON.cpp +++ b/modules/skjson/src/SkJSON.cpp @@ -18,130 +18,356 @@ namespace skjson { //#define SK_JSON_REPORT_ERRORS +namespace { + +/* + Value's impl side: + + -- fixed 64-bit size + + -- 8-byte aligned + + -- union of: + + bool + int32 + float + char[8] (short string storage) + external payload pointer + -- highest 3 bits reserved for type storage + + */ static_assert( sizeof(Value) == 8, ""); static_assert(alignof(Value) == 8, ""); static constexpr size_t kRecAlign = alignof(Value); -void Value::init_tagged(Tag t) { - memset(fData8, 0, sizeof(fData8)); - fData8[Value::kTagOffset] = SkTo(t); - SkASSERT(this->getTag() == t); -} +// The current record layout assumes LE and will take some tweaking for BE. +#if defined(SK_CPU_BENDIAN) +static_assert(false, "Big-endian builds are not supported."); +#endif + +class ValueRec : public Value { +public: + static constexpr uint64_t kTypeBits = 3, + kTypeShift = 64 - kTypeBits, + kTypeMask = ((1ULL << kTypeBits) - 1) << kTypeShift; + + enum RecType : uint64_t { + // We picked kShortString == 0 so that tag 0b000 and stored max_size-size (7-7=0) + // conveniently overlap the '\0' terminator, allowing us to store a 7 character + // C string inline. + kShortString = 0b000ULL << kTypeShift, // inline payload + kNull = 0b001ULL << kTypeShift, // no payload + kBool = 0b010ULL << kTypeShift, // inline payload + kInt = 0b011ULL << kTypeShift, // inline payload + kFloat = 0b100ULL << kTypeShift, // inline payload + kString = 0b101ULL << kTypeShift, // ptr to external storage + kArray = 0b110ULL << kTypeShift, // ptr to external storage + kObject = 0b111ULL << kTypeShift, // ptr to external storage + }; -// Pointer values store a type (in the upper kTagBits bits) and a pointer. -void Value::init_tagged_pointer(Tag t, void* p) { - *this->cast() = reinterpret_cast(p); + RecType getRecType() const { + return static_cast(*this->cast() & kTypeMask); + } - if (sizeof(Value) == sizeof(uintptr_t)) { - // For 64-bit, we rely on the pointer upper bits being unused/zero. - SkASSERT(!(fData8[kTagOffset] & kTagMask)); - fData8[kTagOffset] |= SkTo(t); - } else { - // For 32-bit, we need to zero-initialize the upper 32 bits - SkASSERT(sizeof(Value) == sizeof(uintptr_t) * 2); - this->cast()[kTagOffset >> 2] = 0; - fData8[kTagOffset] = SkTo(t); + // Access the record data as T. + // + // This is also used to access the payload for inline records. Since the record type lives in + // the high bits, sizeof(T) must be less than sizeof(Value) when accessing inline payloads. + // + // E.g. + // + // uint8_t + // ----------------------------------------------------------------------- + // | val8 | val8 | val8 | val8 | val8 | val8 | val8 | TYPE| + // ----------------------------------------------------------------------- + // + // uint32_t + // ----------------------------------------------------------------------- + // | val32 | unused | TYPE| + // ----------------------------------------------------------------------- + // + // T* (64b) + // ----------------------------------------------------------------------- + // | T* (kTypeShift bits) |TYPE| + // ----------------------------------------------------------------------- + // + template + const T* cast() const { + static_assert(sizeof (T) <= sizeof(ValueRec), ""); + static_assert(alignof(T) <= alignof(ValueRec), ""); + return reinterpret_cast(this); } - SkASSERT(this->getTag() == t); - SkASSERT(this->ptr() == p); -} + template + T* cast() { return const_cast(const_cast(this)->cast()); } -NullValue::NullValue() { - this->init_tagged(Tag::kNull); - SkASSERT(this->getTag() == Tag::kNull); -} + // Access the pointer payload. + template + const T* ptr() const { + static_assert(sizeof(uintptr_t) == sizeof(Value) || + sizeof(uintptr_t) * 2 == sizeof(Value), ""); + + return (sizeof(uintptr_t) < sizeof(Value)) + // For 32-bit, pointers are stored unmodified. + ? *this->cast() + // For 64-bit, we use the high bits of the pointer as type storage. + : reinterpret_cast(*this->cast() & ~kTypeMask); + } + + // Type-bound recs only store their type. + static ValueRec MakeTypeBound(RecType t) { + ValueRec v; + *v.cast() = t; + SkASSERT(v.getRecType() == t); + return v; + } + + // Primitive recs store a type and inline primitive payload. + template + static ValueRec MakePrimitive(RecType t, T src) { + ValueRec v = MakeTypeBound(t); + *v.cast() = src; + SkASSERT(v.getRecType() == t); + return v; + } + + // Pointer recs store a type (in the upper kTypeBits bits) and a pointer. + template + static ValueRec MakePtr(RecType t, const T* p) { + SkASSERT((t & kTypeMask) == t); + if (sizeof(uintptr_t) == sizeof(Value)) { + // For 64-bit, we rely on the pointer hi bits being unused. + SkASSERT(!(reinterpret_cast(p) & kTypeMask)); + } + + ValueRec v = MakeTypeBound(t); + *v.cast() |= reinterpret_cast(p); + + SkASSERT(v.getRecType() == t); + SkASSERT(v.ptr() == p); + + return v; + } + + // Vector recs point to externally allocated slabs with the following layout: + // + // [size_t n] [REC_0] ... [REC_n-1] [optional extra trailing storage] + // + // Long strings use extra_alloc_size == 1 to store the \0 terminator. + template + static ValueRec MakeVector(RecType t, const T* src, size_t size, SkArenaAlloc& alloc) { + // For zero-size arrays, we just store a nullptr. + size_t* size_ptr = nullptr; + + if (size) { + // The Ts are already in memory, so their size should be safeish. + const auto total_size = sizeof(size_t) + sizeof(T) * size + extra_alloc_size; + size_ptr = reinterpret_cast(alloc.makeBytesAlignedTo(total_size, kRecAlign)); + auto* data_ptr = reinterpret_cast(size_ptr + 1); + *size_ptr = size; + memcpy(data_ptr, src, sizeof(T) * size); + } + + return MakePtr(t, size_ptr); + } + + size_t vectorSize(RecType t) const { + if (this->is()) return 0; + SkASSERT(this->getRecType() == t); + + const auto* size_ptr = this->ptr(); + return size_ptr ? *size_ptr : 0; + } + + template + const T* vectorBegin(RecType t) const { + if (this->is()) return nullptr; + SkASSERT(this->getRecType() == t); + + const auto* size_ptr = this->ptr(); + return size_ptr ? reinterpret_cast(size_ptr + 1) : nullptr; + } + + template + const T* vectorEnd(RecType t) const { + if (this->is()) return nullptr; + SkASSERT(this->getRecType() == t); + + const auto* size_ptr = this->ptr(); + return size_ptr ? reinterpret_cast(size_ptr + 1) + *size_ptr : nullptr; + } + + // Strings have two flavors: + // + // -- short strings (len <= 7) -> these are stored inline, in the record + // (one byte reserved for null terminator/type): + // + // [str] [\0]|[max_len - actual_len] + // + // Storing [max_len - actual_len] allows the 'len' field to double-up as a + // null terminator when size == max_len (this works 'cause kShortString == 0). + // + // -- long strings (len > 7) -> these are externally allocated vectors (VectorRec). + // + // The string data plus a null-char terminator are copied over. + static constexpr size_t kMaxInlineStringSize = sizeof(Value) - 1; + + static ValueRec MakeString(const char* src, size_t size, SkArenaAlloc& alloc) { + ValueRec v; + + if (size > kMaxInlineStringSize) { + v = MakeVector(kString, src, size, alloc); + const_cast(v.vectorBegin(ValueRec::kString))[size] = '\0'; + } else { + v = MakeTypeBound(kShortString); + auto* payload = v.cast(); + memcpy(payload, src, size); + payload[size] = '\0'; + + const auto len_tag = SkTo(kMaxInlineStringSize - size); + // This technically overwrites the type hi bits, but is safe because + // 1) kShortString == 0 + // 2) 0 <= len_tag <= 7 + static_assert(kShortString == 0, "please don't break this"); + payload[kMaxInlineStringSize] = len_tag; + SkASSERT(v.getRecType() == kShortString); + } + return v; + } -BoolValue::BoolValue(bool b) { - this->init_tagged(Tag::kBool); - *this->cast() = b; - SkASSERT(this->getTag() == Tag::kBool); + size_t stringSize() const { + if (this->getRecType() == ValueRec::kShortString) { + const auto* payload = this->cast(); + return kMaxInlineStringSize - SkToSizeT(payload[kMaxInlineStringSize]); + } + + return this->vectorSize(ValueRec::kString); + } + + const char* stringBegin() const { + if (this->getRecType() == ValueRec::kShortString) { + return this->cast(); + } + + return this->vectorBegin(ValueRec::kString); + } + + const char* stringEnd() const { + if (this->getRecType() == ValueRec::kShortString) { + const auto* payload = this->cast(); + return payload + kMaxInlineStringSize - SkToSizeT(payload[kMaxInlineStringSize]); + } + + return this->vectorEnd(ValueRec::kString); + } +}; + +} // namespace + + +// Boring public Value glue. + +const Value& Value::Null() { + static const Value g_null = ValueRec::MakeTypeBound(ValueRec::kNull); + return g_null; } -NumberValue::NumberValue(int32_t i) { - this->init_tagged(Tag::kInt); - *this->cast() = i; - SkASSERT(this->getTag() == Tag::kInt); +const Member& Member::Null() { + static const Member g_null = { Value::Null().as(), Value::Null() }; + return g_null; } -NumberValue::NumberValue(float f) { - this->init_tagged(Tag::kFloat); - *this->cast() = f; - SkASSERT(this->getTag() == Tag::kFloat); +Value::Type Value::getType() const { + static constexpr Value::Type kTypeMap[] = { + Value::Type::kString, // kShortString + Value::Type::kNull, // kNull + Value::Type::kBool, // kBool + Value::Type::kNumber, // kInt + Value::Type::kNumber, // kFloat + Value::Type::kString, // kString + Value::Type::kArray, // kArray + Value::Type::kObject, // kObject + }; + + const auto& rec = *reinterpret_cast(this); + const auto type_index = static_cast(rec.getRecType() >> ValueRec::kTypeShift); + SkASSERT(type_index < SK_ARRAY_COUNT(kTypeMap)); + + return kTypeMap[type_index]; } -// Vector recs point to externally allocated slabs with the following layout: -// -// [size_t n] [REC_0] ... [REC_n-1] [optional extra trailing storage] -// -// Long strings use extra_alloc_size == 1 to store the \0 terminator. -// -template -static void* MakeVector(const void* src, size_t size, SkArenaAlloc& alloc) { - // The Ts are already in memory, so their size should be safe. - const auto total_size = sizeof(size_t) + size * sizeof(T) + extra_alloc_size; - auto* size_ptr = reinterpret_cast(alloc.makeBytesAlignedTo(total_size, kRecAlign)); - auto* data_ptr = reinterpret_cast(size_ptr + 1); - *size_ptr = size; - memcpy(data_ptr, src, size * sizeof(T)); - - return size_ptr; +template <> +bool PrimitiveValue::operator*() const { + const auto& rec = *reinterpret_cast(this); + + if (rec.is()) return false; + + SkASSERT(rec.getRecType() == ValueRec::kBool); + + return *rec.cast(); } -ArrayValue::ArrayValue(const Value* src, size_t size, SkArenaAlloc& alloc) { - this->init_tagged_pointer(Tag::kArray, MakeVector(src, size, alloc)); - SkASSERT(this->getTag() == Tag::kArray); +template <> +double PrimitiveValue::operator*() const { + const auto& rec = *reinterpret_cast(this); + + if (rec.is()) return 0; + + SkASSERT(rec.getRecType() == ValueRec::kInt || + rec.getRecType() == ValueRec::kFloat); + + return rec.getRecType() == ValueRec::kInt + ? static_cast(*rec.cast()) + : static_cast(*rec.cast()); } -// Strings have two flavors: -// -// -- short strings (len <= 7) -> these are stored inline, in the record -// (one byte reserved for null terminator/type): -// -// [str] [\0]|[max_len - actual_len] -// -// Storing [max_len - actual_len] allows the 'len' field to double-up as a -// null terminator when size == max_len (this works 'cause kShortString == 0). -// -// -- long strings (len > 7) -> these are externally allocated vectors (VectorRec). -// -// The string data plus a null-char terminator are copied over. -// -StringValue::StringValue(const char* src, size_t size, SkArenaAlloc& alloc) { - if (size > kMaxInlineStringSize) { - this->init_tagged_pointer(Tag::kString, MakeVector(src, size, alloc)); +template <> +size_t VectorValue::size() const { + return reinterpret_cast(this)->vectorSize(ValueRec::kArray); +} - auto* data = this->cast>()->begin(); - const_cast(data)[size] = '\0'; - SkASSERT(this->getTag() == Tag::kString); - return; - } +template <> +const Value* VectorValue::begin() const { + return reinterpret_cast(this)->vectorBegin(ValueRec::kArray); +} - this->init_tagged(Tag::kShortString); +template <> +const Value* VectorValue::end() const { + return reinterpret_cast(this)->vectorEnd(ValueRec::kArray); +} - auto* payload = this->cast(); - memcpy(payload, src, size); - payload[size] = '\0'; +template <> +size_t VectorValue::size() const { + return reinterpret_cast(this)->vectorSize(ValueRec::kObject); +} - const auto len_tag = SkTo(kMaxInlineStringSize - size); - // This technically overwrites the tag, but is safe because - // 1) kShortString == 0 - // 2) 0 <= len_tag <= 7 - static_assert(static_cast(Tag::kShortString) == 0, "please don't break this"); - payload[kMaxInlineStringSize] = len_tag; +template <> +const Member* VectorValue::begin() const { + return reinterpret_cast(this)->vectorBegin(ValueRec::kObject); +} - SkASSERT(this->getTag() == Tag::kShortString); +template <> +const Member* VectorValue::end() const { + return reinterpret_cast(this)->vectorEnd(ValueRec::kObject); } -ObjectValue::ObjectValue(const Member* src, size_t size, SkArenaAlloc& alloc) { - this->init_tagged_pointer(Tag::kObject, MakeVector(src, size, alloc)); - SkASSERT(this->getTag() == Tag::kObject); +template <> +size_t VectorValue::size() const { + return reinterpret_cast(this)->stringSize(); } +template <> +const char* VectorValue::begin() const { + return reinterpret_cast(this)->stringBegin(); +} -// Boring public Value glue. +template <> +const char* VectorValue::end() const { + return reinterpret_cast(this)->stringEnd(); +} const Value& ObjectValue::operator[](const char* key) const { // Reverse search for duplicates resolution (policy: return last). @@ -155,8 +381,7 @@ const Value& ObjectValue::operator[](const char* key) const { } } - static const Value g_null = NullValue(); - return g_null; + return Value::Null(); } namespace { @@ -232,7 +457,7 @@ public: fScopeStack.reserve(kScopeStackReserve); } - const Value parse(const char* p) { + const Value& parse(const char* p) { p = skip_ws(p); switch (*p) { @@ -241,7 +466,7 @@ public: case '[': goto match_array; default: - return this->error(NullValue(), p, "invalid top-level value"); + return this->error(Value::Null(), p, "invalid top-level value"); } match_object: @@ -255,15 +480,15 @@ public: // goto match_object_key; match_object_key: p = skip_ws(p); - if (*p != '"') return this->error(NullValue(), p, "expected object key"); + if (*p != '"') return this->error(Value::Null(), p, "expected object key"); p = this->matchString(p, [this](const char* key, size_t size) { this->pushObjectKey(key, size); }); - if (!p) return NullValue(); + if (!p) return Value::Null(); p = skip_ws(p); - if (*p != ':') return this->error(NullValue(), p, "expected ':' separator"); + if (*p != ':') return this->error(Value::Null(), p, "expected ':' separator"); ++p; @@ -273,7 +498,7 @@ public: switch (*p) { case '\0': - return this->error(NullValue(), p, "unexpected input end"); + return this->error(Value::Null(), p, "unexpected input end"); case '"': p = this->matchString(p, [this](const char* str, size_t size) { this->pushString(str, size); @@ -297,7 +522,7 @@ public: break; } - if (!p) return NullValue(); + if (!p) return Value::Null(); // goto match_post_value; match_post_value: @@ -317,7 +542,7 @@ public: case '}': goto pop_object; default: - return this->error(NullValue(), p - 1, "unexpected value-trailing token"); + return this->error(Value::Null(), p - 1, "unexpected value-trailing token"); } // unreachable @@ -327,7 +552,7 @@ public: SkASSERT(*p == '}'); if (fScopeStack.back() < 0) { - return this->error(NullValue(), p, "unexpected object terminator"); + return this->error(Value::Null(), p, "unexpected object terminator"); } this->popObjectScope(); @@ -340,11 +565,13 @@ public: if (fScopeStack.empty()) { SkASSERT(fValueStack.size() == 1); + auto* root = fAlloc.make(); + *root = fValueStack.front(); // Stop condition: parsed the top level element and there is no trailing garbage. return *skip_ws(p) == '\0' - ? fValueStack.front() - : this->error(NullValue(), p, "trailing root garbage"); + ? *root + : this->error(Value::Null(), p, "trailing root garbage"); } goto match_post_value; @@ -362,7 +589,7 @@ public: SkASSERT(*p == ']'); if (fScopeStack.back() >= 0) { - return this->error(NullValue(), p, "unexpected array terminator"); + return this->error(Value::Null(), p, "unexpected array terminator"); } this->popArrayScope(); @@ -370,7 +597,7 @@ public: goto pop_common; SkASSERT(false); - return NullValue(); + return Value::Null(); } const SkString& getError() const { @@ -387,12 +614,11 @@ private: SkString fError; - template - void popScopeAsVec(size_t scope_start) { + template + void popScopeAsVec(ValueRec::RecType type, size_t scope_start) { SkASSERT(scope_start > 0); SkASSERT(scope_start <= fValueStack.size()); - using T = typename VectorT::ValueT; static_assert( sizeof(T) >= sizeof(Value), ""); static_assert( sizeof(T) % sizeof(Value) == 0, ""); static_assert(alignof(T) == alignof(Value), ""); @@ -404,7 +630,7 @@ private: const auto* begin = reinterpret_cast(fValueStack.data() + scope_start); // Instantiate the placeholder value added in onPush{Object/Array}. - fValueStack[scope_start - 1] = VectorT(begin, count, fAlloc); + fValueStack[scope_start - 1] = ValueRec::MakeVector(type, begin, count, fAlloc); // Drop the current scope. fScopeStack.pop_back(); @@ -422,7 +648,7 @@ private: void popObjectScope() { const auto scope_start = fScopeStack.back(); SkASSERT(scope_start > 0); - this->popScopeAsVec(SkTo(scope_start)); + this->popScopeAsVec(ValueRec::kObject, SkTo(scope_start)); SkDEBUGCODE( const auto& obj = fValueStack.back().as(); @@ -444,7 +670,7 @@ private: void popArrayScope() { const auto scope_start = -fScopeStack.back(); SkASSERT(scope_start > 0); - this->popScopeAsVec(SkTo(scope_start)); + this->popScopeAsVec(ValueRec::kArray, SkTo(scope_start)); SkDEBUGCODE( const auto& arr = fValueStack.back().as(); @@ -460,31 +686,31 @@ private: } void pushTrue() { - fValueStack.push_back(BoolValue(true)); + fValueStack.push_back(ValueRec::MakePrimitive(ValueRec::kBool, true)); } void pushFalse() { - fValueStack.push_back(BoolValue(false)); + fValueStack.push_back(ValueRec::MakePrimitive(ValueRec::kBool, false)); } void pushNull() { - fValueStack.push_back(NullValue()); + fValueStack.push_back(ValueRec::MakeTypeBound(ValueRec::kNull)); } void pushString(const char* s, size_t size) { - fValueStack.push_back(StringValue(s, size, fAlloc)); + fValueStack.push_back(ValueRec::MakeString(s, size, fAlloc)); } void pushInt32(int32_t i) { - fValueStack.push_back(NumberValue(i)); + fValueStack.push_back(ValueRec::MakePrimitive(ValueRec::kInt, i)); } void pushFloat(float f) { - fValueStack.push_back(NumberValue(f)); + fValueStack.push_back(ValueRec::MakePrimitive(ValueRec::kFloat, f)); } template - T error(T&& ret_val, const char* p, const char* msg) { + const T& error(const T& ret_val, const char* p, const char* msg) { #if defined(SK_JSON_REPORT_ERRORS) static constexpr size_t kMaxContext = 128; fError = SkStringPrintf("%s: >", msg); @@ -704,25 +930,17 @@ void Write(const Value& v, SkWStream* stream) { } // namespace -SkString Value::toString() const { - SkDynamicMemoryWStream wstream; - Write(*this, &wstream); - const auto data = wstream.detachAsData(); - // TODO: is there a better way to pass data around without copying? - return SkString(static_cast(data->data()), data->size()); -} - static constexpr size_t kMinChunkSize = 4096; -DOM::DOM(const char* c_str) +DOM::DOM(const char* cstr) : fAlloc(kMinChunkSize) { DOMParser parser(fAlloc); - fRoot = parser.parse(c_str); + fRoot = &parser.parse(cstr); } void DOM::write(SkWStream* stream) const { - Write(fRoot, stream); + Write(*fRoot, stream); } } // namespace skjson diff --git a/modules/skjson/src/SkJSONTest.cpp b/modules/skjson/src/SkJSONTest.cpp index 6d4338cbbb..8b876ddf4e 100644 --- a/modules/skjson/src/SkJSONTest.cpp +++ b/modules/skjson/src/SkJSONTest.cpp @@ -7,9 +7,7 @@ #include "Test.h" -#include "SkArenaAlloc.h" #include "SkJSON.h" -#include "SkString.h" #include "SkStream.h" using namespace skjson; @@ -109,29 +107,18 @@ static void check_primitive(skiatest::Reporter* reporter, const Value& v, T pv, bool is_type) { REPORTER_ASSERT(reporter, v.is() == is_type); - const VT* cast_t = v; - REPORTER_ASSERT(reporter, (cast_t != nullptr) == is_type); - - if (is_type) { - REPORTER_ASSERT(reporter, &v.as() == cast_t); - REPORTER_ASSERT(reporter, *v.as() == pv); - } + REPORTER_ASSERT(reporter, *v.as() == pv); } template static void check_vector(skiatest::Reporter* reporter, const Value& v, size_t expected_size, bool is_vector) { REPORTER_ASSERT(reporter, v.is() == is_vector); - const T* cast_t = v; - REPORTER_ASSERT(reporter, (cast_t != nullptr) == is_vector); - - if (is_vector) { - const auto& vec = v.as(); - REPORTER_ASSERT(reporter, &vec == cast_t); - REPORTER_ASSERT(reporter, vec.size() == expected_size); - REPORTER_ASSERT(reporter, vec.begin() != nullptr); - REPORTER_ASSERT(reporter, vec.end() == vec.begin() + expected_size); - } + + const auto& vec = v.as(); + REPORTER_ASSERT(reporter, vec.size() == expected_size); + REPORTER_ASSERT(reporter, (vec.begin() != nullptr) == is_vector); + REPORTER_ASSERT(reporter, vec.end() == vec.begin() + expected_size); } static void check_string(skiatest::Reporter* reporter, const Value& v, const char* s) { @@ -141,7 +128,7 @@ static void check_string(skiatest::Reporter* reporter, const Value& v, const cha } } -DEF_TEST(SkJSON_DOM_visit, reporter) { +DEF_TEST(SkJSON_DOM, reporter) { static constexpr char json[] = "{ \n\ \"k1\": null, \n\ \"k2\": false, \n\ @@ -244,6 +231,7 @@ DEF_TEST(SkJSON_DOM_visit, reporter) { check_primitive(reporter, v.as()[0], 1, true); check_primitive(reporter, v.as()[1], true, true); check_vector(reporter, v.as()[2], 3, true); + REPORTER_ASSERT(reporter, v.as()[3].is()); } { @@ -275,100 +263,10 @@ DEF_TEST(SkJSON_DOM_visit, reporter) { check_string(reporter, v.as()["kk1"], "baz"); check_primitive(reporter, v.as()["kk2"], false, true); } -} - -template -void check_value(skiatest::Reporter* reporter, const Value& v, const char* expected_string) { - REPORTER_ASSERT(reporter, v.is()); - - const T* cast_t = v; - REPORTER_ASSERT(reporter, cast_t == &v.as()); - - const auto vstr = v.toString(); - REPORTER_ASSERT(reporter, 0 == strcmp(expected_string, vstr.c_str())); -} - -DEF_TEST(SkJSON_DOM_build, reporter) { - SkArenaAlloc alloc(4096); - - const auto v0 = NullValue(); - check_value(reporter, v0, "null"); - - const auto v1 = BoolValue(true); - check_value(reporter, v1, "true"); - - const auto v2 = BoolValue(false); - check_value(reporter, v2, "false"); - const auto v3 = NumberValue(0); - check_value(reporter, v3, "0"); - - const auto v4 = NumberValue(42); - check_value(reporter, v4, "42"); - - const auto v5 = NumberValue(42.75f); - check_value(reporter, v5, "42.75"); - - const auto v6 = StringValue(nullptr, 0, alloc); - check_value(reporter, v6, "\"\""); - - const auto v7 = StringValue(" foo ", 5, alloc); - check_value(reporter, v7, "\" foo \""); - - const auto v8 = StringValue(" foo bar baz ", 13, alloc); - check_value(reporter, v8, "\" foo bar baz \""); - - const auto v9 = ArrayValue(nullptr, 0, alloc); - check_value(reporter, v9, "[]"); - - const Value values0[] = { v0, v3, v9 }; - const auto v10 = ArrayValue(values0, SK_ARRAY_COUNT(values0), alloc); - check_value(reporter, v10, "[null,0,[]]"); - - const auto v11 = ObjectValue(nullptr, 0, alloc); - check_value(reporter, v11, "{}"); - - const Member members0[] = { - { StringValue("key_0", 5, alloc), v1 }, - { StringValue("key_1", 5, alloc), v4 }, - { StringValue("key_2", 5, alloc), v11 }, - }; - const auto v12 = ObjectValue(members0, SK_ARRAY_COUNT(members0), alloc); - check_value(reporter, v12, "{" - "\"key_0\":true," - "\"key_1\":42," - "\"key_2\":{}" - "}"); - - const Value values1[] = { v2, v6, v12 }; - const auto v13 = ArrayValue(values1, SK_ARRAY_COUNT(values1), alloc); - check_value(reporter, v13, "[" - "false," - "\"\"," - "{" - "\"key_0\":true," - "\"key_1\":42," - "\"key_2\":{}" - "}" - "]"); - - const Member members1[] = { - { StringValue("key_00", 6, alloc), v5 }, - { StringValue("key_01", 6, alloc), v7 }, - { StringValue("key_02", 6, alloc), v13 }, - }; - const auto v14 = ObjectValue(members1, SK_ARRAY_COUNT(members1), alloc); - check_value(reporter, v14, "{" - "\"key_00\":42.75," - "\"key_01\":\" foo \"," - "\"key_02\":[" - "false," - "\"\"," - "{" - "\"key_0\":true," - "\"key_1\":42," - "\"key_2\":{}" - "}" - "]" - "}"); + { + const auto& v = + jroot["foo"].as()["bar"].as()["baz"]; + REPORTER_ASSERT(reporter, v.is()); + } } -- cgit v1.2.3