New LogicArrayOf datatype

[desmonddak]

Description & Motivation

It would be nice to not lose abstraction when we pack typed Logics (like LogicStructures) into a LogicArray. This helper class gives us a new datatype that retains the type and still supports array operations.

Related Issue(s)

None.

Testing

Basic testing of the class.

Backwards-compatibility

Is this a breaking change that will not be backwards-compatible? If yes, how so?

No

Documentation

Does the change require any updates to documentation? If so, where? Are they included?

Yes, the ROHD API for LogicArray is extended to cover this class.

[mkorbel1]

Note: we should consider how this interacts with Enums (#599)

[desmonddak]

I'm wondering if we should convert LogicArray to derive from LogicArrayOf<Logic> and make sure that we really have only one base type and all methods and tests for LogicArray work to reduce the chance of duplicate code / bugs

[mkorbel1]

I'm wondering if we should convert LogicArray to derive from LogicArrayOf and make sure that we really have only one base type and all methods and tests for LogicArray work to reduce the chance of duplicate code / bugs

That is a cool idea, I haven't looked deeply into your implementation yet but conceptually that sounds good

[desmonddak]

I tested this with PR #599, and it is compatible; we can create LogicArrayOf<LogicEnum>.
Now LogicArray is an extension of LogicArrayOf<Logic>, and we have flattening routines if we want to remove nesting that we may have created (added an outer flatten for LogicStructure as well). This gives us flexibility in composing more complex shapes and not having gaps in managing them.

[Copilot]

🟡 Changes recommended

Moderate issues remain in array detection, net propagation, constant flattening, value reads, and nested-structure synthesis.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds typed multidimensional logic/value arrays while integrating them with ports, interfaces, simulation, synthesis, netlists, tests, and documentation.

Changes:

• Introduces LogicArrayOf, LogicValueArray, and LogicValueArrayOf.
• Generalizes array infrastructure through BaseLogicArray.
• Adds structure flattening and typed-array synthesis support.

File summaries

File	Description
test/logic_structure_test.dart	Tests structure flattening.
test/logic_array_of_test.dart	Tests typed and value arrays.
lib/src/utilities/simcompare.dart	Supports generalized arrays in simulation comparisons.
lib/src/synthesizers/utilities/synth_structure_layout.dart	Excludes base arrays from structure expansion.
lib/src/synthesizers/utilities/synth_module_definition.dart	Synthesizes typed-array descendants.
lib/src/synthesizers/utilities/synth_logic.dart	Adds structured-array field references.
lib/src/synthesizers/utilities/synth_array_slice.dart	Generalizes array slicing.
lib/src/synthesizers/utilities/synth_array_concat.dart	Generalizes array concatenation.
lib/src/synthesizers/systemverilog/systemverilog_synth_module_definition.dart	Extends SystemVerilog array handling.
lib/src/synthesizers/netlist/netlist_utils.dart	Emits typed-array metadata.
lib/src/synthesizers/netlist/netlist_synthesizer.dart	Handles generalized array aliases.
lib/src/synthesizers/netlist/netlist_synth_module_definition.dart	Adds typed-array slice and concatenation cells.
lib/src/synthesizers/netlist/netlist_module_translation.dart	Translates generalized array ports.
lib/src/signals/signals.dart	Registers the new signal types.
lib/src/signals/logic.dart	Recognizes base-array membership.
lib/src/signals/logic_value_array.dart	Implements packed value arrays.
lib/src/signals/logic_value_array_of.dart	Implements semantic value arrays.
lib/src/signals/logic_structure.dart	Adds outer structure flattening.
lib/src/signals/logic_array.dart	Introduces shared array infrastructure.
lib/src/signals/logic_array_of.dart	Implements typed logic arrays.
lib/src/module.dart	Extends typed port handling.
lib/src/interfaces/interface.dart	Preserves typed arrays through interfaces.
doc/user_guide/_docs/A20-logic-arrays.md	Documents typed and value arrays.
doc/user_guide/_docs/A19-logic-structures.md	Documents structure flattening.
.devcontainer/devcontainer.json	Adds Node.js 24.

Review details

Suppressed comments (3)

lib/src/signals/logic_array_of.dart:169

• The inherited named contract allows callers to select a Naming, but this override discards the argument and always rebuilds the array with default naming. For example, typed.named('x', naming: Naming.reserved).naming is not reserved, unlike Logic and LogicArray; propagate naming through the typed-array construction/clone path.

  @override
  LogicArrayOf<T> named(String name, {Naming? naming}) =>
      clone(name: name)..gets(this);

lib/src/signals/logic_array_of.dart:112

• A nested LogicArray may legally be empty, but after expanding such leaves this list is empty and leaves.first throws an unrelated state/range error. Since LogicArrayOf cannot construct zero-sized dimensions, reject this case explicitly with LogicConstructionException before accessing the prototype.

    final prototype = leaves.first as U;

lib/src/signals/logic_value_array.dart:118

• Zero-sized inner dimensions are explicitly accepted by the constructor, but for a shape such as [2, 0], length is zero and this loop yields no slices instead of two empty [0] slices. Iterate over dimensions.first rather than the flattened value count so majorSlices preserves the outer shape and can be stacked back correctly.

    for (var start = 0; start < length; start += sliceLength) {
      yield LogicValueArray(sliceDimensions, elementWidth,
          _values.getRange(start, start + sliceLength));
    }

• Files reviewed: 25/25 changed files
• Comments generated: 5
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[desmonddak]

A few more related fixes regarding naming support, rejecting unassignable leaves, cloning losing metadata and subclass identity

[mkorbel1]

I'd like to do an initial iteration on the architecture and public API before diving deeper into implementation details. Retaining specialized signal types through array operations makes sense, but I think the current public surface offers too many overlapping ways to do the same things. There are also three reproducible correctness issues in the inline comments below.

Prefer One Typed API for Each Operation

Please carry the appropriate types through the primary APIs wherever that is sound and backwards-compatible, instead of adding parallel typed/untyped accessors or separate methods for equivalent operations. An additional public entry point should have a distinct behavior or an explicit compatibility reason. Some concrete places to consider:

Area	Current APIs / Locations	Suggested Direction
Array elements	arrayElements and typedLeafElements	Expose List<T> arrayElements on the typed array, keeping it unmodifiable, rather than introducing another name for the same objects.
Element indexing	at and elementAt	Use one accessor returning T on the typed array. These currently perform the same lookup; one only casts the result.
Indexed traversal	indexedLeaves and indexedElements	Use one typed array-element traversal instead of two names that differ only in the static type of the returned element.
Value construction	LogicValueArray, fromInts, and LogicValueArrayOf	Consider accepting nested input directly, with an explicit fromFlat path for flat values plus shape metadata. These are genuinely different input forms. Flat internal storage need not force callers to flatten multidimensional data; the constructor comment below gives an example and inference limits.
Value assignment	putInto, its typed-value counterpart, and putLogicValues / putValueArrayOf	Consider accepting the new value types through the existing target-side put API. LogicArray(dimensions, width)..put(values) already provides construction/composition convenience.
Fluent operations	getsEach / getsGenerated	These return this, but widen its type to BaseLogicArray. Preserve the useful receiver type rather than losing typed access while chaining calls.
Shape operations	reshape / transpose2D / majorSlices	Preserve typed element access where meaningful. Reshape/transpose currently create ordinary LogicArrays, so reshaping an array of Samples loses .data/.valid access. Define the unpacked-dimension policy rather than silently resetting it.

This does not mean changing recursive leafElements to List<T>: for a Sample structure, those leaves are its fields, not Sample instances. Keep genuinely different traversal boundaries, but document them clearly and avoid duplicate names for the same boundary. The inline documentation comment gives examples.

For put, retain scalar fallback behavior and test inherited inject, which dispatches through put. Validate shaped inputs before writing elements, without tightening existing packed-value behavior by accident. There is no need to invent broadcasting semantics for fill: existing LogicValue.of rejects fill: true for multi-bit values, so reject it where it is inapplicable rather than silently inventing a new meaning.

Keep Implementation Details Out of the Public Contract

Is BaseLogicArray intended as a supported user extension point, or just shared implementation? If the latter, I'd prefer public APIs in terms of LogicArrayOf<T> and the existing LogicArray, without another publicly constructible array type.

Please consider the duplicate base constructors/net/port factories, the base type in fromLogicArray and putInto signatures, and its exposure in traversal return types above. Also clarify the supported subclass contract around clone and createClone. Hiding an export alone will not remove those dependencies. A minimal public abstract contract may be justified for independent array implementations, but that use case should be explicit. Dart's library-scoped privacy is an implementation constraint, not by itself a reason to expose the concrete base to users.

Similarly, do the general-purpose ExactZip and Unzip extensions need to become part of ROHD's public API for this feature, or can they remain implementation helpers?

Consider a Value Subtype with a Packed Fallback

Could LogicValueArray be a LogicValue, analogous to LogicArray being a Logic, so arrays can return a more specific type through the existing value getter instead of adding logicValues? The existing LogicStructure.packed pattern seems worth evaluating here: ordinary values could return themselves from packed, while shaped values provide an ordinary packed representation for fallback bit operations.

This is a design proposal, not a claim that changing the superclass alone is sufficient. Places to consider include the LogicValueArray declaration, the signal's value/logicValues accessors, previousValue, and the conversion/assignment APIs. The LogicValue contract must still hold: width and deprecated length count bits, [] selects bits, and equality/hash behavior depends on width and bits rather than shape. LogicValueArray.length currently counts elements, so that would need a different name. Existing same-width packed assignments must also remain valid.

The bitwise dispatch and equality paths need to support ordinary/shaped operands in both orders, with compatible hashes. The values library's private implementation hooks also need integration. If these compatibility/performance costs justify keeping composition, please explain that tradeoff; splitting the value-array feature into a follow-up is also reasonable rather than freezing a parallel API now.

Next Iteration

Please address the inline defects and propose the intended public API/support boundaries before another deep implementation pass. Add focused tests for the settled contracts: typed access and transformations, clone metadata, empty shapes, and value round-trips. If value arrays remain, the constructor input forms and codec behavior raised inline also need clear contracts and tests.

For synthesis coverage, the current netlist['modules'] nonempty assertion is too weak to verify the new support. Please check bit connectivity, field metadata, and slice/concat offsets, ideally using unequal field widths and a multidimensional/submodule case. This is a coverage request, not a claim that a specific netlist is incorrect. Please also add a changelog entry once the API settles.

The focused array/structure suites passed on VM and Node, and the selected existing regression suites passed, but targeted probes exposed the issues below. I think this is enough feedback for an initial revision. This is an architecture/API review with selected correctness checks, not an exhaustive correctness sign-off; I would revisit deeper synthesis, subclassing, and broader integration coverage after these decisions are settled.

[Copilot]

🟡 Changes recommended

Mixed net kinds are accepted incorrectly, and PairInterface cloning loses typed-array structure.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 30/30 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Nested arrays can incorrectly receive a conflicting structure-pack driver during netlist translation.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 32/32 changed files
• Comments generated: 1
• Review effort level: Balanced

[mkorbel1]

The restructure addresses the six original inline items and substantially improves the public API: the base is private, typed traversal is consolidated, value arrays use the LogicValue/packed model, and nested construction and netlist coverage are in place.

Three points remain below: preserving existing value-assignment behavior, finishing consolidation around the public put API rather than retaining putInto, and filling the identified documentation/test gaps. The first point is a reproduced backwards-compatibility regression; the third is a request for clearer contracts and coverage, not a claim that every untested operation is broken.

The focused suites passed locally (110 tests on the VM and 41 on Node), but two targeted compatibility probes still fail. Please address these items. Deeper review is still ongoing, and this follow-up is not an exhaustive correctness review or approval.

[Copilot]

🟡 Changes recommended

Nested codec compatibility and constant-containing structure flattening have unresolved correctness issues.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 33/33 changed files
• Comments generated: 2
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

The broad public API and synthesis changes warrant final human review, and one constructor error message remains inaccurate.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

lib/src/signals/typed_logic_array.dart:449

• An empty dimension list reaches this combined branch, but the reported reason says only that dimensions must be non-negative; every member of an empty list satisfies that condition. Split the checks so callers are told that at least one dimension is required, matching BaseLogicArray and the value-array validators.

• Files reviewed: 33/33 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[mkorbel1]

The two-type model now carries the hardware element type and semantic value type through the primary API, and the latest changes address the three points from my previous review: packed assignment and one-bit fill compatibility are restored, public putInto is removed, and the requested documentation/coverage is substantially expanded. The old compatibility probes pass unchanged.

At 23c9fa73d, the focused checks I ran passed: 668 checked-in tests on the VM with one skip, 57 on Node, and static analysis of lib and test. I inspected the subsequent delta through c5f74cd1e, but these test results apply to the earlier revision; I am not claiming a fresh test run on the latest head. This is not a full-suite or exhaustive correctness sign-off.

The latest commits add the sibling-codec identity check and recursive driveable cloning for promoted structures containing constants, with regression tests for both earlier findings. I am not repeating those defect requests here; the remaining feedback concerns API design, scope, and documentation.

The codec is a natural fit for paired hardware and value types such as ROHD-HCL's FloatingPoint and FloatingPointValue; I am not requesting a redesign. The additional inline points concern a small semantic-value ownership documentation clarification, the justification for a public BaseLogicArray, naming the value type TypedLogicValueArray for consistency, and the construction/connection contract for reshape, transpose, nested flattening, and the hardware toLogicArray adapter. The non-generic structural base makes sense internally, but cross-library implementation use alone does not establish a need to export another supported public base. We should settle direction-neutral shape construction and connection direction before these new transform APIs become compatibility commitments, and justify the additional reconstruction parameters on nested flattening.

I lean toward removing public getsEach unless there is a strong use case beyond existing gets and assignSubset (and matching their void return if retained), removing getsGenerated unless it adds enough beyond an indexedElements loop, and separating optional shape verification from ordinary gets instead of exposing getsPackedValues. Similarly, mapMajorSlices needs justification beyond composing majorSlices, map, and stack. I also lean toward removing flattenOuter from this PR unless there is a concrete reason to include it; if retained, the construction and connection-direction concerns apply there too. Please also finish documentation for the newly introduced private members. Deeper review is still ongoing.

[mkorbel1]

Thanks for the updates. The public API is getting smaller and the main hardware/value architecture is clearer. This round focuses on the remaining API surface and contracts: constructor visibility, redundant value-array entry points, consistent width arguments, the meaning of dimension labels, and keeping synthesis implementation types explicitly internal.

The eight inline comments also suggest a few targeted synthesis tests for nested array/structure combinations, mixed layouts, and structured inout behavior. These are requests to improve confidence around the new paths, not claims that every untested combination is broken or a request for an exhaustive cross-product test suite.

The previously reopened discussions on construction/connection policy, flattenOuter, remaining private-member documentation, and mapMajorSlices remain separate; I have not duplicated them here. Please address the inline requests and settle those outstanding design questions before we commit to the new public contracts.

At f2d1b5c2d06090d40be31b742ac51026a33dd89e, the four representative tests rerun during this round passed: three exercise generated SV simulation and the mixed-layout ordinary-array case is compile-only. This is not exhaustive correctness coverage or a full-suite sign-off. We are getting through the larger API/design items; the next review round can go deeper into implementation details.