ronantakizawa/codereview-bench huggingface dataset:
Repository: netty/netty
File: transport-classes-io_uring/src/main/java/io/netty/channel/uring/MsgHdrMemoryArray.java (Java)

Code before:
    }

    long id(int idx) {
        return ids[idx];
    }

    void setId(int idx, long id) {
        ids[idx] = id;
    }

    void clear() {
        Arrays.fill(ids, 0, idx, NO_ID);
        idx = 0;
    }

    int length() {
        return idx;
    }

    void release() {
        assert !released;
        released = true;
        for (MsgHdrMemory hdr: hdrs) {
            hdr.release();
        }
        this.msgHdrMemoryArrayMemoryCleanable.clean();
    }

    int capacity() {
        return capacity;
    }
}


Review comment (suggestion):
```suggestion
        msgHdrMemoryArrayMemoryCleanable.clean();
```

Code after:
    }

    long id(int idx) {
        return ids[idx];
    }

    void setId(int idx, long id) {
        ids[idx] = id;
    }

    void clear() {
        Arrays.fill(ids, 0, idx, NO_ID);
        idx = 0;
    }

    int length() {
        return idx;
    }

    void release() {
        assert !released;
        released = true;
        for (MsgHdrMemory hdr: hdrs) {
            hdr.release();
        }
        msgHdrMemoryArrayMemoryCleanable.clean();
    }

    int capacity() {
        return capacity;
    }
}
Repository: nlohmann/json
File: include/nlohmann/detail/iterators/iterator_traits.hpp (C++)

Code before:
{
template <class It, class = void>
struct _iterator_types {};

template <class It>
struct _iterator_types<
    It,
    void_t<typename It::difference_type, typename It::value_type, typename It::pointer,
           typename It::reference, typename It::iterator_category>> {
  using difference_type = typename It::difference_type;
  using value_type = typename It::value_type;
  using pointer = typename It::pointer;
  using reference = typename It::reference;
  using iterator_category = typename It::iterator_category;
};

template <class Iter>
struct iterator_traits : _iterator_types<Iter> {};

template <class T>
struct iterator_traits<T*> {
  typedef std::random_access_iterator_tag iterator_category;
  typedef T value_type;
  typedef ptrdiff_t difference_type;
  typedef T* pointer;
  typedef T& reference;
};

template <class T>
struct iterator_traits<const T*> {
  typedef std::random_access_iterator_tag iterator_category;
  typedef T value_type;
  typedef ptrdiff_t difference_type;
  typedef const T* pointer;
  typedef const T& reference;
};
}
}

Review comment (suggestion):
While we're at writing our `iterator_traits`, it would be a good idea to make it 100% SFINAE correct by applying this [LWG fix](https://cplusplus.github.io/LWG/issue2951)

Code after:
namespace detail
{
template <typename It, typename = void>
struct iterator_types {};

template <typename It>
struct iterator_types<
    It,
    void_t<typename It::difference_type, typename It::value_type, typename It::pointer,
           typename It::reference, typename It::iterator_category>> {
  using difference_type = typename It::difference_type;
  using value_type = typename It::value_type;
  using pointer = typename It::pointer;
  using reference = typename It::reference;
  using iterator_category = typename It::iterator_category;
};

// This is required as some compilers implement std::iterator_traits in a way that
// doesn't work with SFINAE. See https://github.com/nlohmann/json/issues/1341.
template <typename T, typename = void>
struct iterator_traits
{
};

template <typename T>
struct iterator_traits<T, enable_if_t<!std::is_pointer<T>::value>>
  : iterator_types<T>
{
};

template <typename T>
struct iterator_traits<T*, enable_if_t<std::is_object<T>::value>> {
  using iterator_category = std::random_access_iterator_tag;
  using value_type = T;
  using difference_type = ptrdiff_t;
  using pointer = T*;
  using reference = T&;
};
}
}
Repository: nlohmann/json
File: test/src/unit-testsuites.cpp (C++)

Code before:
        CHECK_NOTHROW(f >> j);
    }

    SECTION("3.json")
    {
        std::ifstream f("test/data/json.org/3.json");
        json j;
        CHECK_NOTHROW(f >> j);
    }

    SECTION("4.json")
    {
        std::ifstream f("test/data/json.org/4.json");
        json j;
        CHECK_NOTHROW(f >> j);
    }

    SECTION("5.json")
    {
        std::ifstream f("test/data/json.org/5.json");
        json j;
        CHECK_NOTHROW(f >> j);
    }
    SECTION("FILE 1.json")
    {
        auto f = fopen("test/data/json.org/1.json", "r");
        json j;
        CHECK_NOTHROW(j.parse(f));
        fclose(f);
    }

    SECTION("FILE 2.json")
    {
        auto f = fopen("test/data/json.org/2.json", "r");
        json j;
        CHECK_NOTHROW(j.parse(f));
        fclose(f);
    }

    SECTION("FILE 3.json")
    {
        auto f = fopen("test/data/json.org/3.json", "r");
        json j;
        CHECK_NOTHROW(j.parse(f));
        fclose(f);
    }

    SECTION("FILE 4.json")
    {
        auto f = fopen("test/data/json.org/4.json", "r");
        json j;

Review comment (refactor):
This is a minor issue, but will avoid having to write "fclose" at the bottom, and technically also makes sure that things get cleaned up even if an exception gets thrown. (Not a serious issue for test code, but it's still nice to have unit tests always clean up their resources in all cases).

```
std::unique_ptr<FILE, decltype(&fclose)> f(fopen("test/data/json.org/1.json", "r"), &fclose);
```
You'll have to do ```j.parse(f.get())``` in the lines below.

There are ways to make this prettier if we want, but this is bare minimum I would recommend.

Code after:
        CHECK_NOTHROW(f >> j);
    }

    SECTION("3.json")
    {
        std::ifstream f("test/data/json.org/3.json");
        json j;
        CHECK_NOTHROW(f >> j);
    }

    SECTION("4.json")
    {
        std::ifstream f("test/data/json.org/4.json");
        json j;
        CHECK_NOTHROW(f >> j);
    }

    SECTION("5.json")
    {
        std::ifstream f("test/data/json.org/5.json");
        json j;
        CHECK_NOTHROW(f >> j);
    }
    SECTION("FILE 1.json")
    {
        std::unique_ptr<std::FILE, decltype(&std::fclose)> f(std::fopen("test/data/json.org/1.json", "r"), &std::fclose);
        json j;
        CHECK_NOTHROW(j.parse(f.get()));
    }

    SECTION("FILE 2.json")
    {
        std::unique_ptr<std::FILE, decltype(&std::fclose)> f(std::fopen("test/data/json.org/2.json", "r"), &std::fclose);
        json j;
        CHECK_NOTHROW(j.parse(f.get()));
    }

    SECTION("FILE 3.json")
    {
        std::unique_ptr<std::FILE, decltype(&std::fclose)> f(std::fopen("test/data/json.org/3.json", "r"), &std::fclose);
        json j;
        CHECK_NOTHROW(j.parse(f.get()));
    }

    SECTION("FILE 4.json")
    {
        std::unique_ptr<std::FILE, decltype(&std::fclose)> f(std::fopen("test/data/json.org/4.json", "r"), &std::fclose);
        json j;
        CHECK_NOTHROW(j.parse(f.get()));
    }
Repository: nlohmann/json
File: include/nlohmann/json.hpp (C++)

Code before:
                else
                {
                    operator[](it.key()).merge_patch(it.value());
                }
            }
        }
        else
        {
            *this = apply_patch;
        }
    }

    /// @}
};

/*!
@brief user-defined to_string function for JSON values

This function implements a user-defined to_string  for JSON objects.

@param[in] j  a JSON object
@return a std::string object
*/

template <class T>
std::string to_string(const T& j){
    return j.dump();
}
} // namespace nlohmann

///////////////////////
// nonmember support //
///////////////////////

// specialization of std::swap, and std::hash
namespace std
{

/// hash value for JSON objects
template<>
struct hash<nlohmann::json>
{
    /*!
    @brief return a hash value for a JSON object

    @since version 1.0.0
    */
    std::size_t operator()(const nlohmann::json& j) const
    {
        // a naive hashing via the string representation
        const auto& h = hash<nlohmann::json::string_t>();

Review comment (suggestion):
It'd be better to constrain `T` to be a `basic_json<...>`.

You can use the ugly macro defined in `macro_scope.hpp` for that matter.

Code after:
                else
                {
                    operator[](it.key()).merge_patch(it.value());
                }
            }
        }
        else
        {
            *this = apply_patch;
        }
    }

    /// @}
};

/*!
@brief user-defined to_string function for JSON values

This function implements a user-defined to_string  for JSON objects.

@param[in] j  a JSON object
@return a std::string object
*/

NLOHMANN_BASIC_JSON_TPL_DECLARATION
std::string to_string(const NLOHMANN_BASIC_JSON_TPL& j)
{
    return j.dump();
}
} // namespace nlohmann

///////////////////////
// nonmember support //
///////////////////////

// specialization of std::swap, and std::hash
namespace std
{

/// hash value for JSON objects
template<>
struct hash<nlohmann::json>
{
    /*!
    @brief return a hash value for a JSON object

    @since version 1.0.0
    */
    std::size_t operator()(const nlohmann::json& j) const
    {
        // a naive hashing via the string representation
Repository: nlohmann/json
File: include/nlohmann/detail/json_pointer.hpp (C++)

Code before:

    @sa @ref operator/=(std::string) to append a reference token

    @since version 3.6.0
    */
    friend json_pointer operator/(const json_pointer& ptr, std::string token)
    {
        return json_pointer(ptr) /= std::move(token);
    }

    /*!
    @brief create a new JSON pointer by appending the array-index-token at the end of the JSON pointer

    @param[in] ptr  JSON pointer
    @param[in] array_indx  array index
    @return a new JSON pointer with @a array_indx appended to @a ptr

    @liveexample{The example shows the usage of `operator/`.,json_pointer__operator_add_binary}

    @complexity Linear in the length of @a ptr.

    @sa @ref operator/=(std::size_t) to append an array index

    @since version 3.6.0
    */
    friend json_pointer operator/(const json_pointer& ptr, std::size_t array_indx)
    {
        return json_pointer(ptr) /= array_indx;
    }

    /*!
    @brief returns the parent of this JSON pointer

    @return parent of this JSON pointer; in case this JSON pointer is the root,
            the root itself is returned

    @complexity Linear in the length of the JSON pointer.

    @liveexample{The example shows the result of `parent_pointer` for different
    JSON Pointers.,json_pointer__parent_pointer}

    @since version 3.6.0
    */
    json_pointer parent_pointer() const
    {
        if (empty())
        {
            return *this;
        }

        json_pointer res = *this;

Review comment (suggestion):
```suggestion
    friend json_pointer operator/(const json_pointer& ptr, std::size_t array_idx)
```

Code after:

    @sa @ref operator/=(std::string) to append a reference token

    @since version 3.6.0
    */
    friend json_pointer operator/(const json_pointer& ptr, std::string token)
    {
        return json_pointer(ptr) /= std::move(token);
    }

    /*!
    @brief create a new JSON pointer by appending the array-index-token at the end of the JSON pointer

    @param[in] ptr  JSON pointer
    @param[in] array_idx  array index
    @return a new JSON pointer with @a array_idx appended to @a ptr

    @liveexample{The example shows the usage of `operator/`.,json_pointer__operator_add_binary}

    @complexity Linear in the length of @a ptr.

    @sa @ref operator/=(std::size_t) to append an array index

    @since version 3.6.0
    */
    friend json_pointer operator/(const json_pointer& ptr, std::size_t array_idx)
    {
        return json_pointer(ptr) /= array_idx;
    }

    /*!
    @brief returns the parent of this JSON pointer

    @return parent of this JSON pointer; in case this JSON pointer is the root,
            the root itself is returned

    @complexity Linear in the length of the JSON pointer.

    @liveexample{The example shows the result of `parent_pointer` for different
    JSON Pointers.,json_pointer__parent_pointer}

    @since version 3.6.0
    */
    json_pointer parent_pointer() const
    {
        if (empty())
        {
            return *this;
        }

        json_pointer res = *this;
Repository: nlohmann/json
File: include/nlohmann/detail/json_pointer.hpp (C++)

Code before:

    @since version 3.6.0
    */
    friend json_pointer operator/(const json_pointer& ptr, std::string token)
    {
        return json_pointer(ptr) /= std::move(token);
    }

    /*!
    @brief create a new JSON pointer by appending the array-index-token at the end of the JSON pointer

    @param[in] ptr  JSON pointer
    @param[in] array_indx  array index
    @return a new JSON pointer with @a array_indx appended to @a ptr

    @liveexample{The example shows the usage of `operator/`.,json_pointer__operator_add_binary}

    @complexity Linear in the length of @a ptr.

    @sa @ref operator/=(std::size_t) to append an array index

    @since version 3.6.0
    */
    friend json_pointer operator/(const json_pointer& ptr, std::size_t array_indx)
    {
        return json_pointer(ptr) /= array_indx;
    }

    /*!
    @brief returns the parent of this JSON pointer

    @return parent of this JSON pointer; in case this JSON pointer is the root,
            the root itself is returned

    @complexity Linear in the length of the JSON pointer.

    @liveexample{The example shows the result of `parent_pointer` for different
    JSON Pointers.,json_pointer__parent_pointer}

    @since version 3.6.0
    */
    json_pointer parent_pointer() const
    {
        if (empty())
        {
            return *this;
        }

        json_pointer res = *this;
        res.pop_back();
        return res;

Review comment (suggestion):
```suggestion
        return json_pointer(ptr) /= array_idx;
```

Code after:

    @since version 3.6.0
    */
    friend json_pointer operator/(const json_pointer& ptr, std::string token)
    {
        return json_pointer(ptr) /= std::move(token);
    }

    /*!
    @brief create a new JSON pointer by appending the array-index-token at the end of the JSON pointer

    @param[in] ptr  JSON pointer
    @param[in] array_idx  array index
    @return a new JSON pointer with @a array_idx appended to @a ptr

    @liveexample{The example shows the usage of `operator/`.,json_pointer__operator_add_binary}

    @complexity Linear in the length of @a ptr.

    @sa @ref operator/=(std::size_t) to append an array index

    @since version 3.6.0
    */
    friend json_pointer operator/(const json_pointer& ptr, std::size_t array_idx)
    {
        return json_pointer(ptr) /= array_idx;
    }

    /*!
    @brief returns the parent of this JSON pointer

    @return parent of this JSON pointer; in case this JSON pointer is the root,
            the root itself is returned

    @complexity Linear in the length of the JSON pointer.

    @liveexample{The example shows the result of `parent_pointer` for different
    JSON Pointers.,json_pointer__parent_pointer}

    @since version 3.6.0
    */
    json_pointer parent_pointer() const
    {
        if (empty())
        {
            return *this;
        }

        json_pointer res = *this;
        res.pop_back();
        return res;
Repository: nlohmann/json
File: test/src/unit-allocator.cpp (C++)

Code before:
    using std::allocator<T>::allocator;

    template<class... Args>
    void construct(T* p, Args&& ... args)
    {
        if (next_construct_fails)
        {
            next_construct_fails = false;
            throw std::bad_alloc();
        }

        ::new (reinterpret_cast<void*>(p)) T(std::forward<Args>(args)...);
    }

    void deallocate(T* p, std::size_t n)
    {
        if (next_deallocate_fails)
        {
            next_deallocate_fails = false;
            throw std::bad_alloc();
        }

        std::allocator<T>::deallocate(p, n);
    }

    // the code below warns about p in MSVC 2015 - this could be a bug
    DOCTEST_MSVC_SUPPRESS_WARNING_PUSH
    DOCTEST_MSVC_SUPPRESS_WARNING(4100)
    void destroy(T* p)
    {
        if (next_destroy_fails)
        {
            next_destroy_fails = false;
            throw std::bad_alloc();
        }

        p->~T();
    }
    DOCTEST_MSVC_SUPPRESS_WARNING_POP

    template <class U>
    struct rebind
    {
        using other = my_allocator<U>;
    };
};

// allows deletion of raw pointer, usually hold by json_value
template<class T>
void my_allocator_clean_up(T* p)
{

Review comment (suggestion):
"C4100 can also be issued when code calls a destructor on a otherwise unreferenced parameter of primitive type. This is a limitation of the Microsoft C++ compiler."
https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4100?view=msvc-160


Code after:
    using std::allocator<T>::allocator;

    template<class... Args>
    void construct(T* p, Args&& ... args)
    {
        if (next_construct_fails)
        {
            next_construct_fails = false;
            throw std::bad_alloc();
        }

        ::new (reinterpret_cast<void*>(p)) T(std::forward<Args>(args)...);
    }

    void deallocate(T* p, std::size_t n)
    {
        if (next_deallocate_fails)
        {
            next_deallocate_fails = false;
            throw std::bad_alloc();
        }

        std::allocator<T>::deallocate(p, n);
    }

    void destroy(T* p)
    {
        if (next_destroy_fails)
        {
            next_destroy_fails = false;
            throw std::bad_alloc();
        }

        static_cast<void>(p); // fix MSVC's C4100 warning
        p->~T();
    }

    template <class U>
    struct rebind
    {
        using other = my_allocator<U>;
    };
};

// allows deletion of raw pointer, usually hold by json_value
template<class T>
void my_allocator_clean_up(T* p)
{
    assert(p != nullptr);
    my_allocator<T> alloc;
    alloc.destroy(p);
Repository: nlohmann/json
File: test/src/unit-udt.cpp (C++)

Code before:
        custom_json cj = j.get<custom_json>();
        CHECK(cj == 42);
    }
}

namespace
{
struct incomplete;

// std::is_constructible is broken on macOS' libc++
// use the cppreference implementation

template <typename T, typename = void>
struct is_constructible_patched : std::false_type {};

template <typename T>
struct is_constructible_patched<T, decltype(void(json(std::declval<T>())))> : std::true_type {};
} // namespace

TEST_CASE("an incomplete type does not trigger a compiler error in non-evaluated context" * doctest::test_suite("udt"))
{
    static_assert(!is_constructible_patched<json, incomplete>::value, "");
}

// the code below warns about t in MSVC 2015 - this could be a bug
DOCTEST_MSVC_SUPPRESS_WARNING_PUSH
DOCTEST_MSVC_SUPPRESS_WARNING(4100)

namespace
{
class Evil
{
  public:
    Evil() = default;
    template <typename T>
    Evil(T t) : m_i(sizeof(t)) {}

    int m_i = 0;
};

DOCTEST_MSVC_SUPPRESS_WARNING_POP

void from_json(const json& /*unused*/, Evil& /*unused*/) {}
} // namespace

TEST_CASE("Issue #924")
{
    // Prevent get<std::vector<Evil>>() to throw
    auto j = json::array();

    CHECK_NOTHROW(j.get<Evil>());

Review comment (suggestion):
Same class of error as the other one, `sizeof(t)` is a compile-time constant based only on `T`, not on the value of `t`.

Code after:
        custom_json cj = j.get<custom_json>();
        CHECK(cj == 42);
    }
}

namespace
{
struct incomplete;

// std::is_constructible is broken on macOS' libc++
// use the cppreference implementation

template <typename T, typename = void>
struct is_constructible_patched : std::false_type {};

template <typename T>
struct is_constructible_patched<T, decltype(void(json(std::declval<T>())))> : std::true_type {};
} // namespace

TEST_CASE("an incomplete type does not trigger a compiler error in non-evaluated context" * doctest::test_suite("udt"))
{
    static_assert(!is_constructible_patched<json, incomplete>::value, "");
}

namespace
{
class Evil
{
  public:
    Evil() = default;
    template <typename T>
    Evil(T t) : m_i(sizeof(t))
    {
        static_cast<void>(t); // fix MSVC's C4100 warning
    }

    int m_i = 0;
};

void from_json(const json& /*unused*/, Evil& /*unused*/) {}
} // namespace

TEST_CASE("Issue #924")
{
    // Prevent get<std::vector<Evil>>() to throw
    auto j = json::array();

    CHECK_NOTHROW(j.get<Evil>());
    CHECK_NOTHROW(j.get<std::vector<Evil>>());

    // silence Wunused-template warnings
Repository: nlohmann/json
File: test/src/unit-diagnostics.cpp (C++)

Code before:
    }

    SECTION("Regression test for issue #2962 - JSON_DIAGNOSTICS assertion for ordered_json")
    {
        nlohmann::ordered_json j;
        nlohmann::ordered_json j2;
        const std::string value;
        j["first"] = value;
        j["second"] = value;
        j2["something"] = j;
    }

    SECTION("Regression test for issue #3007 - Parent pointers properly set when using update()")
    {
        json j = json::object();

        {
            json j2 = json::object();
            j2["one"] = 1;

            j.update(j2);
        }

        for (auto const& kv : j)
        {
            CHECK(kv.m_parent == &j);
        }
    }
}


Review comment (bug):
I do not like this test too much as it is using private members. And since the tests are executed in debug mode, the invariant is checked which would trigger an assertion in case of an error.

Instead, you could do the following (not tested):

```cpp
j["one"].at(0);
```

This should trigger an exception, because `j["one"]` is a number and `at` is only defined for `null` or arrays. This exception should then contain a proper JSON path which shows that parents are properly set. This can be checked with `CHECK_THROWS_WITH_AS`.

Code after:

    SECTION("Regression test for issue #2962 - JSON_DIAGNOSTICS assertion for ordered_json")
    {
        nlohmann::ordered_json j;
        nlohmann::ordered_json j2;
        const std::string value;
        j["first"] = value;
        j["second"] = value;
        j2["something"] = j;
    }

    SECTION("Regression test for issue #3007 - Parent pointers properly set when using update()")
    {
        // void update(const_reference j)
        {
            json j = json::object();

            {
                json j2 = json::object();
                j2["one"] = 1;

                j.update(j2);
            }

            // Must call operator[] on const element, otherwise m_parent gets updated.
            auto const& constJ = j;
            CHECK_THROWS_WITH_AS(constJ["one"].at(0), "[json.exception.type_error.304] (/one) cannot use at() with number", json::type_error);
        }

        // void update(const_iterator first, const_iterator last)
        {
            json j = json::object();

            {
                json j2 = json::object();
                j2["one"] = 1;

                j.update(j2.begin(), j2.end());
            }

            // Must call operator[] on const element, otherwise m_parent gets updated.
            auto const& constJ = j;
            CHECK_THROWS_WITH_AS(constJ["one"].at(0), "[json.exception.type_error.304] (/one) cannot use at() with number", json::type_error);
        }

        // Code from #3007 triggering unwanted assertion without fix to update().
        {
            json root = json::array();
            json lower = json::object();

            {