Using define_static_string from https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2025/p3491r2.html#2389
Merged
lemire merged 3 commits intojson_builder_initfrom Jul 14, 2025
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR integrates P3491R2’s std::define_static_string into the reflection-based JSON serializer/deserializer and adds negative-zero handling when SIMDJSON_MINUS_ZERO_AS_FLOAT is defined.
- Wraps std::meta::identifier_of(...) results in std::define_static_string in both serialization and deserialization code
- Switches reflection expansions to use std::meta::access_context::unchecked()
- Regenerates the singleheader, including new conditional handling to emit -0.0 for negative zero
Reviewed Changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| singleheader/simdjson.cpp | Adds special-casing for negative zero in parse_number and get_number_type under a macro |
| include/simdjson/generic/ondemand/std_deserialize.h | Wraps identifier_of(...) calls with define_static_string and updates reflection context |
| include/simdjson/generic/ondemand/json_builder.h | Wraps quoted field names with define_static_string and updates reflection context |
Comments suppressed due to low confidence (4)
singleheader/simdjson.cpp:9831
- New conditional paths for SIMDJSON_MINUS_ZERO_AS_FLOAT introduce special handling of negative zero. Ensure that unit tests cover serialization of negative zero and type detection via get_number_type to prevent regressions.
#if SIMDJSON_MINUS_ZERO_AS_FLOAT
include/simdjson/generic/ondemand/json_builder.h:105
- [nitpick] Wrapping the quoted escaped identifier with std::define_static_string may allocate static storage for each field key. If reflection is used on many types, consider caching or deduplicating these static strings to avoid binary bloat.
constexpr auto key = std::define_static_string(consteval_to_quoted_escaped(std::meta::identifier_of(dm)));
include/simdjson/generic/ondemand/std_deserialize.h:15
- The std::define_static_string call requires the <static_reflection> header, but it's currently only commented out. Consider adding a proper include guard or feature detection for this header so the code remains compilable once the header is available.
// #include <static_reflection> // for std::define_static_string - header not available yet
include/simdjson/generic/ondemand/json_builder.h:20
- Similar to std_deserialize.h, the <static_reflection> include is commented out. Add proper feature detection or conditional include to ensure std::define_static_string is available during compilation.
// #include <static_reflection> // for std::define_static_string - header not available yet
Member
|
This change is correct as far as I know. The CI failures are unrelated but have to do with Google Benchmarks. Merging to |
lemire
added a commit
that referenced
this pull request
Jul 14, 2025
* Initial work on JSON builder * moving the files back to ondemand for now. * tweak * more later * update * minor edits * dropping vs arm (missing support) * adding tests. we still specialized write_string_escaped * tweaking * fix typo * tweaking the approach * minor fix * missing store * another missing store * Attempt at fixing failing serialization tests. (#2292) * Fixing appeand_float typo (#2294) * applying a couple of fixes * updating single header * fix for pre C++17 if constexpr * Fixing unused argument problem and updating the singleheader file * various pedantic fixes * Sketch of builder * reordering. * simplify * Adding draft of static reflection based deserialization * Updating simdjson singleheader * patching the automated deserialization. * automated * Adding support for smart pointers of user defined types. * Adding specialization for smart pointers for basic types. I think it is highly likely that this can be done in a more generic way. * Referncing a later version of rapidjson that fixed the issue related with assignment attempt of a const variable for GenericStringRef class. * guarding the tests * adding documentation for string_builder * saving * rename to 'append' * saving * non-functional benchmarks (#2342) * non-functional benchmarks * Fix typo * various fixes * tweaking --------- Co-authored-by: Daniel Lemire <[email protected]> Co-authored-by: Francisco Geiman Thiesen <[email protected]> * tuning * various minor fixes * minor tweak * minor simplification * updating amal * adding a cast * update * fancy casting * removing dead code * Pushing latest changes. CITM benchmark is still not working. * Still not working, but now I am getting only 10 errors. * add static reflection benchmark to 'large random' benchmark and allows (#2349) deserialization (with static reflection) from objects and arrays. Co-authored-by: Daniel Lemire <[email protected]> * Removing std::map from CitmCatalog definition, since that is not currently supported. * Added free to rust bench, segfault is still happening.. * The syntax changed: ^E became ^^E. (#2350) * The syntax changed: ^E became ^^E. * guarding --------- Co-authored-by: Daniel Lemire <[email protected]> * Adding support for string_view_keyed_map types. * Adding concepts as a conditional include. * updating single-header * Adding concepts to ondemand deps * rust benchmark is finally working * Fixing small typo in docs. * adding docker config and instructions so that our users can test the static reflection (#2358) * adding docker config and instructions so that our users can test the static reflection * completing the instructions * pruning white spaces --------- Co-authored-by: Daniel Lemire <[email protected]> * minor optimizations on the JSON builder branch * avoiding undef behaviour * saving * somewhat nicer builder * make it possible to run just one benchmark * adding linux perf * fixing minor issue * updating swar * Adding real world compilation benchmark (#2379) * Adding compilation benchmark for json parsing with and without reflection * Moving it to the benchmark folder, also reducing a bit the number of iterations. * Removing script from root folder. * Reducing number of iterations * Update benchmark/benchmark_reflection_usage_compilation.sh Co-authored-by: Daniel Lemire <[email protected]> * Update benchmark/benchmark_reflection_usage_compilation.sh Co-authored-by: Daniel Lemire <[email protected]> * Update benchmark/benchmark_reflection_usage_compilation.sh Co-authored-by: Daniel Lemire <[email protected]> * Making the script more customizable and also test whether the compiler being used supports reflection before actually running the benchmark --------- Co-authored-by: Daniel Lemire <[email protected]> * Using define_static_string from https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2025/p3491r2.html (#2389) * Applying changes needed after latest reflection paper updates. * Working, but no template for yet. * Updating single-header to incldue the use of define_static_string. * copying over master --------- Co-authored-by: Daniel Lemire <[email protected]> Co-authored-by: Francisco Geiman Thiesen <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Use P3491r2 std::define_static_string for reflection-based field names
Summary
This PR updates the reflection-based JSON serialization/deserialization code to use std::define_static_string() from P3491r2 when converting struct field names from compile-time reflection to runtime usage.
Changes
Rationale
When using reflection to extract struct field names at compile-time with std::meta::identifier_of(), the resulting strings need to be available at runtime for JSON key lookup operations. P3491r2's std::define_static_string() ensures these
compile-time strings are properly promoted to static storage with guaranteed lifetime, preventing potential issues with string lifetime and ensuring safe usage across compile-time to runtime boundaries.
Technical Details
Before:
constexpr std::string_view key = std::meta::identifier_of(mem);
After:
constexpr std::string_view key = std::define_static_string(std::meta::identifier_of(mem));
This change aligns with the latest C++26 reflection standards and follows the intended usage pattern for promoting compile-time reflection data to runtime-accessible static storage.
Testing