Skip to content

docs: auto-update for PR #2968 — test(wasm-abi): restore ignored normalize_type tests#2988

Closed
meroreviewer[bot] wants to merge 2 commits into
masterfrom
docs/auto-pr2968-f62a683
Closed

docs: auto-update for PR #2968 — test(wasm-abi): restore ignored normalize_type tests#2988
meroreviewer[bot] wants to merge 2 commits into
masterfrom
docs/auto-pr2968-f62a683

Conversation

@meroreviewer

@meroreviewer meroreviewer Bot commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Automatic Documentation Update

Opened automatically after PR #2968 merged.

Each block shows the documentation change as a diff (added lines in green, removed in red); expand "Why this changed" for the source rationale.

Documentation changes

architecture/abi-conformance.html — Reject non-u8 fixed-array element types with UnsupportedArrayElement; Skip String-key validation for CRDT map types to avoid regression; Re-enable 11 previously-ignored normalize_type tests

 Verify the coverage gate passes with an empty allowlist — do not add an exempti…
 Never add a new variant to the schema without a matching conformance field. A C…
+Fixed-size array type restrictions
+The ABI normalizer enforces a strict rule on fixed-size array types ([T; N]):
+only [u8; N] is valid. Any fixed-size array whose element type is not u8 is
+immediately rejected with NormalizeError::UnsupportedArrayElement instead of
+silently falling through to list normalization and producing an incorrect ABI
+representation.
+[u8; 32], [u8; 64], etc. — accepted; serialized as a fixed-length bytes type.
+[u32; 8], [String; 4], etc. — rejected with UnsupportedArrayElement.
+Note that Vec<u8> and newtype wrappers around byte arrays remain valid and are
+normalized to the bytes ABI type as before.
+CRDT map key validation
+The stricter key-type validation introduced for standard maps is guarded by an
+is_crdt_map check. UnorderedMap, SortedMap, and AuthoredMap skip the key
+validation step to avoid regressions in existing app code (e.g.
+UnorderedMap<Slug, V> where Slug is a newtype key). Standard BTreeMap and
+HashMap types remain subject to key validation.
 File locations
 apps/abi_conformance/src/lib.rs — AbiState struct definition; add new fields he…
 apps/abi_conformance/abi.expected.json — golden ABI snapshot; regenerate after …
 crates/primitives/src/crdt.rs — authoritative CrdtCollectionType enum.
+crates/wasm-abi/tests/normalize.rs — 21 normalization tests (all active); covers
+array acceptance/rejection, map key validation, bytes types, record/variant
+types, and nested generics.
 scripts/check_crdt_coverage.sh (or equivalent CI step) — the coverage gate; all…
 Relationship to other conformance corpus entries
Why this changed (source: PR #2968)

Previously, [T; N] arrays whose element type was not u8 silently fell through to list normalization, producing an incorrect ABI representation. Now, any fixed-size array with a non-u8 element type immediately returns NormalizeError::UnsupportedArrayElement instead.

The key-validation logic introduced for standard maps is guarded by an is_crdt_map check that skips validation for UnorderedMap, SortedMap, and AuthoredMap. Without this guard, existing app code using UnorderedMap<Slug, V> would fail ABI emission after the stricter validation was applied universally.

Eleven tests in crates/wasm-abi/tests/normalize.rs that were marked #[ignore] are now active. They cover BTreeMap key validation, invalid map keys, [u8; N] array acceptance, non-u8 array rejection, Vec<u8> bytes, newtype bytes, record/variant types, unknown external types, unit type, nested generics, and complex nested scenarios. All 21 normalize tests now pass.

architecture/error-flows.html — Collect normalize_type errors in AbiEmitter instead of panicking

 cascade_duration_ms — histogram for time spent in cascade cleanup
 No upper limit on contexts per member — cascade is O(n) in number of contexts t…
+9. ABI Emitter — Collected normalize_type Errors
+Previously, AbiEmitter called normalize_type(...).unwrap() or
+.unwrap_or_else(|e| panic!(...)) at every call site. A single unsupported type —
+such as a new UnsupportedArrayElement or a stricter UnsupportedMapKey — would
+abort the entire build script with an opaque panic, hiding all other type errors
+in the same crate.
+What Changed
+AbiEmitter gained a normalize_errors: Vec<String> field that accumulates every
+failed normalize_type call.
+A new helper method normalize_or_record replaces all former unwrap/panic call
+sites. On error it appends a descriptive context string to the vector and
+returns the placeholder type string so AST visitation continues uninterrupted.
+After all AST passes complete, emit_manifest_from_crate inspects
+normalize_errors. If it is non-empty, all collected messages are joined into a
+single combined diagnostic and returned as an Err — no panic is ever allowed to
+propagate.
+Error Flow
+Encounter unsupported type — normalize_type returns Err(UnsupportedArrayElement
+| UnsupportedMapKey | …).
+Record, don't abort — normalize_or_record pushes "context: <error message>" onto
+normalize_errors and returns "string" as a stand-in so the remaining fields and
+methods continue to be visited.
+Full-pass collection — every unsupported type anywhere in the crate is recorded
+in one compilation, not just the first one encountered.
+Combined diagnostic — emit_manifest_from_crate returns a multi-line Err
+enumerating all unsupported types. The build script surfaces this as a single,
+readable compiler error.
+Before vs. After
+// Before — panics on first failure, hides the rest
+let ty = normalize_type(&raw_ty).unwrap_or_else(|e| panic!("unsupported: {e}"));
+// After — records and continues; errors surfaced together at the end
+let ty = self.normalize_or_record(&raw_ty, "MyStruct::field_name");
+// → on failure: pushes "MyStruct::field_name: UnsupportedArrayElement(…)"
+// returns "string" placeholder, visitation continues
+Developer Impact
+Build failures caused by unsupported ABI types now produce a single, enumerated
+error message listing every problematic field in one shot.
+No more opaque thread 'main' panicked at … output from the build script; the
Why this changed (source: PR #2968)

Added a normalize_errors: Vec<String> field to AbiEmitter and a normalize_or_record helper method. All call sites that previously called normalize_type(...).unwrap() or .unwrap_or_else(|e| panic!(...)) now call normalize_or_record, which records the error with a descriptive context string and returns a placeholder string type so visitation can continue. After all AST passes, emit_manifest_from_crate checks the collected errors and returns them as a combined diagnostic error instead of allowing a panic to propagate.


Generated by ai-reviewer update-docs. Nothing was auto-merged.

@meroreviewer meroreviewer Bot added automated-docs documentation Improvements or additions to documentation labels Jun 27, 2026

@meroreviewer meroreviewer Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🤖 AI Code Reviewer

Reviewed by 1 agents | Quality score: 85% | Review time: 201.1s


✅ No Issues Found

All agents reviewed the code and found no issues. LGTM! 🎉


🤖 Generated by AI Code Reviewer | Review ID: review-9c8dfd6a

@chefsale chefsale closed this Jun 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automated-docs documentation Improvements or additions to documentation external

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant