From ed94fe52bf56a02ee3a1863ad0c14c43df440acc Mon Sep 17 00:00:00 2001 From: James M Snell Date: Tue, 10 Oct 2023 07:38:40 -0700 Subject: [PATCH] fixup! Add iterator support to url_search_params and ada_c --- README.md | 2 +- fuzz/parse.cc | 6 +++--- include/ada/url_search_params-inl.h | 10 +++++----- include/ada/url_search_params.h | 25 +++++++++++++++++++++---- include/ada_c.h | 6 +++--- src/ada_c.cpp | 12 ++++++------ tests/ada_c.cpp | 12 ++++++------ tests/url_search_params.cpp | 16 ++++++++++++++++ 8 files changed, 61 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index 926065189..6f3cdbc36 100644 --- a/README.md +++ b/README.md @@ -211,7 +211,7 @@ search_params.append("g=h"); search_params.get("g"); // will return "h" auto keys = search_params.get_keys(); -while (!keys.is_done()) { +while (keys.has_next()) { auto key = keys.next(); // "a", "c", "e", "g" } ``` diff --git a/fuzz/parse.cc b/fuzz/parse.cc index 42bd492e8..8041093d1 100644 --- a/fuzz/parse.cc +++ b/fuzz/parse.cc @@ -122,17 +122,17 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { } auto keys = search_params.get_keys(); - while (!keys.is_done()) { + while (keys.has_next()) { keys.next(); } auto values = search_params.get_values(); - while (!values.is_done()) { + while (values.has_next()) { values.next(); } auto entries = search_params.get_entries(); - while (!entries.is_done()) { + while (entries.has_next()) { entries.next(); } diff --git a/include/ada/url_search_params-inl.h b/include/ada/url_search_params-inl.h index c6d3a884a..a7ca01f86 100644 --- a/include/ada/url_search_params-inl.h +++ b/include/ada/url_search_params-inl.h @@ -184,26 +184,26 @@ inline url_search_params_entries_iter url_search_params::get_entries() { } template -inline bool url_search_params_iter::is_done() { - return pos >= params.params.size(); +inline bool url_search_params_iter::has_next() { + return pos < params.params.size(); } template <> inline std::optional url_search_params_keys_iter::next() { - if (is_done()) return std::nullopt; + if (!has_next()) return std::nullopt; return params.params[pos++].first; } template <> inline std::optional url_search_params_values_iter::next() { - if (is_done()) return std::nullopt; + if (!has_next()) return std::nullopt; return params.params[pos++].second; } template <> inline std::optional url_search_params_entries_iter::next() { - if (is_done()) return std::nullopt; + if (!has_next()) return std::nullopt; return params.params[pos++]; } diff --git a/include/ada/url_search_params.h b/include/ada/url_search_params.h index 8e6708c7e..b7950d5df 100644 --- a/include/ada/url_search_params.h +++ b/include/ada/url_search_params.h @@ -95,7 +95,7 @@ struct url_search_params { inline std::string to_string(); /** - * Returns a simple iterator over all of the keys in this url_search_params. + * Returns a simple JS-style iterator over all of the keys in this url_search_params. * The keys in the iterator are not unique. The valid lifespan of the iterator * is tied to the url_search_params. The iterator must be freed when you're * done with it. @@ -104,7 +104,7 @@ struct url_search_params { inline url_search_params_keys_iter get_keys(); /** - * Returns a simple iterator over all of the values in this url_search_params. + * Returns a simple JS-style iterator over all of the values in this url_search_params. * The valid lifespan of the iterator is tied to the url_search_params. The * iterator must be freed when you're done with it. * @see https://url.spec.whatwg.org/#interface-urlsearchparams @@ -112,7 +112,7 @@ struct url_search_params { inline url_search_params_values_iter get_values(); /** - * Returns a simple iterator over all of the entries in this + * Returns a simple JS-style iterator over all of the entries in this * url_search_params. The entries are pairs of keys and corresponding values. * The valid lifespan of the iterator is tied to the url_search_params. The * iterator must be freed when you're done with it. @@ -120,6 +120,16 @@ struct url_search_params { */ inline url_search_params_entries_iter get_entries(); + /** + * C++ style conventional iterator support. const only because we + * do not really want the params to be modified via the iterator. + */ + inline const auto begin() const { return params.begin(); } + inline const auto end() const { return params.end(); } + inline const auto front() const { return params.front(); } + inline const auto back() const { return params.back(); } + inline const auto operator[](size_t index) const { return params[index]; } + private: typedef std::pair key_value_pair; std::vector params{}; @@ -133,6 +143,12 @@ struct url_search_params { friend struct url_search_params_iter; }; // url_search_params +/** + * Implements a non-conventional iterator pattern that is closer in style to + * JavaScript's definition of an iterator. + * + * @see https://webidl.spec.whatwg.org/#idl-iterable + */ template struct url_search_params_iter { url_search_params_iter(const url_search_params_iter &u) = default; @@ -146,7 +162,8 @@ struct url_search_params_iter { * Return the next item in the iterator or std::nullopt if done. */ inline std::optional next(); - inline bool is_done(); + + inline bool has_next(); private: inline url_search_params_iter(url_search_params ¶ms_) : params(params_) {} diff --git a/include/ada_c.h b/include/ada_c.h index 1574cfab4..173e27bb1 100644 --- a/include/ada_c.h +++ b/include/ada_c.h @@ -165,21 +165,21 @@ ada_string ada_strings_get(ada_strings result, size_t index); void ada_free_search_params_keys_iter(ada_url_search_params_keys_iter result); ada_string ada_search_params_keys_iter_next( ada_url_search_params_keys_iter result); -bool ada_search_params_keys_iter_is_done( +bool ada_search_params_keys_iter_has_next( ada_url_search_params_keys_iter result); void ada_free_search_params_values_iter( ada_url_search_params_values_iter result); ada_string ada_search_params_values_iter_next( ada_url_search_params_values_iter result); -bool ada_search_params_values_iter_is_done( +bool ada_search_params_values_iter_has_next( ada_url_search_params_values_iter result); void ada_free_search_params_entries_iter( ada_url_search_params_entries_iter result); ada_string_pair ada_search_params_entries_iter_next( ada_url_search_params_entries_iter result); -bool ada_search_params_entries_iter_is_done( +bool ada_search_params_entries_iter_has_next( ada_url_search_params_entries_iter result); #endif // ADA_C_H diff --git a/src/ada_c.cpp b/src/ada_c.cpp index c6e1389d9..77633bc07 100644 --- a/src/ada_c.cpp +++ b/src/ada_c.cpp @@ -626,12 +626,12 @@ ada_string ada_search_params_keys_iter_next( return ada_string_create(next->data(), next->length()); } -bool ada_search_params_keys_iter_is_done( +bool ada_search_params_keys_iter_has_next( ada_url_search_params_keys_iter result) { ada::result* r = (ada::result*)result; if (!r) return true; - return (*r)->is_done(); + return (*r)->has_next(); } void ada_free_search_params_values_iter( @@ -651,12 +651,12 @@ ada_string ada_search_params_values_iter_next( return ada_string_create(next->data(), next->length()); } -bool ada_search_params_values_iter_is_done( +bool ada_search_params_values_iter_has_next( ada_url_search_params_values_iter result) { ada::result* r = (ada::result*)result; if (!r) return true; - return (*r)->is_done(); + return (*r)->has_next(); } void ada_free_search_params_entries_iter( @@ -679,12 +679,12 @@ ada_string_pair ada_search_params_entries_iter_next( ada_string_create(next->second.data(), next->second.length())}; } -bool ada_search_params_entries_iter_is_done( +bool ada_search_params_entries_iter_has_next( ada_url_search_params_entries_iter result) { ada::result* r = (ada::result*)result; if (!r) return true; - return (*r)->is_done(); + return (*r)->has_next(); } } // extern "C" diff --git a/tests/ada_c.cpp b/tests/ada_c.cpp index 6b48e47ba..b13d2e015 100644 --- a/tests/ada_c.cpp +++ b/tests/ada_c.cpp @@ -295,16 +295,16 @@ TEST(ada_c, ada_url_search_params) { ada_url_search_params_entries_iter entries = ada_search_params_get_entries(out); - ASSERT_FALSE(ada_search_params_keys_iter_is_done(keys)); - ASSERT_FALSE(ada_search_params_values_iter_is_done(values)); - ASSERT_FALSE(ada_search_params_entries_iter_is_done(entries)); + ASSERT_TRUE(ada_search_params_keys_iter_has_next(keys)); + ASSERT_TRUE(ada_search_params_values_iter_has_next(values)); + ASSERT_TRUE(ada_search_params_entries_iter_has_next(entries)); ASSERT_EQ(convert_string(ada_search_params_keys_iter_next(keys)), "a"); ASSERT_EQ(convert_string(ada_search_params_keys_iter_next(keys)), "c"); ASSERT_EQ(convert_string(ada_search_params_keys_iter_next(keys)), "c"); ASSERT_EQ(convert_string(ada_search_params_keys_iter_next(keys)), "f"); ASSERT_EQ(convert_string(ada_search_params_keys_iter_next(keys)), "key1"); - ASSERT_TRUE(ada_search_params_keys_iter_is_done(keys)); + ASSERT_FALSE(ada_search_params_keys_iter_has_next(keys)); ASSERT_EQ(convert_string(ada_search_params_values_iter_next(values)), "b"); ASSERT_EQ(convert_string(ada_search_params_values_iter_next(values)), "d"); @@ -312,7 +312,7 @@ TEST(ada_c, ada_url_search_params) { ASSERT_EQ(convert_string(ada_search_params_values_iter_next(values)), "g"); ASSERT_EQ(convert_string(ada_search_params_values_iter_next(values)), "value2"); - ASSERT_TRUE(ada_search_params_values_iter_is_done(values)); + ASSERT_FALSE(ada_search_params_values_iter_has_next(values)); ada_string_pair pair = ada_search_params_entries_iter_next(entries); ASSERT_EQ(convert_string(pair.value), "b"); @@ -322,7 +322,7 @@ TEST(ada_c, ada_url_search_params) { ASSERT_EQ(convert_string(pair.value), "d"); ASSERT_EQ(convert_string(pair.key), "c"); - while (!ada_search_params_entries_iter_is_done(entries)) { + while (ada_search_params_entries_iter_has_next(entries)) { ada_search_params_entries_iter_next(entries); } diff --git a/tests/url_search_params.cpp b/tests/url_search_params.cpp index b2f3e728c..07e831b70 100644 --- a/tests/url_search_params.cpp +++ b/tests/url_search_params.cpp @@ -169,6 +169,7 @@ TEST(url_search_params, has) { } TEST(url_search_params, iterators) { + // JS style iterators auto search_params = ada::url_search_params("key1=value1&key1=value2&key2=value3"); auto keys = search_params.get_keys(); @@ -201,5 +202,20 @@ TEST(url_search_params, iterators) { ASSERT_FALSE(entries.next().has_value()); + // C++ conventional iterator + std::vector> expected = { + {"foo", "bar"}, + {"key2", "value3"}, + {"key1", "value2"}, + {"key1", "value1"}, + }; + for (auto& entry : search_params) { + auto check = expected.back(); + expected.pop_back(); + ASSERT_EQ(check.first, entry.first); + ASSERT_EQ(check.second, entry.second); + } + ASSERT_EQ(expected.size(), 0); + SUCCEED(); }