Keep the descent guard's bookkeeping self-contained

nesting_depth_limit() and nesting_depth_guard were only used inside
the JSON_NO_THREAD_LOCAL-guarded branch of copy_structured(), but were
defined unconditionally. Move them inside the #ifndef, and have the
guard look up the depth and test it against the limit itself (via
okay()) instead of making the caller do it - the caller no longer
needs to touch nesting_depth() at all. Also shrink the thread-local
counter to std::uint8_t, matching what its own doc comment already
argued.

Addresses gregmarr's review comments on #5389.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
This commit is contained in:
Niels Lohmann
2026-09-02 19:08:35 +02:00
parent aca11ed13d
commit 4aebfc8d4d
2 changed files with 50 additions and 36 deletions
+25 -18
View File
@@ -821,14 +821,17 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
return j; return j;
} }
#ifndef JSON_NO_THREAD_LOCAL
/// the number of levels an operation descends into before it finishes the /// the number of levels an operation descends into before it finishes the
/// value below it without the call stack /// value below it without the call stack
static constexpr std::size_t nesting_depth_limit() ///
/// only meaningful together with @ref nesting_depth_guard, which is why
/// both live inside this very guard's own #ifndef
static constexpr std::uint8_t nesting_depth_limit()
{ {
return 128; return 128;
} }
#ifndef JSON_NO_THREAD_LOCAL
/*! /*!
@brief how many levels the operation going on in this thread has descended into @brief how many levels the operation going on in this thread has descended into
@@ -841,33 +844,32 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
A byte is enough: the count never exceeds the limit by more than the single A byte is enough: the count never exceeds the limit by more than the single
level that notices the limit has been reached. level that notices the limit has been reached.
*/ */
static std::size_t& nesting_depth() noexcept static std::uint8_t& nesting_depth() noexcept
{ {
static thread_local std::size_t depth = 0; // NOLINT(misc-use-internal-linkage) static thread_local std::uint8_t depth = 0; // NOLINT(misc-use-internal-linkage)
return depth; return depth;
} }
#endif
/*! /*!
@brief counts one level of a bounded descent for as long as it runs @brief counts one level of a bounded descent for as long as it runs, and
reports whether the descent was still within the limit when it began
The count is taken rather than looked up here, because the caller has looked Looks the count up and tests it against the limit itself, rather than
it up already to test it against the limit: reaching thread-local storage is leaving that to the caller: either way it is reached exactly once, so
not free, and the path that is taken almost every time should reach it once there is nothing to be gained by making the caller do it.
rather than twice.
*/ */
class nesting_depth_guard class nesting_depth_guard
{ {
public: public:
explicit nesting_depth_guard(std::size_t& depth) noexcept nesting_depth_guard() noexcept
: m_depth(depth) : m_okay(nesting_depth() < nesting_depth_limit())
{ {
++m_depth; ++nesting_depth();
} }
~nesting_depth_guard() ~nesting_depth_guard()
{ {
--m_depth; --nesting_depth();
} }
nesting_depth_guard(const nesting_depth_guard&) = delete; nesting_depth_guard(const nesting_depth_guard&) = delete;
@@ -875,9 +877,15 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
nesting_depth_guard(nesting_depth_guard&&) = delete; nesting_depth_guard(nesting_depth_guard&&) = delete;
nesting_depth_guard& operator=(nesting_depth_guard&&) = delete; nesting_depth_guard& operator=(nesting_depth_guard&&) = delete;
bool okay() const noexcept
{
return m_okay;
}
private: private:
std::size_t& m_depth; bool m_okay;
}; };
#endif
/// an entry of the iterative deep copy's worklist: a structured value and /// an entry of the iterative deep copy's worklist: a structured value and
/// the value that is to become its copy /// the value that is to become its copy
@@ -1133,11 +1141,10 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
void copy_structured(const basic_json& src) void copy_structured(const basic_json& src)
{ {
#ifndef JSON_NO_THREAD_LOCAL #ifndef JSON_NO_THREAD_LOCAL
std::size_t& depth = nesting_depth(); const nesting_depth_guard guard;
if (JSON_HEDLEY_LIKELY(depth < nesting_depth_limit())) if (JSON_HEDLEY_LIKELY(guard.okay()))
{ {
const nesting_depth_guard guard(depth);
copy_level(src); copy_level(src);
return; return;
} }
+25 -18
View File
@@ -22184,14 +22184,17 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
return j; return j;
} }
#ifndef JSON_NO_THREAD_LOCAL
/// the number of levels an operation descends into before it finishes the /// the number of levels an operation descends into before it finishes the
/// value below it without the call stack /// value below it without the call stack
static constexpr std::size_t nesting_depth_limit() ///
/// only meaningful together with @ref nesting_depth_guard, which is why
/// both live inside this very guard's own #ifndef
static constexpr std::uint8_t nesting_depth_limit()
{ {
return 128; return 128;
} }
#ifndef JSON_NO_THREAD_LOCAL
/*! /*!
@brief how many levels the operation going on in this thread has descended into @brief how many levels the operation going on in this thread has descended into
@@ -22204,33 +22207,32 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
A byte is enough: the count never exceeds the limit by more than the single A byte is enough: the count never exceeds the limit by more than the single
level that notices the limit has been reached. level that notices the limit has been reached.
*/ */
static std::size_t& nesting_depth() noexcept static std::uint8_t& nesting_depth() noexcept
{ {
static thread_local std::size_t depth = 0; // NOLINT(misc-use-internal-linkage) static thread_local std::uint8_t depth = 0; // NOLINT(misc-use-internal-linkage)
return depth; return depth;
} }
#endif
/*! /*!
@brief counts one level of a bounded descent for as long as it runs @brief counts one level of a bounded descent for as long as it runs, and
reports whether the descent was still within the limit when it began
The count is taken rather than looked up here, because the caller has looked Looks the count up and tests it against the limit itself, rather than
it up already to test it against the limit: reaching thread-local storage is leaving that to the caller: either way it is reached exactly once, so
not free, and the path that is taken almost every time should reach it once there is nothing to be gained by making the caller do it.
rather than twice.
*/ */
class nesting_depth_guard class nesting_depth_guard
{ {
public: public:
explicit nesting_depth_guard(std::size_t& depth) noexcept nesting_depth_guard() noexcept
: m_depth(depth) : m_okay(nesting_depth() < nesting_depth_limit())
{ {
++m_depth; ++nesting_depth();
} }
~nesting_depth_guard() ~nesting_depth_guard()
{ {
--m_depth; --nesting_depth();
} }
nesting_depth_guard(const nesting_depth_guard&) = delete; nesting_depth_guard(const nesting_depth_guard&) = delete;
@@ -22238,9 +22240,15 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
nesting_depth_guard(nesting_depth_guard&&) = delete; nesting_depth_guard(nesting_depth_guard&&) = delete;
nesting_depth_guard& operator=(nesting_depth_guard&&) = delete; nesting_depth_guard& operator=(nesting_depth_guard&&) = delete;
bool okay() const noexcept
{
return m_okay;
}
private: private:
std::size_t& m_depth; bool m_okay;
}; };
#endif
/// an entry of the iterative deep copy's worklist: a structured value and /// an entry of the iterative deep copy's worklist: a structured value and
/// the value that is to become its copy /// the value that is to become its copy
@@ -22496,11 +22504,10 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
void copy_structured(const basic_json& src) void copy_structured(const basic_json& src)
{ {
#ifndef JSON_NO_THREAD_LOCAL #ifndef JSON_NO_THREAD_LOCAL
std::size_t& depth = nesting_depth(); const nesting_depth_guard guard;
if (JSON_HEDLEY_LIKELY(depth < nesting_depth_limit())) if (JSON_HEDLEY_LIKELY(guard.okay()))
{ {
const nesting_depth_guard guard(depth);
copy_level(src); copy_level(src);
return; return;
} }