Add JSON marshaling library #2255

sudison · 2025-06-30T03:06:06Z

Refer to request: #1395

Add json_marshal.c3 with Go-style API (json::marshal, json::marshal_value, json::marshal_array)
Support all primitive types: String, int, float, double, bool
Support enums (always marshaled as enum names, regardless of associated values)
Support nested structs and arrays of any supported type
Use temp allocator for memory-safe operation
Comprehensive test suite with 15 test cases covering all features
Follow C3 coding style with proper and kindof usage

- Add json_marshal.c3 with Go-style API (json::marshal, json::marshal_value, json::marshal_array) - Support all primitive types: String, int, float, double, bool - Support enums (always marshaled as enum names, regardless of associated values) - Support nested structs and arrays of any supported type - Use temp allocator for memory-safe operation - Comprehensive test suite with 15 test cases covering all features - Follow C3 coding style with proper and kindof usage

data-man · 2025-06-30T09:30:01Z

I think that serialization and deserialization are more common and frequently used terms.

- Extract common marshaling logic into marshal_value function - Simplify marshal() to use marshal_value for all struct fields - Simplify marshal_array() to use marshal_value for all array elements - Add array/slice support to marshal_value function - Reduce code duplication by ~60 lines while maintaining all functionality - All existing tests continue to pass

sudison · 2025-06-30T18:05:52Z

I think that serialization and deserialization are more common and frequently used terms.

serialization and deserialization are too long for a function name, IMHO. I'd prefer shorter name like, marshal/unmarshal, like in golang.

BWindey

While I do not have any power over merging PRs, I went ahead and looked at the code.
The naming issue aside (I also prefer serialize/deserialize), the code seems to be written in a weird way. You claim to follow the C3 coding style, but the use of doc-comments and optionals is just plain different (and in case of doc-comments just wrong) from what the rest of the standard library uses.

If you're serious about getting this working and up to standards, feel free to join the Discord to chat about improving this. (Discord link: https://discord.gg/qN76R87, can also be found on the C3 website.)

BWindey · 2025-07-01T08:01:57Z

lib/std/encoding/json_marshal.c3

+/**
+ * Marshal a struct with primitive fields and nested structs to JSON
+ * @param [in] value The struct value to marshal
+ * @return The JSON string representation (using temp allocator)
+ */


Doc-comments are written with <* *> in C3.

<* Marshal a struct with primitive fields and nested structs to JSON @param [in] value The struct value to marshal @return The JSON string representation (using temp allocator) *>

BWindey · 2025-07-01T08:02:43Z

lib/std/encoding/json_marshal.c3

+    // Only handle structs
+    $if $Type.kindof != STRUCT:
+        return UNSUPPORTED_TYPE?;
+    $endif


This is the kind of thing you put as an @require in the doc-contract.

BWindey · 2025-07-01T08:05:15Z

lib/std/encoding/json_marshal.c3

+        return UNSUPPORTED_TYPE?;
+    $endif
+
+    DString result = dstring::temp();


Using temp allocator not in a @pool and then return a string-view to it?

While this might work because it's a macro, it feels wrong, especially because the macro-name does not signify that any allocations would happen that can have an influence outside this function.
Take a look at how String.escape is handled in core/string_escape.c3.

fixed, adding an allocator as input

BWindey · 2025-07-01T08:07:16Z

lib/std/encoding/json_marshal.c3

+                String? field_result = marshal_value($member.get(value));
+                if (catch err = field_result) return err?;


We'd write this as:

String field_result = marshal_value($member.get(value))!;

BWindey · 2025-07-01T08:11:16Z

test/unit/stdlib/encoding/json_marshal.c3

+    String? result = json::marshal_value(42);
+    assert(result!! == "42");


Would be better to write

String result = json::marshal_value(42)!!; assert(result == "42");

Idem for all other tests btw.

BWindey · 2025-07-01T08:12:58Z

test/unit/stdlib/encoding/json_marshal.c3

+    assert(json.contains(`"complex_state":"BUSY"`));     // Always uses enum name
+    assert(json.contains(`"pid":1234`));
+
+    // Should be valid JSON format


Testing if it's actually valid JSON is not 100% covered here, it might be worth writing a small checking function that will check if all keys are in quotes, that key-value pairs are seperated by :, that there are commas where needed...

fixed, now using json::tparse to parse the json, and verify the result.

sudison · 2025-07-02T05:10:54Z

While I do not have any power over merging PRs, I went ahead and looked at the code. The naming issue aside (I also prefer serialize/deserialize), the code seems to be written in a weird way. You claim to follow the C3 coding style, but the use of doc-comments and optionals is just plain different (and in case of doc-comments just wrong) from what the rest of the standard library uses.

If you're serious about getting this working and up to standards, feel free to join the Discord to chat about improving this. (Discord link: https://discord.gg/qN76R87, can also be found on the C3 website.)

Sorry about the that, most of the code and docs are written by AI, both AI and myself are still learning how to writing c3 code properly. Thanks for your feedback, all are addressed. I will join Discord soon.

BWindey

I don't know how I feel knowing this is done using AI. I feel like it's weird to jump into an open-source project and start letting AI make contributions. To me I'd feel like I want to do contrib 6D47 utions myself to learn the language and become fluid in it.

On the other hand the implementation seems ok.

But this is the last you'll hear from me about this I think, the rest is for @lerno to decide.

BWindey · 2025-07-02T08:19:12Z

lib/std/encoding/json_marshal.c3

                result.append(field_result);
            };
        $endif
    $endforeach

    result.append_char('}');
-    return result.str_view();
+    return result.copy_str(allocator);


Not deallocating the DString will leak memory, no?

fixed, finally figured out how to use pool and tmem

BWindey · 2025-07-02T08:21:20Z

test/unit/stdlib/encoding/json_marshal.c3

+    Object* parsed = json::tparse_string(json)!!;
+    defer parsed.free();


You're using the temp allocator, no need to free that explicitly.

BWindey · 2025-07-02T08:24:00Z

lib/std/encoding/json_marshal.c3

+ @param allocator : "The allocator to use for the result"
+ @param value: "The struct value to marshal"


inconsistent spaces around the :, it seems to be inconsistent in almost every doc-string.

sudison · 2025-07-03T07:54:37Z

I don't know how I feel knowing this is done using AI. I feel like it's weird to jump into an open-source project and start letting AI make contributions. To me I'd feel like I want to do contributions myself to learn the language and become fluid in it.

On the other hand the implementation seems ok.

But this is the last you'll hear from me about this I think, the rest is for @lerno to decide.

While I use AI to generate code and documentation, I am still a significant contributor to the final code. It takes considerable effort—often an hour or more—to guide the AI to write correct code. Because there is very little C3 code available for training, the AI's knowledge of the language is limited, which requires a lot of manual guidance to get it working correctly.
So, why bother using AI at all?
I believe AI-assisted programming is the future, whether we like it or not.
I wanted to test AI's capabilities on a niche programming language. I've been very satisfied, as it can write decent C3 code.

lerno · 2025-07-03T21:52:26Z

I don't have a set opinion on AI. However, any non-trivial feature should currently be incubated before inclusion. So the process is this:

Create a repo for it.
Send a request for feedback (file an issue)
Handle incoming pull request for improvements and fixes.
The library is then either included or instead recommended to be set up as a separate library. There has to be some curation as to what goes into the stdlib as it adds maintenance costs.

lerno · 2025-07-11T12:20:05Z

Are you alright with closing this @sudison and offering it through the regular process of an incubator?

sudison mentioned this pull request Jul 1, 2025

Add Json unmarshal #2258

Closed

BWindey reviewed Jul 1, 2025

View reviewed changes

sudison added 2 commits July 1, 2025 21:57

Address comments

6fab565

Fix ut

627e856

BWindey reviewed Jul 2, 2025

View reviewed changes

sudison added 2 commits July 2, 2025 07:08

Address comments

4553cba

Fix memory leak

f9c8864

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Add JSON marshaling library #2255

Add JSON marshaling library #2255

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

		String? field_result = marshal_value($member.get(value));
		if (catch err = field_result) return err?;

		String? result = json::marshal_value(42);
		assert(result!! == "42");

		Object* parsed = json::tparse_string(json)!!;
		defer parsed.free();

		@param allocator : "The allocator to use for the result"
		@param value: "The struct value to marshal"

Uh oh!

Add JSON marshaling library #2255

Are you sure you want to change the base?

Add JSON marshaling library #2255

Uh oh!

Conversation

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!