Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add Key() method to the Handler concept #134

Merged
merged 11 commits into from
Sep 5, 2014
Merged

add Key() method to the Handler concept #134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
57e1c87
add `Key()` method to the `Handler` concept
Kosta-Github Sep 4, 2014
17b1a14
add `Key()` method to `Writer` and `PrettyWriter`
Kosta-Github Sep 4, 2014
23b3d24
add `Key()` to IterativeParsingReaderHandler` in `readertest.cpp`
Kosta-Github Sep 4, 2014
0f40710
fix `simplereader.cpp` and the `Key()->String()` forwarding in `BaseR…
Kosta-Github Sep 4, 2014
c8da4d8
add `Key()->String()` forwarding to the `capitalize` example
Kosta-Github Sep 4, 2014
8b2faff
enhance `IterativeParsingReaderHandler` to check for `Key()` events
Kosta-Github Sep 4, 2014
25f4892
removed unused argument names
Kosta-Github Sep 4, 2014
4a71dc6
reunified `ParseKey`, `ParseString`, and `ParseStringOrKey` again
Kosta-Github Sep 4, 2014
e70494b
fix for `Reader::ParseString()` implementation plus some minor code c…
Kosta-Github Sep 4, 2014
00ac102
more changes
Kosta-Github Sep 4, 2014
f0d9ab4
finally fixing `Reader::ParseString()`
Kosta-Github Sep 4, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions example/capitalize/capitalize.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ struct CapitalizeFilter {
return out_.String(&buffer_.front(), length, true); // true = output handler need to copy the string
}
bool StartObject() { return out_.StartObject(); }
bool Key(const char* str, SizeType length, bool copy) { return String(str, length, copy); }
bool EndObject(SizeType memberCount) { return out_.EndObject(memberCount); }
bool StartArray() { return out_.StartArray(); }
bool EndArray(SizeType elementCount) { return out_.EndArray(elementCount); }
Expand Down
4 changes: 4 additions & 0 deletions example/simplereader/simplereader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ struct MyHandler {
return true;
}
bool StartObject() { cout << "StartObject()" << endl; return true; }
bool Key(const char* str, SizeType length, bool copy) {
cout << "Key(" << str << ", " << length << ", " << boolalpha << copy << ")" << endl;
return true;
}
bool EndObject(SizeType memberCount) { cout << "EndObject(" << memberCount << ")" << endl; return true; }
bool StartArray() { cout << "StartArray()" << endl; return true; }
bool EndArray(SizeType elementCount) { cout << "EndArray(" << elementCount << ")" << endl; return true; }
Expand Down
4 changes: 3 additions & 1 deletion include/rapidjson/document.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1386,7 +1386,7 @@ int z = a[0u].GetInt(); // This works too.
if (!handler.StartObject())
return false;
for (ConstMemberIterator m = MemberBegin(); m != MemberEnd(); ++m) {
if (!handler.String(m->name.GetString(), m->name.GetStringLength(), (m->name.flags_ & kCopyFlag) != 0))
if (!handler.Key(m->name.GetString(), m->name.GetStringLength(), (m->name.flags_ & kCopyFlag) != 0))
return false;
if (!m->value.Accept(handler))
return false;
Expand Down Expand Up @@ -1794,6 +1794,8 @@ class GenericDocument : public GenericValue<Encoding, Allocator> {

bool StartObject() { new (stack_.template Push<ValueType>()) ValueType(kObjectType); return true; }

bool Key(const Ch* str, SizeType length, bool copy) { return String(str, length, copy); }

bool EndObject(SizeType memberCount) {
typename ValueType::Member* members = stack_.template Pop<typename ValueType::Member>(memberCount);
stack_.template Top<ValueType>()->SetObjectRaw(members, (SizeType)memberCount, GetAllocator());
Expand Down
3 changes: 3 additions & 0 deletions include/rapidjson/prettywriter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ class PrettyWriter : public Writer<OutputStream, SourceEncoding, TargetEncoding,
return Base::WriteStartObject();
}

bool Key(const Ch* str, SizeType length, bool copy = false) { return String(str, length, copy); }

bool EndObject(SizeType memberCount = 0) {
(void)memberCount;
RAPIDJSON_ASSERT(Base::level_stack_.GetSize() >= sizeof(typename Base::Level));
Expand Down Expand Up @@ -135,6 +137,7 @@ class PrettyWriter : public Writer<OutputStream, SourceEncoding, TargetEncoding,

//! Simpler but slower overload.
bool String(const Ch* str) { return String(str, internal::StrLen(str)); }
bool Key(const Ch* str) { return Key(str, internal::StrLen(str)); }

//@}
protected:
Expand Down
19 changes: 12 additions & 7 deletions include/rapidjson/reader.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ concept Handler {
bool Double(double d);
bool String(const Ch* str, SizeType length, bool copy);
bool StartObject();
bool Key(const Ch* str, SizeType length, bool copy);
bool EndObject(SizeType memberCount);
bool StartArray();
bool EndArray(SizeType elementCount);
Expand Down Expand Up @@ -181,6 +182,7 @@ struct BaseReaderHandler {
bool Double(double) { return static_cast<Override&>(*this).Default(); }
bool String(const Ch*, SizeType, bool) { return static_cast<Override&>(*this).Default(); }
bool StartObject() { return static_cast<Override&>(*this).Default(); }
bool Key(const Ch* str, SizeType len, bool copy) { return static_cast<Override&>(*this).String(str, len, copy); }
bool EndObject(SizeType) { return static_cast<Override&>(*this).Default(); }
bool StartArray() { return static_cast<Override&>(*this).Default(); }
bool EndArray(SizeType) { return static_cast<Override&>(*this).Default(); }
Expand Down Expand Up @@ -471,7 +473,7 @@ class GenericReader {
if (is.Peek() != '"')
RAPIDJSON_PARSE_ERROR(kParseErrorObjectMissName, is.Tell());

ParseString<parseFlags>(is, handler);
ParseString<parseFlags>(is, handler, true);
RAPIDJSON_PARSE_ERROR_EARLY_RETURN_VOID;

SkipWhitespace(is);
Expand Down Expand Up @@ -617,26 +619,29 @@ class GenericReader {

// Parse string and generate String event. Different code paths for kParseInsituFlag.
template<unsigned parseFlags, typename InputStream, typename Handler>
void ParseString(InputStream& is, Handler& handler) {
void ParseString(InputStream& is, Handler& handler, bool isKey = false) {
internal::StreamLocalCopy<InputStream> copy(is);
InputStream& s(copy.s);

bool success = false;
if (parseFlags & kParseInsituFlag) {
typename InputStream::Ch *head = s.PutBegin();
ParseStringToStream<parseFlags, SourceEncoding, SourceEncoding>(s, s);
RAPIDJSON_PARSE_ERROR_EARLY_RETURN_VOID;
size_t length = s.PutEnd(head) - 1;
RAPIDJSON_ASSERT(length <= 0xFFFFFFFF);
if (!handler.String((typename TargetEncoding::Ch*)head, SizeType(length), false))
RAPIDJSON_PARSE_ERROR(kParseErrorTermination, s.Tell());
const typename TargetEncoding::Ch* const str = (typename TargetEncoding::Ch*)head;
success = (isKey ? handler.Key(str, SizeType(length), false) : handler.String(str, SizeType(length), false));
}
else {
StackStream stackStream(stack_);
ParseStringToStream<parseFlags, SourceEncoding, TargetEncoding>(s, stackStream);
RAPIDJSON_PARSE_ERROR_EARLY_RETURN_VOID;
if (!handler.String(stack_.template Pop<typename TargetEncoding::Ch>(stackStream.length_), stackStream.length_ - 1, true))
RAPIDJSON_PARSE_ERROR(kParseErrorTermination, s.Tell());
const typename TargetEncoding::Ch* const str = stack_.template Pop<typename TargetEncoding::Ch>(stackStream.length_);
success = (isKey ? handler.Key(str, stackStream.length_ - 1, true) : handler.String(str, stackStream.length_ - 1, true));
}
if (!success)
RAPIDJSON_PARSE_ERROR(kParseErrorTermination, s.Tell());
}

// Parse string to an output is
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding duplicate code via a template parameter will lead to binary bloat due to the duplicate implementation for each instantiated parseFlags value. I would prefer a parameter to ParseStringOrKey (it's internal anyway).
Why not simply keep it as ParseString and add an optional flag:

void ParseString(InputStream& is, Handler& handler, bool isKey = false) {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

based on all the other templated stuff in that file I think you guys are aiming for absolute runtime performance; therefore I chose the template bool parameter over the "runtime" bool parameter passed as regular argument...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's always a tradeoff. The increased program size will lead to I$ pressure for a very small anticipated performance benefit.

Expand Down Expand Up @@ -1194,7 +1199,7 @@ class GenericReader {
}

case IterativeParsingMemberKeyState:
ParseString<parseFlags>(is, handler);
ParseString<parseFlags>(is, handler, true);
if (HasParseError())
return IterativeParsingErrorState;
else
Expand Down
3 changes: 3 additions & 0 deletions include/rapidjson/writer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,8 @@ class Writer {
return WriteStartObject();
}

bool Key(const Ch* str, SizeType length, bool copy = false) { return String(str, length, copy); }

bool EndObject(SizeType memberCount = 0) {
(void)memberCount;
RAPIDJSON_ASSERT(level_stack_.GetSize() >= sizeof(Level));
Expand Down Expand Up @@ -165,6 +167,7 @@ class Writer {

//! Simpler but slower overload.
bool String(const Ch* str) { return String(str, internal::StrLen(str)); }
bool Key(const Ch* str) { return Key(str, internal::StrLen(str)); }

//@}

Expand Down
13 changes: 8 additions & 5 deletions test/unittest/readertest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -819,9 +819,10 @@ struct IterativeParsingReaderHandler {
const static int LOG_DOUBLE = -7;
const static int LOG_STRING = -8;
const static int LOG_STARTOBJECT = -9;
const static int LOG_ENDOBJECT = -10;
const static int LOG_STARTARRAY = -11;
const static int LOG_ENDARRAY = -12;
const static int LOG_KEY = -10;
const static int LOG_ENDOBJECT = -11;
const static int LOG_STARTARRAY = -12;
const static int LOG_ENDARRAY = -13;

const static size_t LogCapacity = 256;
int Logs[LogCapacity];
Expand All @@ -848,6 +849,8 @@ struct IterativeParsingReaderHandler {

bool StartObject() { RAPIDJSON_ASSERT(LogCount < LogCapacity); Logs[LogCount++] = LOG_STARTOBJECT; return true; }

bool Key (const Ch*, SizeType, bool) { RAPIDJSON_ASSERT(LogCount < LogCapacity); Logs[LogCount++] = LOG_KEY; return true; }

bool EndObject(SizeType c) {
RAPIDJSON_ASSERT(LogCount < LogCapacity);
Logs[LogCount++] = LOG_ENDOBJECT;
Expand Down Expand Up @@ -880,7 +883,7 @@ TEST(Reader, IterativeParsing_General) {
handler.LOG_STARTARRAY,
handler.LOG_INT,
handler.LOG_STARTOBJECT,
handler.LOG_STRING,
handler.LOG_KEY,
handler.LOG_STARTARRAY,
handler.LOG_INT,
handler.LOG_INT,
Expand Down Expand Up @@ -918,7 +921,7 @@ TEST(Reader, IterativeParsing_Count) {
handler.LOG_STARTOBJECT,
handler.LOG_ENDOBJECT, 0,
handler.LOG_STARTOBJECT,
handler.LOG_STRING,
handler.LOG_KEY,
handler.LOG_INT,
handler.LOG_ENDOBJECT, 1,
handler.LOG_STARTARRAY,
Expand Down