Document the vendored doctest.h patch; fix stale doctest_compatibility.h comments

tests/thirdparty/doctest/doctest.h is doctest 2.4.12, imported in
#4771. Two weeks later, #4801 hand-edited translateActiveException()
to declare "String res;" inside the translator loop instead of before
it, so a translator that does not match does not leave a previous
translator's result in "res" for the next iteration to see. Nothing
recorded this, so re-vendoring doctest.h from upstream would silently
drop the fix. Add a comment at the patched site naming the version,
the PR and the reason, so a future re-vendor knows to re-apply it.

Also fix two stale comments in doctest_compatibility.h:
- The DOCTEST_THREAD_LOCAL comment referenced Xcode 6/7, which is no
  longer supported; reword it to explain why the define must stay
  regardless (it keeps doctest's own thread_local usage out of the way
  of the same Clang/MinGW crash that JSON_NO_THREAD_LOCAL works around
  in the library, see ci_test_no_thread_local).
- The <iosfwd> include's comment justified it with tests that define
  "private" as "public"; no test under tests/src does that any more
  (removed by #2352). Reword the comment instead of dropping the
  include, since confirming it is safe to drop needs the full CI
  matrix including MSVC 2015+.

Verified locally that tests/src/unit-readme.cpp still builds and
passes 17/17 with these headers.

Closes #5714 item 9.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

tests/thirdparty/doctest/doctest.h

@@ -4498,6 +4498,12 @@ namespace {
     String translateActiveException() {
 #ifndef DOCTEST_CONFIG_NO_EXCEPTIONS
         auto&  translators = getExceptionTranslators();
+        // nlohmann/json locally patches upstream doctest 2.4.12 here: "res" was
+        // declared once before the loop and reused by every iteration; move it
+        // inside the loop so each translator gets a freshly default-constructed
+        // "res" instead of one a previous, non-matching translator may have
+        // written to (see nlohmann/json#4801). Re-apply this hunk when
+        // re-vendoring doctest.h from upstream.
         for(auto& curr : translators) {
             String res;
             if (curr->translate(res))

tests/thirdparty/doctest/doctest_compatibility.h

@@ -2,7 +2,12 @@
 #define DOCTEST_COMPATIBILITY
 
 #define DOCTEST_CONFIG_VOID_CAST_EXPRESSIONS
-#define DOCTEST_THREAD_LOCAL // enable single-threaded builds on XCode 6/7 - https://github.com/onqtam/doctest/issues/172
+#define DOCTEST_THREAD_LOCAL // make doctest itself avoid thread_local - https://github.com/onqtam/doctest/issues/172
+                              // (Xcode 6/7, for which this define was originally added, is no longer
+                              // supported, but the define must stay: it keeps doctest itself working the
+                              // same way as JSON_NO_THREAD_LOCAL makes the library behave, for the same
+                              // Clang/MinGW crash - see ci_test_no_thread_local in cmake/ci.cmake and
+                              // include/nlohmann/detail/macro_scope.hpp)
 #include "doctest.h"
 
 // Catch doesn't require a semicolon after CAPTURE but doctest does
@@ -26,9 +31,12 @@
 #define CHECK_THROWS_WITH_STD_STR(expr, str)                                   \
     CHECK_THROWS_WITH_STD_STR_IMPL(expr, str, DOCTEST_ANONYMOUS(DOCTEST_STD_STRING_))
 
-// included here because for some tests in the json repository private is defined as
+// No test under tests/src still defines "private" as "public" (the last one was removed
-// public and if no STL header is included before that then in the json include when STL
+// by #2352); this include predates that removal and stayed in case a test that does so
-// stuff is included the MSVC STL complains (errors) that C++ keywords are being redefined
+// is reintroduced, since it must come before <nlohmann/json.hpp> in that case (MSVC's STL
+// errors that C++ keywords are being redefined if an STL header pulled in indirectly by
+// the json include is the first one to see the #define). Keep it: dropping it would need
+// the full CI matrix, including MSVC 2015+, to confirm nothing still depends on it.
 #include <iosfwd>
 
 // Catch does this by default