Skip to content

Conversation

ehughsbaird
Copy link
Contributor

Summary

Infrastructure "Improve generic factory readers for std::pair"

Purpose of change

#82943 (comment) prompted a discussion about the pair readers, and a request for the { "a": b } format.

Describe the solution

Support the "canonical" array format [ a, b ], as well as the object format { "a": b }, and single format a -> (a, a) in pair_reader. Comment on supported format for other pair readers, and improve the generic_map_reader support for different key types in the pairs.

Testing

tools/load_all_mods.py

Rebased onto #82943 and swapped out the reader added there for pair_reader.

Additional context

I considered adding { "a": b } as a "canonical" pair format the the JSON API recognizes, but I think there should only be one "canonical" format. Additionally, serialize would not emit the new format, and I don't think it would be worth the risk there.

diff --git a/src/flexbuffer_json-inl.h b/src/flexbuffer_json-inl.h
index 3e161a7c3c..c9ac2fd6c0 100644
--- a/src/flexbuffer_json-inl.h
+++ b/src/flexbuffer_json-inl.h
@@ -353,6 +353,21 @@ bool JsonValue::read( T &val, bool throw_on_error ) const
 template<typename T, typename U>
 bool JsonValue::read( std::pair<T, U> &p, bool throw_on_error ) const
 {
+    if( test_object() ) {
+        if constexpr( key_from_json_string<T>::valid ) {
+            JsonObject jo = get_object();
+            if( jo.size() != 1 ) {
+                return error_or_false( throw_on_error, "json object encoding pair can only have one element" );
+            }
+            // technically a loop, but only to unwrap the object into the member it consists of
+            for( JsonMember jm : jo ) {
+                p.first = key_from_json_string<T>()( dynamic_cast<const JsonMember *>( this )->name() );
+                return read( p.second, throw_on_error );
+            }
+        } else {
+            return error_or_false( throw_on_error, "Expected json array encoding pair" );
+        }
+    }
     if( !test_array() ) {
         return error_or_false( throw_on_error, "Expected json array encoding pair" );
     }

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Sep 19, 2025
@Maleclypse
Copy link
Member

Conflicts

Document what JSON formats the readers accept, and modify pair_reader so
that it can accept arbitrary types.

Add a new pair type { "a": b } that this pair_reader supports, and
expand key_from_json_string to enable testing if the type of a can
support this.
Copy link
Member

@RenechCDDA RenechCDDA left a comment

Choose a reason for hiding this comment

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

Thanks for doing this, and your comments are a much better explanation than what I wrote!

@Maleclypse Maleclypse merged commit b413383 into CleverRaven:master Sep 24, 2025
32 of 38 checks passed
@ehughsbaird ehughsbaird deleted the improved-pair-readers branch September 24, 2025 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions [C++] Changes (can be) made in C++. Previously named `Code` Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants