From 9f1854acf8a13de98b5706d1246baa15f15f377e Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Tue, 18 Jan 2022 10:36:33 +0100 Subject: [PATCH] JSON: Further improve the duplicate handling in the parser Avoid some unnecessary comparisons and add more tests. Task-number: QTBUG-99799 Change-Id: I3aee9f0b62461d38dadbe8e969444e1cd1f94e68 Reviewed-by: Sona Kurazyan --- src/corelib/serialization/qjsonparser.cpp | 18 ++++- .../corelib/serialization/json/CMakeLists.txt | 3 + .../serialization/json/simple.duplicates.json | 1 + .../serialization/json/test.duplicates.json | 66 +++++++++++++++++++ .../serialization/json/test3.duplicates.json | 15 +++++ .../corelib/serialization/json/tst_qtjson.cpp | 48 +++++++++++--- 6 files changed, 138 insertions(+), 13 deletions(-) create mode 100644 tests/auto/corelib/serialization/json/simple.duplicates.json create mode 100644 tests/auto/corelib/serialization/json/test.duplicates.json create mode 100644 tests/auto/corelib/serialization/json/test3.duplicates.json diff --git a/src/corelib/serialization/qjsonparser.cpp b/src/corelib/serialization/qjsonparser.cpp index df66fc2772e..d8d0ad9c2e2 100644 --- a/src/corelib/serialization/qjsonparser.cpp +++ b/src/corelib/serialization/qjsonparser.cpp @@ -389,12 +389,24 @@ static Iterator customAssigningUniqueLast(Iterator first, Iterator last, if (first == last) return last; - Iterator result = first; + // After adjacent_find, we know that *first and *(first+1) compare equal, + // and that first+1 != last. + Iterator result = first++; + Q_ASSERT(compare(*result, *first)); + assign(*result, *first); + Q_ASSERT(first != last); + while (++first != last) { if (!compare(*result, *first)) ++result; - if (result != first) - assign(*result, *first); + + // Due to adjacent_find above, we know that we've at least eliminated one element. + // Therefore we have to move each further element across the gap. + Q_ASSERT(result != first); + + // We have to overwrite each element we want to eliminate, to deref() the container. + // Therefore we don't try to optimize the number of assignments here. + assign(*result, *first); } return ++result; diff --git a/tests/auto/corelib/serialization/json/CMakeLists.txt b/tests/auto/corelib/serialization/json/CMakeLists.txt index 77d15bedc31..9022ed9f3d1 100644 --- a/tests/auto/corelib/serialization/json/CMakeLists.txt +++ b/tests/auto/corelib/serialization/json/CMakeLists.txt @@ -24,6 +24,9 @@ if(ANDROID OR INTEGRITY) "test.json" "test2.json" "test3.json" + "simple.duplicates.json" + "test.duplicates.json" + "test3.duplicates.json" ) qt_internal_add_resource(tst_json "json" diff --git a/tests/auto/corelib/serialization/json/simple.duplicates.json b/tests/auto/corelib/serialization/json/simple.duplicates.json new file mode 100644 index 00000000000..6f989e8aa9b --- /dev/null +++ b/tests/auto/corelib/serialization/json/simple.duplicates.json @@ -0,0 +1 @@ +{"":{"":0},"":0} diff --git a/tests/auto/corelib/serialization/json/test.duplicates.json b/tests/auto/corelib/serialization/json/test.duplicates.json new file mode 100644 index 00000000000..0d5af8ef749 --- /dev/null +++ b/tests/auto/corelib/serialization/json/test.duplicates.json @@ -0,0 +1,66 @@ +[ + "JSON Test Pattern pass1", + {"a":["array with 1 element"]}, + {}, + [], + -42, + true, + false, + null, + { + "a": 1234567890, + "a": -9876.543210, + "a": 0.123456789e-12, + "a": 1.234567890E+34, + "a": 23456789012E66, + "a": 0, + "a": 1, + "a": " ", + "a": "\"", + "a": "\\", + "a": "\b\f\n\r\t", + "a": "/ & \/", + "a": "abcdefghijklmnopqrstuvwxyz", + "a": "ABCDEFGHIJKLMNOPQRSTUVWXYZ", + "a": "0123456789", + "a": "digit", + "a": "`1~!@#$%^&*()_+-={':[,]}|;.?", + "a": "\u0123\u4567\u89AB\uCDEF\uabcd\uef4A", + "a": true, + "a": false, + "a": null, + "a":[ ], + "a":{ }, + "a": "50 St. James Street", + "a": "nix", + "a": "// /*