From c5297bf9c7f791dd0e5603e147dcb66b1d29d177 Mon Sep 17 00:00:00 2001 From: Hans Johnson Date: Mon, 14 Jan 2019 17:08:51 -0600 Subject: [PATCH 1/8] PERF: readability container size empty The emptiness of a container should be checked using the empty() method instead of the size() method. It is not guaranteed that size() is a constant-time function, and it is generally more efficient and also shows clearer intent to use empty(). Furthermore some containers may implement the empty() method but not implement the size() method. Using empty() whenever possible makes it easier to switch to another container in the future. SRCDIR=/Users/johnsonhj/src/jsoncpp/ #My local SRC BLDDIR=/Users/johnsonhj/src/jsoncpp/cmake-build-debug/ #My local BLD cd /Users/johnsonhj/src/jsoncpp/cmake-build-debug/ run-clang-tidy.py -extra-arg=-D__clang__ -checks=-*,readability-container-size-empty -header-filter=.* -fix --- src/lib_json/json_reader.cpp | 6 +++--- src/lib_json/json_value.cpp | 4 ++-- src/lib_json/json_writer.cpp | 8 ++++---- src/test_lib_json/main.cpp | 28 ++++++++++++++-------------- 4 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/lib_json/json_reader.cpp b/src/lib_json/json_reader.cpp index fe796349b..aa5156f0c 100644 --- a/src/lib_json/json_reader.cpp +++ b/src/lib_json/json_reader.cpp @@ -880,7 +880,7 @@ bool Reader::pushError(const Value& value, return true; } -bool Reader::good() const { return !errors_.size(); } +bool Reader::good() const { return errors_.empty(); } // exact copy of Features class OurFeatures { @@ -1895,7 +1895,7 @@ bool OurReader::pushError(const Value& value, return true; } -bool OurReader::good() const { return !errors_.size(); } +bool OurReader::good() const { return errors_.empty(); } class OurCharReader : public CharReader { bool const collectComments_; @@ -1961,7 +1961,7 @@ bool CharReaderBuilder::validate(Json::Value* invalid) const { inv[key] = settings_[key]; } } - return 0u == inv.size(); + return inv.empty(); } Value& CharReaderBuilder::operator[](const JSONCPP_STRING& key) { return settings_[key]; diff --git a/src/lib_json/json_value.cpp b/src/lib_json/json_value.cpp index 1aec87fd0..c7fcfa413 100644 --- a/src/lib_json/json_value.cpp +++ b/src/lib_json/json_value.cpp @@ -889,8 +889,8 @@ bool Value::isConvertibleTo(ValueType other) const { return (isNumeric() && asDouble() == 0.0) || (type_ == booleanValue && value_.bool_ == false) || (type_ == stringValue && asString().empty()) || - (type_ == arrayValue && value_.map_->size() == 0) || - (type_ == objectValue && value_.map_->size() == 0) || + (type_ == arrayValue && value_.map_->empty()) || + (type_ == objectValue && value_.map_->empty()) || type_ == nullValue; case intValue: return isInt() || diff --git a/src/lib_json/json_writer.cpp b/src/lib_json/json_writer.cpp index 4f7712116..7138b7a7b 100644 --- a/src/lib_json/json_writer.cpp +++ b/src/lib_json/json_writer.cpp @@ -551,7 +551,7 @@ bool StyledWriter::isMultilineArray(const Value& value) { for (ArrayIndex index = 0; index < size && !isMultiLine; ++index) { const Value& childValue = value[index]; isMultiLine = ((childValue.isArray() || childValue.isObject()) && - childValue.size() > 0); + !childValue.empty()); } if (!isMultiLine) // check if line length > max line length { @@ -774,7 +774,7 @@ bool StyledStreamWriter::isMultilineArray(const Value& value) { for (ArrayIndex index = 0; index < size && !isMultiLine; ++index) { const Value& childValue = value[index]; isMultiLine = ((childValue.isArray() || childValue.isObject()) && - childValue.size() > 0); + !childValue.empty()); } if (!isMultiLine) // check if line length > max line length { @@ -1059,7 +1059,7 @@ bool BuiltStyledStreamWriter::isMultilineArray(Value const& value) { for (ArrayIndex index = 0; index < size && !isMultiLine; ++index) { Value const& childValue = value[index]; isMultiLine = ((childValue.isArray() || childValue.isObject()) && - childValue.size() > 0); + !childValue.empty()); } if (!isMultiLine) // check if line length > max line length { @@ -1226,7 +1226,7 @@ bool StreamWriterBuilder::validate(Json::Value* invalid) const { inv[key] = settings_[key]; } } - return 0u == inv.size(); + return inv.empty(); } Value& StreamWriterBuilder::operator[](const JSONCPP_STRING& key) { return settings_[key]; diff --git a/src/test_lib_json/main.cpp b/src/test_lib_json/main.cpp index 23da4b3a3..4dfca7538 100644 --- a/src/test_lib_json/main.cpp +++ b/src/test_lib_json/main.cpp @@ -1818,7 +1818,7 @@ JSONTEST_FIXTURE(StreamWriterTest, dropNullPlaceholders) { b.settings_["dropNullPlaceholders"] = false; JSONTEST_ASSERT(Json::writeString(b, nullValue) == "null"); b.settings_["dropNullPlaceholders"] = true; - JSONTEST_ASSERT(Json::writeString(b, nullValue) == ""); + JSONTEST_ASSERT(Json::writeString(b, nullValue).empty()); } JSONTEST_FIXTURE(StreamWriterTest, writeZeroes) { @@ -1850,8 +1850,8 @@ JSONTEST_FIXTURE(ReaderTest, parseWithNoErrors) { Json::Value root; bool ok = reader.parse("{ \"property\" : \"value\" }", root); JSONTEST_ASSERT(ok); - JSONTEST_ASSERT(reader.getFormattedErrorMessages().size() == 0); - JSONTEST_ASSERT(reader.getStructuredErrors().size() == 0); + JSONTEST_ASSERT(reader.getFormattedErrorMessages().empty()); + JSONTEST_ASSERT(reader.getStructuredErrors().empty()); } JSONTEST_FIXTURE(ReaderTest, parseWithNoErrorsTestingOffsets) { @@ -1862,8 +1862,8 @@ JSONTEST_FIXTURE(ReaderTest, parseWithNoErrorsTestingOffsets) { "null, \"false\" : false }", root); JSONTEST_ASSERT(ok); - JSONTEST_ASSERT(reader.getFormattedErrorMessages().size() == 0); - JSONTEST_ASSERT(reader.getStructuredErrors().size() == 0); + JSONTEST_ASSERT(reader.getFormattedErrorMessages().empty()); + JSONTEST_ASSERT(reader.getStructuredErrors().empty()); JSONTEST_ASSERT(root["property"].getOffsetStart() == 15); JSONTEST_ASSERT(root["property"].getOffsetLimit() == 34); JSONTEST_ASSERT(root["property"][0].getOffsetStart() == 16); @@ -1944,7 +1944,7 @@ JSONTEST_FIXTURE(CharReaderTest, parseWithNoErrors) { char const doc[] = "{ \"property\" : \"value\" }"; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); - JSONTEST_ASSERT(errs.size() == 0); + JSONTEST_ASSERT(errs.empty()); delete reader; } @@ -1958,7 +1958,7 @@ JSONTEST_FIXTURE(CharReaderTest, parseWithNoErrorsTestingOffsets) { "null, \"false\" : false }"; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); - JSONTEST_ASSERT(errs.size() == 0); + JSONTEST_ASSERT(errs.empty()); delete reader; } @@ -2014,7 +2014,7 @@ JSONTEST_FIXTURE(CharReaderTest, parseWithStackLimit) { JSONCPP_STRING errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); - JSONTEST_ASSERT(errs == ""); + JSONTEST_ASSERT(errs.empty()); JSONTEST_ASSERT_EQUAL("value", root["property"]); delete reader; } @@ -2061,7 +2061,7 @@ JSONTEST_FIXTURE(CharReaderFailIfExtraTest, issue164) { JSONCPP_STRING errs; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); - JSONTEST_ASSERT(errs == ""); + JSONTEST_ASSERT(errs.empty()); JSONTEST_ASSERT_EQUAL("property", root); delete reader; } @@ -2176,7 +2176,7 @@ JSONTEST_FIXTURE(CharReaderAllowDropNullTest, issue178) { char const doc[] = "[]"; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); - JSONTEST_ASSERT(errs == ""); + JSONTEST_ASSERT(errs.empty()); JSONTEST_ASSERT_EQUAL(0u, root.size()); JSONTEST_ASSERT_EQUAL(Json::arrayValue, root); } @@ -2184,7 +2184,7 @@ JSONTEST_FIXTURE(CharReaderAllowDropNullTest, issue178) { char const doc[] = "[null]"; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); - JSONTEST_ASSERT(errs == ""); + JSONTEST_ASSERT(errs.empty()); JSONTEST_ASSERT_EQUAL(1u, root.size()); } { @@ -2212,7 +2212,7 @@ JSONTEST_FIXTURE(CharReaderAllowDropNullTest, issue178) { char const doc[] = "[,null]"; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); - JSONTEST_ASSERT(errs == ""); + JSONTEST_ASSERT(errs.empty()); JSONTEST_ASSERT_EQUAL(2u, root.size()); } { @@ -2240,7 +2240,7 @@ JSONTEST_FIXTURE(CharReaderAllowDropNullTest, issue178) { char const doc[] = "[,,null]"; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); - JSONTEST_ASSERT(errs == ""); + JSONTEST_ASSERT(errs.empty()); JSONTEST_ASSERT_EQUAL(3u, root.size()); } { @@ -2263,7 +2263,7 @@ JSONTEST_FIXTURE(CharReaderAllowDropNullTest, issue178) { char const doc[] = "[,,,[]]"; bool ok = reader->parse(doc, doc + std::strlen(doc), &root, &errs); JSONTEST_ASSERT(ok); - JSONTEST_ASSERT(errs == ""); + JSONTEST_ASSERT(errs.empty()); JSONTEST_ASSERT_EQUAL(4u, root.size()); JSONTEST_ASSERT_EQUAL(Json::arrayValue, root[3u]); } From c44bfc562d6803844a949490061d7eb984e08f0e Mon Sep 17 00:00:00 2001 From: Hans Johnson Date: Mon, 14 Jan 2019 17:09:12 -0600 Subject: [PATCH 2/8] STYLE: Use range-based loops from C++11 C++11 Range based for loops can be used in Used as a more readable equivalent to the traditional for loop operating over a range of values, such as all elements in a container, in the forward direction.. Range based loopes are more explicit for only computing the end location once for containers. SRCDIR=/Users/johnsonhj/src/jsoncpp/ #My local SRC BLDDIR=/Users/johnsonhj/src/jsoncpp/cmake-build-debug/ #My local BLD cd /Users/johnsonhj/src/jsoncpp/cmake-build-debug/ run-clang-tidy.py -extra-arg=-D__clang__ -checks=-*,modernize-loop-convert -header-filter=.* -fix --- src/jsontestrunner/main.cpp | 4 +--- src/lib_json/json_reader.cpp | 16 ++++------------ src/lib_json/json_value.cpp | 9 +++------ src/test_lib_json/jsontest.cpp | 7 ++----- src/test_lib_json/main.cpp | 3 +-- 5 files changed, 11 insertions(+), 28 deletions(-) diff --git a/src/jsontestrunner/main.cpp b/src/jsontestrunner/main.cpp index 28691a2de..544929932 100644 --- a/src/jsontestrunner/main.cpp +++ b/src/jsontestrunner/main.cpp @@ -110,9 +110,7 @@ static void printValueTree(FILE* fout, Json::Value::Members members(value.getMemberNames()); std::sort(members.begin(), members.end()); JSONCPP_STRING suffix = *(path.end() - 1) == '.' ? "" : "."; - for (Json::Value::Members::iterator it = members.begin(); - it != members.end(); ++it) { - const JSONCPP_STRING name = *it; + for (auto name : members) { printValueTree(fout, value[name], path + suffix + name); } } break; diff --git a/src/lib_json/json_reader.cpp b/src/lib_json/json_reader.cpp index aa5156f0c..077da1be8 100644 --- a/src/lib_json/json_reader.cpp +++ b/src/lib_json/json_reader.cpp @@ -818,9 +818,7 @@ JSONCPP_STRING Reader::getFormatedErrorMessages() const { JSONCPP_STRING Reader::getFormattedErrorMessages() const { JSONCPP_STRING formattedMessage; - for (Errors::const_iterator itError = errors_.begin(); - itError != errors_.end(); ++itError) { - const ErrorInfo& error = *itError; + for (const auto & error : errors_) { formattedMessage += "* " + getLocationLineAndColumn(error.token_.start_) + "\n"; formattedMessage += " " + error.message_ + "\n"; @@ -833,9 +831,7 @@ JSONCPP_STRING Reader::getFormattedErrorMessages() const { std::vector Reader::getStructuredErrors() const { std::vector allErrors; - for (Errors::const_iterator itError = errors_.begin(); - itError != errors_.end(); ++itError) { - const ErrorInfo& error = *itError; + for (const auto & error : errors_) { Reader::StructuredError structured; structured.offset_start = error.token_.start_ - begin_; structured.offset_limit = error.token_.end_ - begin_; @@ -1833,9 +1829,7 @@ JSONCPP_STRING OurReader::getLocationLineAndColumn(Location location) const { JSONCPP_STRING OurReader::getFormattedErrorMessages() const { JSONCPP_STRING formattedMessage; - for (Errors::const_iterator itError = errors_.begin(); - itError != errors_.end(); ++itError) { - const ErrorInfo& error = *itError; + for (const auto & error : errors_) { formattedMessage += "* " + getLocationLineAndColumn(error.token_.start_) + "\n"; formattedMessage += " " + error.message_ + "\n"; @@ -1848,9 +1842,7 @@ JSONCPP_STRING OurReader::getFormattedErrorMessages() const { std::vector OurReader::getStructuredErrors() const { std::vector allErrors; - for (Errors::const_iterator itError = errors_.begin(); - itError != errors_.end(); ++itError) { - const ErrorInfo& error = *itError; + for (const auto & error : errors_) { OurReader::StructuredError structured; structured.offset_start = error.token_.start_ - begin_; structured.offset_limit = error.token_.end_ - begin_; diff --git a/src/lib_json/json_value.cpp b/src/lib_json/json_value.cpp index c7fcfa413..5e832392b 100644 --- a/src/lib_json/json_value.cpp +++ b/src/lib_json/json_value.cpp @@ -1655,8 +1655,7 @@ void Path::invalidPath(const JSONCPP_STRING& /*path*/, int /*location*/) { const Value& Path::resolve(const Value& root) const { const Value* node = &root; - for (Args::const_iterator it = args_.begin(); it != args_.end(); ++it) { - const PathArgument& arg = *it; + for (const auto & arg : args_) { if (arg.kind_ == PathArgument::kindIndex) { if (!node->isArray() || !node->isValidIndex(arg.index_)) { // Error: unable to resolve path (array value expected at position... @@ -1681,8 +1680,7 @@ const Value& Path::resolve(const Value& root) const { Value Path::resolve(const Value& root, const Value& defaultValue) const { const Value* node = &root; - for (Args::const_iterator it = args_.begin(); it != args_.end(); ++it) { - const PathArgument& arg = *it; + for (const auto & arg : args_) { if (arg.kind_ == PathArgument::kindIndex) { if (!node->isArray() || !node->isValidIndex(arg.index_)) return defaultValue; @@ -1700,8 +1698,7 @@ Value Path::resolve(const Value& root, const Value& defaultValue) const { Value& Path::make(Value& root) const { Value* node = &root; - for (Args::const_iterator it = args_.begin(); it != args_.end(); ++it) { - const PathArgument& arg = *it; + for (const auto & arg : args_) { if (arg.kind_ == PathArgument::kindIndex) { if (!node->isArray()) { // Error: node is not an array at position ... diff --git a/src/test_lib_json/jsontest.cpp b/src/test_lib_json/jsontest.cpp index 5e04aa434..f66150587 100644 --- a/src/test_lib_json/jsontest.cpp +++ b/src/test_lib_json/jsontest.cpp @@ -150,9 +150,7 @@ void TestResult::printFailure(bool printTestName) const { } // Print in reverse to display the callstack in the right order - Failures::const_iterator itEnd = failures_.end(); - for (Failures::const_iterator it = failures_.begin(); it != itEnd; ++it) { - const Failure& failure = *it; + for (const auto & failure : failures_ ) { JSONCPP_STRING indent(failure.nestingLevel_ * 2, ' '); if (failure.file_) { printf("%s%s(%u): ", indent.c_str(), failure.file_, failure.line_); @@ -275,8 +273,7 @@ bool Runner::runAllTest(bool printSummary) const { } return true; } else { - for (unsigned int index = 0; index < failures.size(); ++index) { - TestResult& result = failures[index]; + for (auto & result : failures) { result.printFailure(count > 1); } diff --git a/src/test_lib_json/main.cpp b/src/test_lib_json/main.cpp index 4dfca7538..8b2154739 100644 --- a/src/test_lib_json/main.cpp +++ b/src/test_lib_json/main.cpp @@ -2365,8 +2365,7 @@ JSONTEST_FIXTURE(CharReaderAllowSpecialFloatsTest, issue209) { { __LINE__, false, "{\"a\":.Infinity}" }, { __LINE__, false, "{\"a\":_Infinity}" }, { __LINE__, false, "{\"a\":_nfinity}" }, { __LINE__, true, "{\"a\":-Infinity}" } }; - for (size_t tdi = 0; tdi < sizeof(test_data) / sizeof(*test_data); ++tdi) { - const TestData& td = test_data[tdi]; + for (const auto& td : test_data) { bool ok = reader->parse(&*td.in.begin(), &*td.in.begin() + td.in.size(), &root, &errs); JSONTEST_ASSERT(td.ok == ok) << "line:" << td.line << "\n" From 5d2273956d9b2a2e1c780727615dc4a6c314a4d9 Mon Sep 17 00:00:00 2001 From: Hans Johnson Date: Mon, 14 Jan 2019 17:09:15 -0600 Subject: [PATCH 3/8] STYLE: Use auto for variable type matches the type of the initializer expression This check is responsible for using the auto type specifier for variable declarations to improve code readability and maintainability. The auto type specifier will only be introduced in situations where the variable type matches the type of the initializer expression. In other words auto should deduce the same type that was originally spelled in the source SRCDIR=/Users/johnsonhj/src/jsoncpp/ #My local SRC BLDDIR=/Users/johnsonhj/src/jsoncpp/cmake-build-debug/ #My local BLD cd /Users/johnsonhj/src/jsoncpp/cmake-build-debug/ run-clang-tidy.py -extra-arg=-D__clang__ -checks=-*,modernize-use-auto -header-filter = .* -fix --- src/jsontestrunner/main.cpp | 2 +- src/lib_json/json_reader.cpp | 6 +++--- src/lib_json/json_value.cpp | 14 +++++++------- src/lib_json/json_writer.cpp | 10 +++++----- src/test_lib_json/jsontest.cpp | 2 +- src/test_lib_json/main.cpp | 2 +- 6 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/jsontestrunner/main.cpp b/src/jsontestrunner/main.cpp index 544929932..6e0c0049a 100644 --- a/src/jsontestrunner/main.cpp +++ b/src/jsontestrunner/main.cpp @@ -56,7 +56,7 @@ static JSONCPP_STRING readInputTestFile(const char* path) { return JSONCPP_STRING(""); fseek(file, 0, SEEK_END); long const size = ftell(file); - unsigned long const usize = static_cast(size); + size_t const usize = static_cast(size); fseek(file, 0, SEEK_SET); JSONCPP_STRING text; char* buffer = new char[size + 1]; diff --git a/src/lib_json/json_reader.cpp b/src/lib_json/json_reader.cpp index 077da1be8..ce4aac446 100644 --- a/src/lib_json/json_reader.cpp +++ b/src/lib_json/json_reader.cpp @@ -577,7 +577,7 @@ bool Reader::decodeNumber(Token& token, Value& decoded) { Char c = *current++; if (c < '0' || c > '9') return decodeDouble(token, decoded); - Value::UInt digit(static_cast(c - '0')); + auto digit(static_cast(c - '0')); if (value >= threshold) { // We've hit or exceeded the max value divided by 10 (rounded down). If // a) we've only just touched the limit, b) this is the last digit, and @@ -1569,7 +1569,7 @@ bool OurReader::decodeNumber(Token& token, Value& decoded) { Char c = *current++; if (c < '0' || c > '9') return decodeDouble(token, decoded); - Value::UInt digit(static_cast(c - '0')); + auto digit(static_cast(c - '0')); if (value >= threshold) { // We've hit or exceeded the max value divided by 10 (rounded down). If // a) we've only just touched the limit, b) this is the last digit, and @@ -1611,7 +1611,7 @@ bool OurReader::decodeDouble(Token& token, Value& decoded) { if (length < 0) { return addError("Unable to parse token length", token); } - size_t const ulength = static_cast(length); + auto const ulength = static_cast(length); // Avoid using a string constant for the format control string given to // sscanf, as this can cause hard to debug crashes on OS X. See here for more diff --git a/src/lib_json/json_value.cpp b/src/lib_json/json_value.cpp index 5e832392b..ef9511788 100644 --- a/src/lib_json/json_value.cpp +++ b/src/lib_json/json_value.cpp @@ -990,7 +990,7 @@ Value& Value::operator[](ArrayIndex index) { if (type_ == nullValue) *this = Value(arrayValue); CZString key(index); - ObjectValues::iterator it = value_.map_->lower_bound(key); + auto it = value_.map_->lower_bound(key); if (it != value_.map_->end() && (*it).first == key) return (*it).second; @@ -1113,7 +1113,7 @@ Value& Value::resolveReference(const char* key) { *this = Value(objectValue); CZString actualKey(key, static_cast(strlen(key)), CZString::noDuplication); // NOTE! - ObjectValues::iterator it = value_.map_->lower_bound(actualKey); + auto it = value_.map_->lower_bound(actualKey); if (it != value_.map_->end() && (*it).first == actualKey) return (*it).second; @@ -1132,7 +1132,7 @@ Value& Value::resolveReference(char const* key, char const* end) { *this = Value(objectValue); CZString actualKey(key, static_cast(end - key), CZString::duplicateOnCopy); - ObjectValues::iterator it = value_.map_->lower_bound(actualKey); + auto it = value_.map_->lower_bound(actualKey); if (it != value_.map_->end() && (*it).first == actualKey) return (*it).second; @@ -1226,7 +1226,7 @@ bool Value::removeMember(const char* begin, const char* end, Value* removed) { } CZString actualKey(begin, static_cast(end - begin), CZString::noDuplication); - ObjectValues::iterator it = value_.map_->find(actualKey); + auto it = value_.map_->find(actualKey); if (it == value_.map_->end()) return false; if (removed) @@ -1262,7 +1262,7 @@ bool Value::removeIndex(ArrayIndex index, Value* removed) { return false; } CZString key(index); - ObjectValues::iterator it = value_.map_->find(key); + auto it = value_.map_->find(key); if (it == value_.map_->end()) { return false; } @@ -1276,7 +1276,7 @@ bool Value::removeIndex(ArrayIndex index, Value* removed) { } // erase the last one ("leftover") CZString keyLast(oldSize - 1); - ObjectValues::iterator itLast = value_.map_->find(keyLast); + auto itLast = value_.map_->find(keyLast); value_.map_->erase(itLast); return true; } @@ -1608,7 +1608,7 @@ Path::Path(const JSONCPP_STRING& path, void Path::makePath(const JSONCPP_STRING& path, const InArgs& in) { const char* current = path.c_str(); const char* end = current + path.length(); - InArgs::const_iterator itInArg = in.begin(); + auto itInArg = in.begin(); while (current != end) { if (*current == '[') { ++current; diff --git a/src/lib_json/json_writer.cpp b/src/lib_json/json_writer.cpp index 7138b7a7b..993001ca1 100644 --- a/src/lib_json/json_writer.cpp +++ b/src/lib_json/json_writer.cpp @@ -147,7 +147,7 @@ JSONCPP_STRING valueToString(double value, (precisionType == PrecisionType::significantDigits) ? "%.*g" : "%.*f", precision, value); assert(len >= 0); - size_t wouldPrint = static_cast(len); + auto wouldPrint = static_cast(len); if (wouldPrint >= buffer.size()) { buffer.resize(wouldPrint + 1); continue; @@ -409,7 +409,7 @@ void FastWriter::writeValue(const Value& value) { case objectValue: { Value::Members members(value.getMemberNames()); document_ += '{'; - for (Value::Members::iterator it = members.begin(); it != members.end(); + for (auto it = members.begin(); it != members.end(); ++it) { const JSONCPP_STRING& name = *it; if (it != members.begin()) @@ -479,7 +479,7 @@ void StyledWriter::writeValue(const Value& value) { else { writeWithIndent("{"); indent(); - Value::Members::iterator it = members.begin(); + auto it = members.begin(); for (;;) { const JSONCPP_STRING& name = *it; const Value& childValue = value[name]; @@ -699,7 +699,7 @@ void StyledStreamWriter::writeValue(const Value& value) { else { writeWithIndent("{"); indent(); - Value::Members::iterator it = members.begin(); + auto it = members.begin(); for (;;) { const JSONCPP_STRING& name = *it; const Value& childValue = value[name]; @@ -979,7 +979,7 @@ void BuiltStyledStreamWriter::writeValue(Value const& value) { else { writeWithIndent("{"); indent(); - Value::Members::iterator it = members.begin(); + auto it = members.begin(); for (;;) { JSONCPP_STRING const& name = *it; Value const& childValue = value[name]; diff --git a/src/test_lib_json/jsontest.cpp b/src/test_lib_json/jsontest.cpp index f66150587..03703c9c4 100644 --- a/src/test_lib_json/jsontest.cpp +++ b/src/test_lib_json/jsontest.cpp @@ -278,7 +278,7 @@ bool Runner::runAllTest(bool printSummary) const { } if (printSummary) { - unsigned int failedCount = static_cast(failures.size()); + auto failedCount = static_cast(failures.size()); unsigned int passedCount = count - failedCount; printf("%u/%u tests passed (%u failure(s))\n", passedCount, count, failedCount); diff --git a/src/test_lib_json/main.cpp b/src/test_lib_json/main.cpp index 8b2154739..d0315d7b2 100644 --- a/src/test_lib_json/main.cpp +++ b/src/test_lib_json/main.cpp @@ -1036,7 +1036,7 @@ JSONTEST_FIXTURE(ValueTest, integers) { normalizeFloatingPointStr(JsonTest::ToJsonString(val.asString()))); // 10^19 - const Json::UInt64 ten_to_19 = static_cast(1e19); + const auto ten_to_19 = static_cast(1e19); val = Json::Value(Json::UInt64(ten_to_19)); JSONTEST_ASSERT_EQUAL(Json::uintValue, val.type()); From c19402cfa2bd160854c106ce5ca1dc43faa834b1 Mon Sep 17 00:00:00 2001 From: Hans Johnson Date: Mon, 14 Jan 2019 17:09:19 -0600 Subject: [PATCH 4/8] PERF: Allow compiler to choose best way to construct a copy With move semantics added to the language and the standard library updated with move constructors added for many types it is now interesting to take an argument directly by value, instead of by const-reference, and then copy. This check allows the compiler to take care of choosing the best way to construct the copy. The transformation is usually beneficial when the calling code passes an rvalue and assumes the move construction is a cheap operation. This short example illustrates how the construction of the value happens: SRCDIR=/Users/johnsonhj/src/jsoncpp/ #My local SRC BLDDIR=/Users/johnsonhj/src/jsoncpp/cmake-build-debug/ #My local BLD cd /Users/johnsonhj/src/jsoncpp/cmake-build-debug/ run-clang-tidy.py -extra-arg=-D__clang__ -checks=-*,modernize-pass-by-value -header-filter=.* -fix --- include/json/value.h | 2 +- include/json/writer.h | 4 ++-- src/lib_json/json_reader.cpp | 5 ++--- src/lib_json/json_value.cpp | 2 +- src/lib_json/json_writer.cpp | 26 +++++++++++++------------- 5 files changed, 19 insertions(+), 20 deletions(-) diff --git a/include/json/value.h b/include/json/value.h index 3fdd88437..71bc65c0b 100644 --- a/include/json/value.h +++ b/include/json/value.h @@ -54,7 +54,7 @@ namespace Json { */ class JSON_API Exception : public std::exception { public: - Exception(JSONCPP_STRING const& msg); + Exception(JSONCPP_STRING msg); ~Exception() JSONCPP_NOEXCEPT override; char const* what() const JSONCPP_NOEXCEPT override; diff --git a/include/json/writer.h b/include/json/writer.h index 7faaa0241..34e71edac 100644 --- a/include/json/writer.h +++ b/include/json/writer.h @@ -300,8 +300,8 @@ class JSONCPP_DEPRECATED("Use StreamWriterBuilder instead") JSON_API /** * \param indentation Each level will be indented by this amount extra. */ - StyledStreamWriter(const JSONCPP_STRING& indentation = "\t"); - ~StyledStreamWriter() {} + StyledStreamWriter(JSONCPP_STRING indentation = "\t"); + ~StyledStreamWriter() = default; public: /** \brief Serialize a Value in JSON format. diff --git a/src/lib_json/json_reader.cpp b/src/lib_json/json_reader.cpp index ce4aac446..c5af01468 100644 --- a/src/lib_json/json_reader.cpp +++ b/src/lib_json/json_reader.cpp @@ -88,9 +88,8 @@ bool Reader::containsNewLine(Reader::Location begin, Reader::Location end) { // ////////////////////////////////////////////////////////////////// Reader::Reader() - : errors_(), document_(), begin_(), end_(), current_(), lastValueEnd_(), - lastValue_(), commentsBefore_(), features_(Features::all()), - collectComments_() {} + : errors_(), document_(), commentsBefore_(), features_(Features::all()) + {} Reader::Reader(const Features& features) : errors_(), document_(), begin_(), end_(), current_(), lastValueEnd_(), diff --git a/src/lib_json/json_value.cpp b/src/lib_json/json_value.cpp index ef9511788..2445d31a8 100644 --- a/src/lib_json/json_value.cpp +++ b/src/lib_json/json_value.cpp @@ -213,7 +213,7 @@ static inline void releaseStringValue(char* value, unsigned) { free(value); } namespace Json { -Exception::Exception(JSONCPP_STRING const& msg) : msg_(msg) {} +Exception::Exception(JSONCPP_STRING msg) : msg_(std::move(msg)) {} Exception::~Exception() JSONCPP_NOEXCEPT {} char const* Exception::what() const JSONCPP_NOEXCEPT { return msg_.c_str(); } RuntimeError::RuntimeError(JSONCPP_STRING const& msg) : Exception(msg) {} diff --git a/src/lib_json/json_writer.cpp b/src/lib_json/json_writer.cpp index 993001ca1..54fcb4fa1 100644 --- a/src/lib_json/json_writer.cpp +++ b/src/lib_json/json_writer.cpp @@ -642,8 +642,8 @@ bool StyledWriter::hasCommentForValue(const Value& value) { // Class StyledStreamWriter // ////////////////////////////////////////////////////////////////// -StyledStreamWriter::StyledStreamWriter(const JSONCPP_STRING& indentation) - : document_(nullptr), rightMargin_(74), indentation_(indentation), +StyledStreamWriter::StyledStreamWriter(JSONCPP_STRING indentation) + : document_(nullptr), indentation_(std::move(indentation)), addChildValues_(), indented_(false) {} void StyledStreamWriter::write(JSONCPP_OSTREAM& out, const Value& root) { @@ -872,11 +872,11 @@ struct CommentStyle { }; struct BuiltStyledStreamWriter : public StreamWriter { - BuiltStyledStreamWriter(JSONCPP_STRING const& indentation, + BuiltStyledStreamWriter(JSONCPP_STRING indentation, CommentStyle::Enum cs, - JSONCPP_STRING const& colonSymbol, - JSONCPP_STRING const& nullSymbol, - JSONCPP_STRING const& endingLineFeedSymbol, + JSONCPP_STRING colonSymbol, + JSONCPP_STRING nullSymbol, + JSONCPP_STRING endingLineFeedSymbol, bool useSpecialFloats, unsigned int precision, PrecisionType precisionType); @@ -912,17 +912,17 @@ struct BuiltStyledStreamWriter : public StreamWriter { PrecisionType precisionType_; }; BuiltStyledStreamWriter::BuiltStyledStreamWriter( - JSONCPP_STRING const& indentation, + JSONCPP_STRING indentation, CommentStyle::Enum cs, - JSONCPP_STRING const& colonSymbol, - JSONCPP_STRING const& nullSymbol, - JSONCPP_STRING const& endingLineFeedSymbol, + JSONCPP_STRING colonSymbol, + JSONCPP_STRING nullSymbol, + JSONCPP_STRING endingLineFeedSymbol, bool useSpecialFloats, unsigned int precision, PrecisionType precisionType) - : rightMargin_(74), indentation_(indentation), cs_(cs), - colonSymbol_(colonSymbol), nullSymbol_(nullSymbol), - endingLineFeedSymbol_(endingLineFeedSymbol), addChildValues_(false), + : rightMargin_(74), indentation_(std::move(indentation)), cs_(cs), + colonSymbol_(std::move(colonSymbol)), nullSymbol_(std::move(nullSymbol)), + endingLineFeedSymbol_(std::move(endingLineFeedSymbol)), addChildValues_(false), indented_(false), useSpecialFloats_(useSpecialFloats), precision_(precision), precisionType_(precisionType) {} int BuiltStyledStreamWriter::write(Value const& root, JSONCPP_OSTREAM* sout) { From 7b30993baa401158330d31cab04d0c93fde356f3 Mon Sep 17 00:00:00 2001 From: Hans Johnson Date: Mon, 14 Jan 2019 17:09:22 -0600 Subject: [PATCH 5/8] STYLE: Use default member initialization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Converts a default constructor’s member initializers into the new default member initializers in C++11. Other member initializers that match the default member initializer are removed. This can reduce repeated code or allow use of ‘= default’. SRCDIR=/Users/johnsonhj/src/jsoncpp/ #My local SRC BLDDIR=/Users/johnsonhj/src/jsoncpp/cmake-build-debug/ #My local BLD cd /Users/johnsonhj/src/jsoncpp/cmake-build-debug/ run-clang-tidy.py -extra-arg=-D__clang__ -checks=-*,modernize-use-default-member-init -header-filter=.* -fix --- include/json/features.h | 8 ++++---- include/json/reader.h | 12 +++++------ include/json/value.h | 8 ++++---- include/json/writer.h | 14 ++++++------- src/lib_json/json_reader.cpp | 3 +-- src/lib_json/json_value.cpp | 4 ++-- src/lib_json/json_valueiterator.inl | 2 +- src/lib_json/json_writer.cpp | 6 +++--- src/test_lib_json/jsontest.cpp | 4 ++-- src/test_lib_json/jsontest.h | 8 ++++---- src/test_lib_json/main.cpp | 32 ++++++++++++++--------------- 11 files changed, 49 insertions(+), 52 deletions(-) diff --git a/include/json/features.h b/include/json/features.h index 72eb6a8a0..ba25e8da6 100644 --- a/include/json/features.h +++ b/include/json/features.h @@ -41,17 +41,17 @@ class JSON_API Features { Features(); /// \c true if comments are allowed. Default: \c true. - bool allowComments_; + bool allowComments_{true}; /// \c true if root must be either an array or an object value. Default: \c /// false. - bool strictRoot_; + bool strictRoot_{false}; /// \c true if dropped null placeholders are allowed. Default: \c false. - bool allowDroppedNullPlaceholders_; + bool allowDroppedNullPlaceholders_{false}; /// \c true if numeric object key are allowed. Default: \c false. - bool allowNumericKeys_; + bool allowNumericKeys_{false}; }; } // namespace Json diff --git a/include/json/reader.h b/include/json/reader.h index 58cb58b57..1af5cb0dd 100644 --- a/include/json/reader.h +++ b/include/json/reader.h @@ -242,14 +242,14 @@ class JSON_API Reader { Nodes nodes_; Errors errors_; JSONCPP_STRING document_; - Location begin_; - Location end_; - Location current_; - Location lastValueEnd_; - Value* lastValue_; + Location begin_{}; + Location end_{}; + Location current_{}; + Location lastValueEnd_{}; + Value* lastValue_{}; JSONCPP_STRING commentsBefore_; Features features_; - bool collectComments_; + bool collectComments_{}; }; // Reader /** Interface for reading JSON from a char array. diff --git a/include/json/value.h b/include/json/value.h index 71bc65c0b..8321de7bf 100644 --- a/include/json/value.h +++ b/include/json/value.h @@ -630,7 +630,7 @@ Json::Value obj_value(Json::objectValue); // {} void setComment(const char* text, size_t len); - char* comment_; + char* comment_{nullptr}; }; // struct MemberNamesTransform @@ -678,8 +678,8 @@ class JSON_API PathArgument { private: enum Kind { kindNone = 0, kindIndex, kindKey }; JSONCPP_STRING key_; - ArrayIndex index_; - Kind kind_; + ArrayIndex index_{}; + Kind kind_{kindNone}; }; /** \brief Experimental and untested: represents a "path" to access a node. @@ -780,7 +780,7 @@ class JSON_API ValueIteratorBase { private: Value::ObjectValues::iterator current_; // Indicates that iterator is for a null value. - bool isNull_; + bool isNull_{true}; public: // For some reason, BORLAND needs these at the end, rather diff --git a/include/json/writer.h b/include/json/writer.h index 34e71edac..165326060 100644 --- a/include/json/writer.h +++ b/include/json/writer.h @@ -189,9 +189,9 @@ class JSONCPP_DEPRECATED("Use StreamWriterBuilder instead") JSON_API FastWriter void writeValue(const Value& value); JSONCPP_STRING document_; - bool yamlCompatibilityEnabled_; - bool dropNullPlaceholders_; - bool omitEndingLineFeed_; + bool yamlCompatibilityEnabled_{false}; + bool dropNullPlaceholders_{false}; + bool omitEndingLineFeed_{false}; }; #if defined(_MSC_VER) #pragma warning(pop) @@ -257,9 +257,9 @@ class JSONCPP_DEPRECATED("Use StreamWriterBuilder instead") JSON_API ChildValues childValues_; JSONCPP_STRING document_; JSONCPP_STRING indentString_; - unsigned int rightMargin_; - unsigned int indentSize_; - bool addChildValues_; + unsigned int rightMargin_{74}; + unsigned int indentSize_{3}; + bool addChildValues_{false}; }; #if defined(_MSC_VER) #pragma warning(pop) @@ -331,7 +331,7 @@ class JSONCPP_DEPRECATED("Use StreamWriterBuilder instead") JSON_API ChildValues childValues_; JSONCPP_OSTREAM* document_; JSONCPP_STRING indentString_; - unsigned int rightMargin_; + unsigned int rightMargin_{74}; JSONCPP_STRING indentation_; bool addChildValues_ : 1; bool indented_ : 1; diff --git a/src/lib_json/json_reader.cpp b/src/lib_json/json_reader.cpp index c5af01468..e1dc21237 100644 --- a/src/lib_json/json_reader.cpp +++ b/src/lib_json/json_reader.cpp @@ -60,8 +60,7 @@ typedef std::auto_ptr CharReaderPtr; // //////////////////////////////// Features::Features() - : allowComments_(true), strictRoot_(false), - allowDroppedNullPlaceholders_(false), allowNumericKeys_(false) {} + {} Features Features::all() { return {}; } diff --git a/src/lib_json/json_value.cpp b/src/lib_json/json_value.cpp index 2445d31a8..76c14a29d 100644 --- a/src/lib_json/json_value.cpp +++ b/src/lib_json/json_value.cpp @@ -233,7 +233,7 @@ JSONCPP_NORETURN void throwLogicError(JSONCPP_STRING const& msg) { // ////////////////////////////////////////////////////////////////// // ////////////////////////////////////////////////////////////////// -Value::CommentInfo::CommentInfo() : comment_(nullptr) {} +Value::CommentInfo::CommentInfo() {} Value::CommentInfo::~CommentInfo() { if (comment_) @@ -1575,7 +1575,7 @@ Value::iterator Value::end() { // class PathArgument // ////////////////////////////////////////////////////////////////// -PathArgument::PathArgument() : key_(), index_(), kind_(kindNone) {} +PathArgument::PathArgument() : key_() {} PathArgument::PathArgument(ArrayIndex index) : key_(), index_(index), kind_(kindIndex) {} diff --git a/src/lib_json/json_valueiterator.inl b/src/lib_json/json_valueiterator.inl index 7cc379227..4a2fc2e65 100644 --- a/src/lib_json/json_valueiterator.inl +++ b/src/lib_json/json_valueiterator.inl @@ -16,7 +16,7 @@ namespace Json { // ////////////////////////////////////////////////////////////////// ValueIteratorBase::ValueIteratorBase() - : current_(), isNull_(true) { + : current_() { } ValueIteratorBase::ValueIteratorBase( diff --git a/src/lib_json/json_writer.cpp b/src/lib_json/json_writer.cpp index 54fcb4fa1..02d9a74f4 100644 --- a/src/lib_json/json_writer.cpp +++ b/src/lib_json/json_writer.cpp @@ -352,8 +352,8 @@ Writer::~Writer() {} // ////////////////////////////////////////////////////////////////// FastWriter::FastWriter() - : yamlCompatibilityEnabled_(false), dropNullPlaceholders_(false), - omitEndingLineFeed_(false) {} + + {} void FastWriter::enableYAMLCompatibility() { yamlCompatibilityEnabled_ = true; } @@ -428,7 +428,7 @@ void FastWriter::writeValue(const Value& value) { // ////////////////////////////////////////////////////////////////// StyledWriter::StyledWriter() - : rightMargin_(74), indentSize_(3), addChildValues_(false) {} + {} JSONCPP_STRING StyledWriter::write(const Value& root) { document_.clear(); diff --git a/src/test_lib_json/jsontest.cpp b/src/test_lib_json/jsontest.cpp index 03703c9c4..0329eeab9 100644 --- a/src/test_lib_json/jsontest.cpp +++ b/src/test_lib_json/jsontest.cpp @@ -74,7 +74,7 @@ namespace JsonTest { // ////////////////////////////////////////////////////////////////// TestResult::TestResult() - : predicateId_(1), lastUsedPredicateId_(0), messageTarget_(nullptr) { + { // The root predicate has id 0 rootPredicateNode_.id_ = 0; rootPredicateNode_.next_ = nullptr; @@ -205,7 +205,7 @@ TestResult& TestResult::operator<<(bool value) { // class TestCase // ////////////////////////////////////////////////////////////////// -TestCase::TestCase() : result_(nullptr) {} +TestCase::TestCase() {} TestCase::~TestCase() {} diff --git a/src/test_lib_json/jsontest.h b/src/test_lib_json/jsontest.h index e508282b6..76504228e 100644 --- a/src/test_lib_json/jsontest.h +++ b/src/test_lib_json/jsontest.h @@ -60,7 +60,7 @@ class TestResult { /// Not encapsulated to prevent step into when debugging failed assertions /// Incremented by one on assertion predicate entry, decreased by one /// by addPredicateContext(). - PredicateContext::Id predicateId_; + PredicateContext::Id predicateId_{1}; /// \internal Implementation detail for predicate macros PredicateContext* predicateStackTail_; @@ -109,9 +109,9 @@ class TestResult { Failures failures_; JSONCPP_STRING name_; PredicateContext rootPredicateNode_; - PredicateContext::Id lastUsedPredicateId_; + PredicateContext::Id lastUsedPredicateId_{0}; /// Failure which is the target of the messages added using operator << - Failure* messageTarget_; + Failure* messageTarget_{nullptr}; }; class TestCase { @@ -125,7 +125,7 @@ class TestCase { virtual const char* testName() const = 0; protected: - TestResult* result_; + TestResult* result_{nullptr}; private: virtual void runTestCase() = 0; diff --git a/src/test_lib_json/main.cpp b/src/test_lib_json/main.cpp index d0315d7b2..b4f3f597a 100644 --- a/src/test_lib_json/main.cpp +++ b/src/test_lib_json/main.cpp @@ -82,19 +82,19 @@ struct ValueTest : JsonTest::TestCase { /// Initialize all checks to \c false by default. IsCheck(); - bool isObject_; - bool isArray_; - bool isBool_; - bool isString_; - bool isNull_; - - bool isInt_; - bool isInt64_; - bool isUInt_; - bool isUInt64_; - bool isIntegral_; - bool isDouble_; - bool isNumeric_; + bool isObject_{false}; + bool isArray_{false}; + bool isBool_{false}; + bool isString_{false}; + bool isNull_{false}; + + bool isInt_{false}; + bool isInt64_{false}; + bool isUInt_{false}; + bool isUInt64_{false}; + bool isIntegral_{false}; + bool isDouble_{false}; + bool isNumeric_{false}; }; void checkConstMemberCount(const Json::Value& value, @@ -1331,10 +1331,8 @@ void ValueTest::checkMemberCount(Json::Value& value, } ValueTest::IsCheck::IsCheck() - : isObject_(false), isArray_(false), isBool_(false), isString_(false), - isNull_(false), isInt_(false), isInt64_(false), isUInt_(false), - isUInt64_(false), isIntegral_(false), isDouble_(false), - isNumeric_(false) {} + + {} void ValueTest::checkIs(const Json::Value& value, const IsCheck& check) { JSONTEST_ASSERT_EQUAL(check.isObject_, value.isObject()); From 22eba75ac55aa08105d08895952814577f71921f Mon Sep 17 00:00:00 2001 From: Hans Johnson Date: Mon, 14 Jan 2019 17:09:26 -0600 Subject: [PATCH 6/8] STYLE: Pefer = default to explicitly trivial implementations This check replaces default bodies of special member functions with = default;. The explicitly defaulted function declarations enable more opportunities in optimization, because the compiler might treat explicitly defaulted functions as trivial. Additionally, the C++11 use of = default more clearly expreses the intent for the special member functions. SRCDIR=/Users/johnsonhj/src/jsoncpp/ #My local SRC BLDDIR=/Users/johnsonhj/src/jsoncpp/cmake-build-debug/ #My local BLD cd /Users/johnsonhj/src/jsoncpp/cmake-build-debug/ run-clang-tidy.py -extra-arg=-D__clang__ -checks=-*,modernize-use-equals-default -header-filter=.* -fix --- include/json/reader.h | 4 ++-- include/json/writer.h | 4 ++-- src/lib_json/json_reader.cpp | 4 ++-- src/lib_json/json_value.cpp | 2 +- src/lib_json/json_valueiterator.inl | 6 +++--- src/lib_json/json_writer.cpp | 12 ++++++------ src/test_lib_json/jsontest.cpp | 6 +++--- src/test_lib_json/main.cpp | 2 +- 8 files changed, 20 insertions(+), 20 deletions(-) diff --git a/include/json/reader.h b/include/json/reader.h index 1af5cb0dd..ac8b7bd80 100644 --- a/include/json/reader.h +++ b/include/json/reader.h @@ -256,7 +256,7 @@ class JSON_API Reader { */ class JSON_API CharReader { public: - virtual ~CharReader() {} + virtual ~CharReader() = default; /** \brief Read a Value from a JSON document. * The document must be a UTF-8 encoded string containing the document to @@ -282,7 +282,7 @@ class JSON_API CharReader { class JSON_API Factory { public: - virtual ~Factory() {} + virtual ~Factory() = default; /** \brief Allocate a CharReader via operator new(). * \throw std::exception if something goes wrong (e.g. invalid settings) */ diff --git a/include/json/writer.h b/include/json/writer.h index 165326060..10a1c8e11 100644 --- a/include/json/writer.h +++ b/include/json/writer.h @@ -169,7 +169,7 @@ class JSONCPP_DEPRECATED("Use StreamWriterBuilder instead") JSON_API FastWriter : public Writer { public: FastWriter(); - ~FastWriter() override {} + ~FastWriter() override = default; void enableYAMLCompatibility(); @@ -229,7 +229,7 @@ class JSONCPP_DEPRECATED("Use StreamWriterBuilder instead") JSON_API StyledWriter : public Writer { public: StyledWriter(); - ~StyledWriter() override {} + ~StyledWriter() override = default; public: // overridden from Writer /** \brief Serialize a Value in JSON format. diff --git a/src/lib_json/json_reader.cpp b/src/lib_json/json_reader.cpp index e1dc21237..fc6b1397d 100644 --- a/src/lib_json/json_reader.cpp +++ b/src/lib_json/json_reader.cpp @@ -60,7 +60,7 @@ typedef std::auto_ptr CharReaderPtr; // //////////////////////////////// Features::Features() - {} + = default; Features Features::all() { return {}; } @@ -1907,7 +1907,7 @@ class OurCharReader : public CharReader { }; CharReaderBuilder::CharReaderBuilder() { setDefaults(&settings_); } -CharReaderBuilder::~CharReaderBuilder() {} +CharReaderBuilder::~CharReaderBuilder() = default; CharReader* CharReaderBuilder::newCharReader() const { bool collectComments = settings_["collectComments"].asBool(); OurFeatures features = OurFeatures::all(); diff --git a/src/lib_json/json_value.cpp b/src/lib_json/json_value.cpp index 76c14a29d..f05d7ae0e 100644 --- a/src/lib_json/json_value.cpp +++ b/src/lib_json/json_value.cpp @@ -233,7 +233,7 @@ JSONCPP_NORETURN void throwLogicError(JSONCPP_STRING const& msg) { // ////////////////////////////////////////////////////////////////// // ////////////////////////////////////////////////////////////////// -Value::CommentInfo::CommentInfo() {} +Value::CommentInfo::CommentInfo() = default; Value::CommentInfo::~CommentInfo() { if (comment_) diff --git a/src/lib_json/json_valueiterator.inl b/src/lib_json/json_valueiterator.inl index 4a2fc2e65..21d0611de 100644 --- a/src/lib_json/json_valueiterator.inl +++ b/src/lib_json/json_valueiterator.inl @@ -123,7 +123,7 @@ char const* ValueIteratorBase::memberName(char const** end) const { // ////////////////////////////////////////////////////////////////// // ////////////////////////////////////////////////////////////////// -ValueConstIterator::ValueConstIterator() {} +ValueConstIterator::ValueConstIterator() = default; ValueConstIterator::ValueConstIterator( const Value::ObjectValues::iterator& current) @@ -146,7 +146,7 @@ operator=(const ValueIteratorBase& other) { // ////////////////////////////////////////////////////////////////// // ////////////////////////////////////////////////////////////////// -ValueIterator::ValueIterator() {} +ValueIterator::ValueIterator() = default; ValueIterator::ValueIterator(const Value::ObjectValues::iterator& current) : ValueIteratorBase(current) {} @@ -157,7 +157,7 @@ ValueIterator::ValueIterator(const ValueConstIterator& other) } ValueIterator::ValueIterator(const ValueIterator& other) - : ValueIteratorBase(other) {} + = default; ValueIterator& ValueIterator::operator=(const SelfType& other) { copy(other); diff --git a/src/lib_json/json_writer.cpp b/src/lib_json/json_writer.cpp index 02d9a74f4..406d4bc26 100644 --- a/src/lib_json/json_writer.cpp +++ b/src/lib_json/json_writer.cpp @@ -346,14 +346,14 @@ JSONCPP_STRING valueToQuotedString(const char* value) { // Class Writer // ////////////////////////////////////////////////////////////////// -Writer::~Writer() {} +Writer::~Writer() = default; // Class FastWriter // ////////////////////////////////////////////////////////////////// FastWriter::FastWriter() - {} + = default; void FastWriter::enableYAMLCompatibility() { yamlCompatibilityEnabled_ = true; } @@ -428,7 +428,7 @@ void FastWriter::writeValue(const Value& value) { // ////////////////////////////////////////////////////////////////// StyledWriter::StyledWriter() - {} + = default; JSONCPP_STRING StyledWriter::write(const Value& root) { document_.clear(); @@ -1156,10 +1156,10 @@ bool BuiltStyledStreamWriter::hasCommentForValue(const Value& value) { // StreamWriter StreamWriter::StreamWriter() : sout_(nullptr) {} -StreamWriter::~StreamWriter() {} -StreamWriter::Factory::~Factory() {} +StreamWriter::~StreamWriter() = default; +StreamWriter::Factory::~Factory() = default; StreamWriterBuilder::StreamWriterBuilder() { setDefaults(&settings_); } -StreamWriterBuilder::~StreamWriterBuilder() {} +StreamWriterBuilder::~StreamWriterBuilder() = default; StreamWriter* StreamWriterBuilder::newStreamWriter() const { JSONCPP_STRING indentation = settings_["indentation"].asString(); JSONCPP_STRING cs_str = settings_["commentStyle"].asString(); diff --git a/src/test_lib_json/jsontest.cpp b/src/test_lib_json/jsontest.cpp index 0329eeab9..369ca9fce 100644 --- a/src/test_lib_json/jsontest.cpp +++ b/src/test_lib_json/jsontest.cpp @@ -205,9 +205,9 @@ TestResult& TestResult::operator<<(bool value) { // class TestCase // ////////////////////////////////////////////////////////////////// -TestCase::TestCase() {} +TestCase::TestCase() = default; -TestCase::~TestCase() {} +TestCase::~TestCase() = default; void TestCase::run(TestResult& result) { result_ = &result; @@ -217,7 +217,7 @@ void TestCase::run(TestResult& result) { // class Runner // ////////////////////////////////////////////////////////////////// -Runner::Runner() {} +Runner::Runner() = default; Runner& Runner::add(TestCaseFactory factory) { tests_.push_back(factory); diff --git a/src/test_lib_json/main.cpp b/src/test_lib_json/main.cpp index b4f3f597a..b787a98a0 100644 --- a/src/test_lib_json/main.cpp +++ b/src/test_lib_json/main.cpp @@ -1332,7 +1332,7 @@ void ValueTest::checkMemberCount(Json::Value& value, ValueTest::IsCheck::IsCheck() - {} + = default; void ValueTest::checkIs(const Json::Value& value, const IsCheck& check) { JSONTEST_ASSERT_EQUAL(check.isObject_, value.isObject()); From 4ec678c0151f3e2e382c4a6daca4f7ab314c9a7f Mon Sep 17 00:00:00 2001 From: Hans Johnson Date: Mon, 14 Jan 2019 17:09:29 -0600 Subject: [PATCH 7/8] STYLE: Pefer = delete to explicitly trivial implementations This check replaces undefined special member functions with = delete;. The explicitly deleted function declarations enable more opportunities in optimization, because the compiler might treat explicitly delted functions as noops. Additionally, the C++11 use of = delete more clearly expreses the intent for the special member functions. SRCDIR=/Users/johnsonhj/src/jsoncpp/ #My local SRC BLDDIR=/Users/johnsonhj/src/jsoncpp/cmake-build-debug/ #My local BLD cd /Users/johnsonhj/src/jsoncpp/cmake-build-debug/ run-clang-tidy.py -extra-arg=-D__clang__ -checks=-*,modernize-use-equals-delete -header-filter=.* -fix --- src/test_lib_json/jsontest.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test_lib_json/jsontest.h b/src/test_lib_json/jsontest.h index 76504228e..85952a580 100644 --- a/src/test_lib_json/jsontest.h +++ b/src/test_lib_json/jsontest.h @@ -162,8 +162,8 @@ class Runner { static void printUsage(const char* appName); private: // prevents copy construction and assignment - Runner(const Runner& other); - Runner& operator=(const Runner& other); + Runner(const Runner& other) = delete; + Runner& operator=(const Runner& other) = delete; private: void listTests() const; From 67faffd400d87c2f9f8d96839bfef3f14379c373 Mon Sep 17 00:00:00 2001 From: Hans Johnson Date: Mon, 14 Jan 2019 19:02:40 -0600 Subject: [PATCH 8/8] STYLE: Avoid unnecessary conversions from size_t to unsigned int Make the index values consistent with size_t. --- src/lib_json/json_reader.cpp | 10 +++++++--- src/test_lib_json/jsontest.cpp | 34 +++++++++++++++++----------------- src/test_lib_json/jsontest.h | 8 ++++---- 3 files changed, 28 insertions(+), 24 deletions(-) diff --git a/src/lib_json/json_reader.cpp b/src/lib_json/json_reader.cpp index fc6b1397d..56acc6ae3 100644 --- a/src/lib_json/json_reader.cpp +++ b/src/lib_json/json_reader.cpp @@ -888,7 +888,7 @@ class OurFeatures { bool failIfExtra_; bool rejectDupKeys_; bool allowSpecialFloats_; - int stackLimit_; + size_t stackLimit_; }; // OurFeatures // exact copy of Implementation of class Features @@ -1087,7 +1087,7 @@ bool OurReader::parse(const char* beginDoc, bool OurReader::readValue() { // To preserve the old behaviour we cast size_t to int. - if (static_cast(nodes_.size()) > features_.stackLimit_) + if (nodes_.size() > features_.stackLimit_) throwRuntimeError("Exceeded stackLimit in readValue()."); Token token; skipCommentTokens(token); @@ -1917,7 +1917,11 @@ CharReader* CharReaderBuilder::newCharReader() const { settings_["allowDroppedNullPlaceholders"].asBool(); features.allowNumericKeys_ = settings_["allowNumericKeys"].asBool(); features.allowSingleQuotes_ = settings_["allowSingleQuotes"].asBool(); - features.stackLimit_ = settings_["stackLimit"].asInt(); +#if defined(JSON_HAS_INT64) + features.stackLimit_ = settings_["stackLimit"].asUInt64(); +#else + features.stackLimit_ = settings_["stackLimit"].asUInt(); +#endif features.failIfExtra_ = settings_["failIfExtra"].asBool(); features.rejectDupKeys_ = settings_["rejectDupKeys"].asBool(); features.allowSpecialFloats_ = settings_["allowSpecialFloats"].asBool(); diff --git a/src/test_lib_json/jsontest.cpp b/src/test_lib_json/jsontest.cpp index 369ca9fce..1545e0333 100644 --- a/src/test_lib_json/jsontest.cpp +++ b/src/test_lib_json/jsontest.cpp @@ -224,18 +224,18 @@ Runner& Runner::add(TestCaseFactory factory) { return *this; } -unsigned int Runner::testCount() const { - return static_cast(tests_.size()); +size_t Runner::testCount() const { + return tests_.size(); } -JSONCPP_STRING Runner::testNameAt(unsigned int index) const { +JSONCPP_STRING Runner::testNameAt(size_t index) const { TestCase* test = tests_[index](); JSONCPP_STRING name = test->testName(); delete test; return name; } -void Runner::runTestAt(unsigned int index, TestResult& result) const { +void Runner::runTestAt(size_t index, TestResult& result) const { TestCase* test = tests_[index](); result.setTestName(test->testName()); printf("Testing %s: ", test->testName()); @@ -257,9 +257,9 @@ void Runner::runTestAt(unsigned int index, TestResult& result) const { } bool Runner::runAllTest(bool printSummary) const { - unsigned int count = testCount(); + size_t const count = testCount(); std::deque failures; - for (unsigned int index = 0; index < count; ++index) { + for (size_t index = 0; index < count; ++index) { TestResult result; runTestAt(index, result); if (result.failed()) { @@ -269,7 +269,7 @@ bool Runner::runAllTest(bool printSummary) const { if (failures.empty()) { if (printSummary) { - printf("All %u tests passed\n", count); + printf("All %zu tests passed\n", count); } return true; } else { @@ -278,19 +278,19 @@ bool Runner::runAllTest(bool printSummary) const { } if (printSummary) { - auto failedCount = static_cast(failures.size()); - unsigned int passedCount = count - failedCount; - printf("%u/%u tests passed (%u failure(s))\n", passedCount, count, - failedCount); + size_t const failedCount = failures.size(); + size_t const passedCount = count - failedCount; + printf("%zu/%zu tests passed (%zu failure(s))\n", + passedCount, count, failedCount); } return false; } } bool Runner::testIndex(const JSONCPP_STRING& testName, - unsigned int& indexOut) const { - unsigned int count = testCount(); - for (unsigned int index = 0; index < count; ++index) { + size_t& indexOut) const { + const size_t count = testCount(); + for (size_t index = 0; index < count; ++index) { if (testNameAt(index) == testName) { indexOut = index; return true; @@ -300,8 +300,8 @@ bool Runner::testIndex(const JSONCPP_STRING& testName, } void Runner::listTests() const { - unsigned int count = testCount(); - for (unsigned int index = 0; index < count; ++index) { + const size_t count = testCount(); + for (size_t index = 0; index < count; ++index) { printf("%s\n", testNameAt(index).c_str()); } } @@ -319,7 +319,7 @@ int Runner::runCommandLine(int argc, const char* argv[]) const { } else if (opt == "--test") { ++index; if (index < argc) { - unsigned int testNameIndex; + size_t testNameIndex; if (testIndex(argv[index], testNameIndex)) { subrunner.add(tests_[testNameIndex]); } else { diff --git a/src/test_lib_json/jsontest.h b/src/test_lib_json/jsontest.h index 85952a580..57d42ba6e 100644 --- a/src/test_lib_json/jsontest.h +++ b/src/test_lib_json/jsontest.h @@ -151,13 +151,13 @@ class Runner { bool runAllTest(bool printSummary) const; /// Returns the number of test case in the suite - unsigned int testCount() const; + size_t testCount() const; /// Returns the name of the test case at the specified index - JSONCPP_STRING testNameAt(unsigned int index) const; + JSONCPP_STRING testNameAt(size_t index) const; /// Runs the test case at the specified index using the specified TestResult - void runTestAt(unsigned int index, TestResult& result) const; + void runTestAt(size_t index, TestResult& result) const; static void printUsage(const char* appName); @@ -167,7 +167,7 @@ class Runner { private: void listTests() const; - bool testIndex(const JSONCPP_STRING& testName, unsigned int& indexOut) const; + bool testIndex(const JSONCPP_STRING& testName, size_t& indexOut) const; static void preventDialogOnCrash(); private: