Atheel/jsoncons release 173 3

[amassalha]

No description provided.

[danielaparker]

@amassalha, We can't release this change as a patch on master, because master contains additional commits since 0.173.2 that we're not prepared to release yet. We could incorporate the change in a patch directly on 0.173.2.

[amassalha]

@danielaparker that would be great. I need to take it from Conan

[amassalha]

How would it be done?

[danielaparker]

How would it be done?

Please check new branch v0.173.3-patch and verify that it has what you need.

[amassalha]

Yes v0.173.3-patch is exactly what I need (from conan center).

[amassalha]

@danielaparker I tried to release that branch by-myself but seems I don't have permissions
are you planning to release it to conan?
thanks

[danielaparker]

@amassalha I had another look at this. I don't think you need 0.173.3. I added a number of test cases for cbegin()/cend() and crbegin()/crend(), and they work fine with the range type in 0.173.2. I don't think we need const qualifiers on cbegin(), cend(), crbegin() and crend() in class range. I agree that they should be noexcept.

Note that object_range() and its const overload both return a range object, but with different template parameters, such that range::begin() resolves to either an object_iterator or const_object_iterator. range::cbegin() and range::crbegin() of course always resolve to const_object_iterator.

        range<object_iterator, const_object_iterator> object_range()
        {
            return evaluate().object_range();
        }

        range<const_object_iterator, const_object_iterator> object_range() const
        {
            return evaluate().object_range();
        }

You'll always get the right iterator. We're not using constness in the range member functions to make that determination.

I think you could argue that all of the member functions of range, including begin(), should be const, because none of them affect the state of range, rather, they all return copies of iterators held by range. But practically I don't see how it makes any difference.

[amassalha]

@danielaparker I see your latests fixes, but you still have the const which is good because without it I will get the below compilation errors. So do we agree on the 'const' or you still have concerns? The issue is when I hold const json object, and I want to call its cbegin(), it needs to be marked as const function.
example:
using jsoncons_list = jsoncons::range<json::const_array_iterator , json::const_array_iterator>;
func(const jsoncons_list jlist) {
jlist.cbegin();
}

errors (not related to the code example above):

error: 'this' argument to member function 'cbegin' has type 'const jsoncons_list' (aka 'const range<__normal_iterator<const jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>> *, std::vector<jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>>, std::allocator<jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>>>>>, __normal_iterator<const jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>> *, std::vector<jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>>, std::allocator<jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>>>>>>'), but function is not marked const
  auto list_size = std::distance(list_values.cbegin(), list_values.cend());
                                 ^~~~~~~~~~~
/usr/local/miniconda/envs/mfc/include/jsoncons/basic_json.hpp:357:24: note: 'cbegin' declared here
        const_iterator cbegin() noexcept
                       ^
../cpp/formatters/json_cpp_formatter.cpp:267:56: error: 'this' argument to member function 'cend' has type 'const jsoncons_list' (aka 'const range<__normal_iterator<const jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>> *, std::vector<jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>>, std::allocator<jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>>>>>, __normal_iterator<const jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>> *, std::vector<jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>>, std::allocator<jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>>>>>>'), but function is not marked const
  auto list_size = std::distance(list_values.cbegin(), list_values.cend());
                                                       ^~~~~~~~~~~
/usr/local/miniconda/envs/mfc/include/jsoncons/basic_json.hpp:361:24: note: 'cend' declared here
        const_iterator cend() noexcept
                       ^
../cpp/formatters/json_cpp_formatter.cpp:271:19: error: 'this' argument to member function 'cbegin' has type 'const jsoncons_list' (aka 'const range<__normal_iterator<const jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>> *, std::vector<jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>>, std::allocator<jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>>>>>, __normal_iterator<const jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>> *, std::vector<jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>>, std::allocator<jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>>>>>>'), but function is not marked const
  for (auto elm = list_values.cbegin(); elm != list_values.cend(); ++elm) {  
                  ^~~~~~~~~~~
/usr/local/miniconda/envs/mfc/include/jsoncons/basic_json.hpp:357:24: note: 'cbegin' declared here
        const_iterator cbegin() noexcept
                       ^
../cpp/formatters/json_cpp_formatter.cpp:271:48: error: 'this' argument to member function 'cend' has type 'const jsoncons_list' (aka 'const range<__normal_iterator<const jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>> *, std::vector<jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>>, std::allocator<jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>>>>>, __normal_iterator<const jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>> *, std::vector<jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>>, std::allocator<jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>>>>>>'), but function is not marked const
  for (auto elm = list_values.cbegin(); elm != list_values.cend(); ++elm) {  
                                               ^~~~~~~~~~~
/usr/local/miniconda/envs/mfc/include/jsoncons/basic_json.hpp:361:24: note: 'cend' declared here
        const_iterator cend() noexcept
                       ^

[amassalha]

take this small full example:

#include <iostream>
#include <jsoncons/json.hpp>

using json = jsoncons::ojson;
using jsoncons_list = jsoncons::range<json::const_array_iterator , json::const_array_iterator>;

void print_json_list(const jsoncons_list arr_range) {
    auto list_size = std::distance(arr_range.cbegin(), arr_range.cend());
    std::cout <<"list size: "<<list_size<<std::endl;
    for (auto it = arr_range.cbegin(); it != arr_range.cend(); ++it) {
         std::cout << it->as_string() << "\n";
    }
}

int main() {
    // Create a JSON array
    const json arr = json::parse(R"({
     "names" :   ["Obaida", "Zaid", "Othman", "Salsabel"]

    })");

    auto arr_range = arr["names"].array_range();
    print_json_list(arr_range);

    return 0;
}

compilation error if having no 'const':

clang++ -std=gnu++20 -o jsoncons_example jsoncon.cpp -I/usr/local/miniconda/envs/mfc/include/ -fPIC -Wno-unused-parameter

jsoncon.cpp:8:36: error: 'this' argument to member function 'cbegin' has type 'const jsoncons_list' (aka 'const range<__normal_iterator<const jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>> *, std::vector<jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>>, std::allocator<jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>>>>>, __normal_iterator<const jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>> *, std::vector<jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>>, std::allocator<jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>>>>>>'), but function is not marked const
    auto list_size = std::distance(arr_range.cbegin(), arr_range.cend());
                                   ^~~~~~~~~
/usr/local/miniconda/envs/mfc/include/jsoncons/basic_json.hpp:357:24: note: 'cbegin' declared here
        const_iterator cbegin() noexcept
                       ^
jsoncon.cpp:8:56: error: 'this' argument to member function 'cend' has type 'const jsoncons_list' (aka 'const range<__normal_iterator<const jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>> *, std::vector<jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>>, std::allocator<jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>>>>>, __normal_iterator<const jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>> *, std::vector<jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>>, std::allocator<jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>>>>>>'), but function is not marked const
    auto list_size = std::distance(arr_range.cbegin(), arr_range.cend());
                                                       ^~~~~~~~~
/usr/local/miniconda/envs/mfc/include/jsoncons/basic_json.hpp:361:24: note: 'cend' declared here
        const_iterator cend() noexcept
                       ^
jsoncon.cpp:10:20: error: 'this' argument to member function 'cbegin' has type 'const jsoncons_list' (aka 'const range<__normal_iterator<const jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>> *, std::vector<jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>>, std::allocator<jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>>>>>, __normal_iterator<const jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>> *, std::vector<jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>>, std::allocator<jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>>>>>>'), but function is not marked const
    for (auto it = arr_range.cbegin(); it != arr_range.cend(); ++it) {
                   ^~~~~~~~~
/usr/local/miniconda/envs/mfc/include/jsoncons/basic_json.hpp:357:24: note: 'cbegin' declared here
        const_iterator cbegin() noexcept
                       ^
jsoncon.cpp:10:46: error: 'this' argument to member function 'cend' has type 'const jsoncons_list' (aka 'const range<__normal_iterator<const jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>> *, std::vector<jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>>, std::allocator<jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>>>>>, __normal_iterator<const jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>> *, std::vector<jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>>, std::allocator<jsoncons::basic_json<char, jsoncons::order_preserving_policy, std::allocator<char>>>>>>'), but function is not marked const
    for (auto it = arr_range.cbegin(); it != arr_range.cend(); ++it) {
                                             ^~~~~~~~~
/usr/local/miniconda/envs/mfc/include/jsoncons/basic_json.hpp:361:24: note: 'cend' declared here
        const_iterator cend() noexcept
                       ^
4 errors generated.

btw I see different behaviours between clang++ and g++, what are you using?

[danielaparker]

Oh, I see, the issue is here

void print_json_list(const jsoncons_list arr_range) {

You would only be able to call const member functions on arr_range, as it's passed as a const parameter. That's a use I hadn't anticipated.

It would work written as

void print_json_list(jsoncons_list& arr_range) {

but since we already have 0.173.3-patch started, and I'm fine with making all the member functions of range const, I'll make the release today.

Daniel

[amassalha]

Great, thanks (marking const only the const ones of course, or marking all and adding a non const onse also)
And thanks for the great lib btw, the only cpp one that I find that can print big decimals as numbers and not as strings

[amassalha]

@danielaparker please see std implementation of iterators:
https://github.com/llvm/llvm-project/blob/main/libcxx/include/vector
They have both options (const and without), you must allow also the non-const one to allow modifications

[danielaparker]

@danielaparker please see std implementation of iterators: https://github.com/llvm/llvm-project/blob/main/libcxx/include/vector They have both options (const and without), you must allow also the non-const one to allow modifications

In our case, begin(), end(), rbegin(), rend(), cbegin(), cend(), crbegin(), and crend() are members of the class range, not the container. None of them are capable of changing the state of the range object, a range object holds iterators, and these functions return copies of the held iterators, hence it is appropriate that they all be declared const.

Regarding mutability of a basic_json through a range object, if you obtain a range object through a call to object_range() on a non-const json object, begin(), end(), rbegin(), and rend() will return iterators that can mutate the json object , while cbegin(), cend(), crbegin(), and crend() will return iterators that cannot. If you obtain a range object through a call to object_range() const on a const json object, begin(), end(), rbegin(), and rend() cbegin(), cend(), crbegin(), and crend() will all return iterators that cannot mutate the json object.

Regarding "allowing modifications" (and not allowing modifications when appropriate), we have thousands of test cases that cover that :-)

[amassalha]

I see, good then

[danielaparker]

Incidentally,

    using jsoncons_list = jsoncons::range<json::const_array_iterator , json::const_array_iterator>;

can now be written as

    using jsoncons_list = json::const_object_range_type;

I've added the typedefs object_range_type, const_object_range_type, array_range_type, and const_array_range_type to basic_json.

[amassalha]

Great, thanks!