Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix WebAssembly serialization for comments field in CDDL AST #231

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mirelon
Copy link

@mirelon mirelon commented Jan 18, 2025

This pull request addresses issues related to the serialization of the comments field in the CDDL Abstract Syntax Tree (AST) when targeting WebAssembly.

Problem

Previously, the comments field was marked with

#[cfg_attr(target_arch = "wasm32", serde(skip))]

which led to its omission during serialization in WebAssembly environments. While the intention was to avoid serializing the comments field conditionally, this caused discrepancies between WebAssembly and non-WebAssembly outputs, and made the behavior inconsistent when the ast-comments feature was enabled.

Solution

To fix this issue:

The

#[cfg_attr(target_arch = "wasm32", serde(skip))]

attribute has been removed from the comments field in the CDDL struct.
Both Comments and NonMemberKey structs, which are part of the AST, were updated with

#[cfg_attr(target_arch = "wasm32", derive(Serialize))]

This ensures they are properly serialized when compiling for WebAssembly.
By applying these changes, the comments field is now serialized correctly in WebAssembly environments when the ast-comments feature is enabled, aligning its behavior across all compilation targets.

Benefits

  • Consistency: The comments field is now included in the serialized output, regardless of the compilation target.
  • Flexibility: Users can still disable the comments field entirely by turning off the ast-comments feature in their Cargo.toml.
  • Compatibility: WebAssembly and non-WebAssembly outputs behave identically when the ast-comments feature is enabled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant