Skip to content

Fix CI on develop after #5585 - #5779

Merged
nlohmann merged 7 commits into
developfrom
claude/fix-ci-errors-develop-c78443
Oct 7, 2026
Merged

nlohmann merged 7 commits into
developfrom
claude/fix-ci-errors-develop-c78443

Conversation

@nlohmann

@nlohmann nlohmann commented Oct 7, 2026 •

Copy link
Copy Markdown
Owner

The Ubuntu workflow on develop fails since #5585 (run https://github.com/nlohmann/json/actions/runs/37530611400). This PR fixes the three causes.

  • ci_test_gcc, ci_test_standards_gcc (14): the new test test-diagnostics-optimized is built with -O3. At that level, the -Winline and -Wsuggest-attribute=pure warnings in the ci_test_gcc flag set fire on the library's inline functions. The test does not check these warnings, so they are now turned off for this test, together with -Wsuggest-attribute=const. The -O3 -Werror=array-bounds check for Release build fails when JSON diagnostics enabled #5742 stays. Reproduced with GCC 16.2 in gcc:latest; the test now compiles cleanly.

  • ci_test_clang, ci_test_standards_clang (11): the static table in the test's to_json triggers -Wexit-time-destructors. The warning is now suppressed with DOCTEST_CLANG_SUPPRESS_WARNING, as other tests already do.

  • ci_infer: 7 false NULLPTR_DEREFERENCE findings in tests/src/unit-pointer_access.cpp, caused by Create a value before giving it its type #5585. basic_json::replace_value() calls destroy() and then assert_invariant(). Together they create more paths than Pulse explores by default (pulse-max-disjuncts, 20). Pulse then loses track of the stored type and assumes get_ptr() can return null for a value it just constructed. Removing either call made the findings go away, but the library code stays as is. Instead, .inferconfig raises pulse-max-disjuncts to 40. Checked with Infer 1.3.0 (the CI version) on test-pointer_access: 7 findings with the default, none with 40.

  • ci_infer: USE_AFTER_DELETE at include/nlohmann/ordered_map.hpp:262 (erase(first, last)). This is a false positive: Infer models std::string as keeping the buffer of a moved-from string, so the loop that destroys and re-creates elements looks like it destroys a buffer twice. It now has an @infer-ignore USE_AFTER_DELETE comment, the form already used in json.hpp and binary_reader.hpp. A full Infer run with pulse-max-disjuncts 40 and without this comment reported only this finding.

Two more failures came in with that merge, and both also affect develop:

On the previous push (2e4d840), ci_infer passed ("No issues found"). It took 81 minutes, against 50 on develop. A local run with the same settings took 52 minutes, so most of the difference is probably runner variance.

Develop is merged in (#5740, #5766, #5774-#5777); none of those commits touch the files changed here.

Public API

No breaking changes. The library changes are a comment in ordered_map.hpp and two static_cast<void> statements in a private function of binary_writer; the rest is CI configuration and tests.


Written by Claude Code.

🤖 Generated with Claude Code

- test-diagnostics-optimized: -O3 makes GCC's -Winline and
  -Wsuggest-attribute=pure/const warnings fire with the ci_test_gcc flag
  set; turn them off for this test.
- test-diagnostics-optimized: suppress Clang's -Wexit-time-destructors for
  the static table in to_json.
- Infer: raise pulse-max-disjuncts from 20 to 40. With the default,
  Pulse loses the stored type in basic_json::replace_value() and reports
  false null dereferences of get_ptr() results in unit-pointer_access.cpp.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Infer's std::string model keeps the buffer of a moved-from string, so the
destroy-and-reconstruct loop in erase(first, last) looks like it destroys a
buffer twice.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
With JSON_NOEXCEPTION, JSON_THROW expands to std::abort(), so Clang's
-Wunused-parameter breaks test-disabled_exceptions (since #5761).

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

The #5676 regression test (#5740) calls the deprecated
sax_parse(span_input_adapter&&, ...), which JSON_DELETE_DEPRECATED_FUNCTIONS
deletes, so ci_test_delete_deprecated_functions failed to build.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
@github-actions github-actions Bot added the aspect: binary formats BSON, CBOR, MessagePack, UBJSON label Oct 7, 2026
clang-tidy (clang-analyzer-security.ArrayBound) flagged it->second for a
value not in the table. Use the same fallback as
NLOHMANN_JSON_SERIALIZE_ENUM; the test still fails with -Werror=array-bounds
on the headers from before #5585.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
- unit-regression2.cpp (#5762): const/auto for the destroy() test values;
  NOLINT the intended copy in check_destroy_edge_case().
- unit-serialization.cpp (#5774): build the expected strings with += instead
  of chained operator+ (performance-inefficient-string-concatenation).

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
@nlohmann
nlohmann merged commit 367336c into develop Oct 7, 2026
169 of 170 checks passed
@nlohmann nlohmann added this to the Release 3.13.0 milestone Oct 7, 2026
@nlohmann
nlohmann deleted the claude/fix-ci-errors-develop-c78443 branch October 7, 2026 14:37
Comment thread tests/CMakeLists.txt
json_test_set_test_options(test-regression1 LINK_LIBRARIES fifo_map_include)

# GCC's false -Warray-bounds error with JSON_DIAGNOSTICS only shows up when optimizing (#5742)
# GCC's false -Warray-bounds error with JSON_DIAGNOSTICS only shows up when optimizing (#5742).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that was fixed now.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is fixed. The reordering in #5585 removed the false positive, so the comment was out of date. The test is now the regression test that keeps it fixed, and #5784 rewords the comment to say that.

Posted by Claude Code on behalf of @nlohmann.

nlohmann added a commit that referenced this pull request Oct 8, 2026
…5784)

* Address review comments on #5775 and #5779

Allow new defaulted parameters and new default arguments in the API
stability rules, mention the macro opt-in, and drop the redundant
recompile advice. Describe test-diagnostics-optimized as the regression
test for the fixed #5742.

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

* Document what counts as a breaking change in the API stability rules

Spell out the 3.x compatibility rules in the roadmap: new defaulted
parameters, new default arguments, noexcept/constexpr, template
parameters, parse and dump results, accepted input, key iteration order,
iterator invalidation, implicit conversions, to_json/from_json lookup,
json_sax, value_t enumerators, and documented macros, CMake options and
headers. Also list std::hash values as not part of the public API, and
link the macro overview from the section.

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

---------

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aspect: binary formats BSON, CBOR, MessagePack, UBJSON CMake M tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants