From 82e900e48a66704cf22c385959e5ee1d6a56be7b Mon Sep 17 00:00:00 2001 From: "Desmond A. Kirkpatrick" Date: Fri, 17 Apr 2026 08:30:50 -0700 Subject: [PATCH 1/8] move synthesis naming to a common naming utility so all synthesizers agree on names --- lib/src/module.dart | 39 +- lib/src/synthesizers/synth_builder.dart | 5 +- lib/src/synthesizers/synthesizer.dart | 22 +- .../systemverilog_synthesizer.dart | 6 +- .../synthesizers/utilities/synth_logic.dart | 81 +-- .../utilities/synth_module_definition.dart | 60 +- .../synth_sub_module_instantiation.dart | 14 +- lib/src/utilities/signal_namer.dart | 271 ++++++++ test/naming_cases_test.dart | 583 ++++++++++++++++++ test/naming_consistency_test.dart | 247 ++++++++ 10 files changed, 1215 insertions(+), 113 deletions(-) create mode 100644 lib/src/utilities/signal_namer.dart create mode 100644 test/naming_cases_test.dart create mode 100644 test/naming_consistency_test.dart diff --git a/lib/src/module.dart b/lib/src/module.dart index 0fd51eac7..09e11fdc7 100644 --- a/lib/src/module.dart +++ b/lib/src/module.dart @@ -1,4 +1,4 @@ -// Copyright (C) 2021-2025 Intel Corporation +// Copyright (C) 2021-2026 Intel Corporation // SPDX-License-Identifier: BSD-3-Clause // // module.dart @@ -11,12 +11,12 @@ import 'dart:async'; import 'dart:collection'; import 'package:meta/meta.dart'; - import 'package:rohd/rohd.dart'; import 'package:rohd/src/collections/traverseable_collection.dart'; import 'package:rohd/src/diagnostics/inspector_service.dart'; import 'package:rohd/src/utilities/config.dart'; import 'package:rohd/src/utilities/sanitizer.dart'; +import 'package:rohd/src/utilities/signal_namer.dart'; import 'package:rohd/src/utilities/timestamper.dart'; import 'package:rohd/src/utilities/uniquifier.dart'; @@ -52,6 +52,41 @@ abstract class Module { /// An internal mapping of input names to their sources to this [Module]. late final Map _inputSources = {}; + // ─── Canonical naming (SignalNamer) ───────────────────────────── + + /// Lazily-constructed namer that owns the [Uniquifier] and the + /// sparse Logic→String cache. Initialized on first access. + @internal + late final SignalNamer signalNamer = _createSignalNamer(); + + SignalNamer _createSignalNamer() { + assert(hasBuilt, 'Module must be built before canonical names are bound.'); + return SignalNamer.forModule( + inputs: _inputs, + outputs: _outputs, + inOuts: _inOuts, + ); + } + + /// Returns the collision-free signal name for [logic] within this module. + String signalName(Logic logic) => signalNamer.nameOf(logic); + + /// Allocates a collision-free signal name in this module's namespace. + /// + /// Used by synthesizers to name connection nets, submodule instances, + /// intermediate wires, and other artifacts that have no user-created + /// [Logic] object. The returned name is guaranteed not to collide with + /// any signal name or any previously allocated name. + /// + /// When [reserved] is `true`, the exact [baseName] (after sanitization) is + /// claimed without modification; an exception is thrown if it collides. + String allocateSignalName(String baseName, {bool reserved = false}) => + signalNamer.allocate(baseName, reserved: reserved); + + /// Returns `true` if [name] has not yet been claimed as a signal name in + /// this module's namespace. + bool isSignalNameAvailable(String name) => signalNamer.isAvailable(name); + /// An internal mapping of inOut names to their sources to this [Module]. late final Map _inOutSources = {}; diff --git a/lib/src/synthesizers/synth_builder.dart b/lib/src/synthesizers/synth_builder.dart index 54e312ab3..3b3a6011c 100644 --- a/lib/src/synthesizers/synth_builder.dart +++ b/lib/src/synthesizers/synth_builder.dart @@ -1,4 +1,4 @@ -// Copyright (C) 2021-2025 Intel Corporation +// Copyright (C) 2021-2026 Intel Corporation // SPDX-License-Identifier: BSD-3-Clause // // synth_builder.dart @@ -56,6 +56,9 @@ class SynthBuilder { } } + // Allow the synthesizer to prepare with knowledge of top module(s) + synthesizer.prepare(this.tops); + final modulesToParse = [...tops]; for (var i = 0; i < modulesToParse.length; i++) { final moduleI = modulesToParse[i]; diff --git a/lib/src/synthesizers/synthesizer.dart b/lib/src/synthesizers/synthesizer.dart index b70c9338e..2d7730208 100644 --- a/lib/src/synthesizers/synthesizer.dart +++ b/lib/src/synthesizers/synthesizer.dart @@ -1,4 +1,4 @@ -// Copyright (C) 2021-2023 Intel Corporation +// Copyright (C) 2021-2026 Intel Corporation // SPDX-License-Identifier: BSD-3-Clause // // synthesizer.dart @@ -6,18 +6,34 @@ // // 2021 August 26 // Author: Max Korbel -// import 'package:rohd/rohd.dart'; /// An object capable of converting a module into some new output format abstract class Synthesizer { + /// Called by [SynthBuilder] before synthesis begins, with the top-level + /// module(s) being synthesized. + /// + /// Override this method to perform any initialization that requires + /// knowledge of the top module, such as resolving port names to [Logic] + /// objects, or computing global signal sets. + /// + /// The default implementation does nothing. + void prepare(List tops) {} + /// Determines whether [module] needs a separate definition or can just be /// described in-line. bool generatesDefinition(Module module); /// Synthesizes [module] into a [SynthesisResult], given the mapping provided /// by [getInstanceTypeOfModule]. + /// + /// Optionally a [lookupExistingResult] callback may be supplied which + /// allows the synthesizer to query already-generated `SynthesisResult`s + /// for child modules (useful when building parent output that needs + /// information from children). SynthesisResult synthesize( - Module module, String Function(Module module) getInstanceTypeOfModule); + Module module, String Function(Module module) getInstanceTypeOfModule, + {SynthesisResult? Function(Module module)? lookupExistingResult, + Map? existingResults}); } diff --git a/lib/src/synthesizers/systemverilog/systemverilog_synthesizer.dart b/lib/src/synthesizers/systemverilog/systemverilog_synthesizer.dart index d8b5bae36..b83acb9cc 100644 --- a/lib/src/synthesizers/systemverilog/systemverilog_synthesizer.dart +++ b/lib/src/synthesizers/systemverilog/systemverilog_synthesizer.dart @@ -1,4 +1,4 @@ -// Copyright (C) 2021-2025 Intel Corporation +// Copyright (C) 2021-2026 Intel Corporation // SPDX-License-Identifier: BSD-3-Clause // // systemverilog_synthesizer.dart @@ -137,7 +137,9 @@ class SystemVerilogSynthesizer extends Synthesizer { @override SynthesisResult synthesize( - Module module, String Function(Module module) getInstanceTypeOfModule) { + Module module, String Function(Module module) getInstanceTypeOfModule, + {SynthesisResult? Function(Module module)? lookupExistingResult, + Map? existingResults}) { assert( module is! SystemVerilog || module.generatedDefinitionType != DefinitionGenerationType.none, diff --git a/lib/src/synthesizers/utilities/synth_logic.dart b/lib/src/synthesizers/utilities/synth_logic.dart index 64ed3bed1..4a9c0e20a 100644 --- a/lib/src/synthesizers/utilities/synth_logic.dart +++ b/lib/src/synthesizers/utilities/synth_logic.dart @@ -12,7 +12,6 @@ import 'package:meta/meta.dart'; import 'package:rohd/rohd.dart'; import 'package:rohd/src/synthesizers/utilities/utilities.dart'; import 'package:rohd/src/utilities/sanitizer.dart'; -import 'package:rohd/src/utilities/uniquifier.dart'; /// Represents a logic signal in the generated code within a module. @internal @@ -196,81 +195,25 @@ class SynthLogic { /// The name of this, if it has been picked. String? _name; - /// Picks a [name]. + /// Picks a [name] using the module's signal namer. /// /// Must be called exactly once. - void pickName(Uniquifier uniquifier) { + void pickName() { assert(_name == null, 'Should only pick a name once.'); - _name = _findName(uniquifier); + _name = _findName(); } /// Finds the best name from the collection of [Logic]s. - String _findName(Uniquifier uniquifier) { - // check for const - if (_constLogic != null) { - if (!_constNameDisallowed) { - return _constLogic!.value.toString(); - } else { - assert( - logics.length > 1, - 'If there is a constant, but the const name is not allowed, ' - 'there needs to be another option'); - } - } - - // check for reserved - if (_reservedLogic != null) { - return uniquifier.getUniqueName( - initialName: _reservedLogic!.name, reserved: true); - } - - // check for renameable - if (_renameableLogic != null) { - return uniquifier.getUniqueName( - initialName: _renameableLogic!.preferredSynthName); - } - - // pick a preferred, available, mergeable name, if one exists - final unpreferredMergeableLogics = []; - final uniquifiableMergeableLogics = []; - for (final mergeableLogic in _mergeableLogics) { - if (Naming.isUnpreferred(mergeableLogic.name)) { - unpreferredMergeableLogics.add(mergeableLogic); - } else if (!uniquifier.isAvailable(mergeableLogic.preferredSynthName)) { - uniquifiableMergeableLogics.add(mergeableLogic); - } else { - return uniquifier.getUniqueName( - initialName: mergeableLogic.preferredSynthName); - } - } - - // uniquify a preferred, mergeable name, if one exists - if (uniquifiableMergeableLogics.isNotEmpty) { - return uniquifier.getUniqueName( - initialName: uniquifiableMergeableLogics.first.preferredSynthName); - } - - // pick an available unpreferred mergeable name, if one exists, otherwise - // uniquify an unpreferred mergeable name - if (unpreferredMergeableLogics.isNotEmpty) { - return uniquifier.getUniqueName( - initialName: unpreferredMergeableLogics - .firstWhereOrNull((element) => - uniquifier.isAvailable(element.preferredSynthName)) - ?.preferredSynthName ?? - unpreferredMergeableLogics.first.preferredSynthName); - } - - // pick anything (unnamed) and uniquify as necessary (considering preferred) - // no need to prefer an available one here, since it's all unnamed - return uniquifier.getUniqueName( - initialName: _unnamedLogics - .firstWhereOrNull((element) => - !Naming.isUnpreferred(element.preferredSynthName)) - ?.preferredSynthName ?? - _unnamedLogics.first.preferredSynthName); - } + /// + /// Delegates to signal namer which handles constant value naming, priority + /// selection, and uniquification via the module's shared namespace. + String _findName() => + parentSynthModuleDefinition.module.signalNamer.nameOfBest( + logics, + constValue: _constLogic, + constNameDisallowed: _constNameDisallowed, + ); /// Creates an instance to represent [initialLogic] and any that merge /// into it. diff --git a/lib/src/synthesizers/utilities/synth_module_definition.dart b/lib/src/synthesizers/utilities/synth_module_definition.dart index b8b78476a..dac9075e8 100644 --- a/lib/src/synthesizers/utilities/synth_module_definition.dart +++ b/lib/src/synthesizers/utilities/synth_module_definition.dart @@ -14,7 +14,6 @@ import 'package:meta/meta.dart'; import 'package:rohd/rohd.dart'; import 'package:rohd/src/collections/traverseable_collection.dart'; import 'package:rohd/src/synthesizers/utilities/utilities.dart'; -import 'package:rohd/src/utilities/uniquifier.dart'; /// A version of [BusSubset] that can be used for slicing on [LogicStructure] /// ports. @@ -110,10 +109,6 @@ class SynthModuleDefinition { @override String toString() => "module name: '${module.name}'"; - /// Used to uniquify any identifiers, including signal names - /// and module instances. - final Uniquifier _synthInstantiationNameUniquifier; - /// Indicates whether [logic] has a corresponding present [SynthLogic] in /// this definition. @internal @@ -289,14 +284,7 @@ class SynthModuleDefinition { /// Creates a new definition representation for this [module]. SynthModuleDefinition(this.module) - : _synthInstantiationNameUniquifier = Uniquifier( - reservedNames: { - ...module.inputs.keys, - ...module.outputs.keys, - ...module.inOuts.keys, - }, - ), - assert( + : assert( !(module is SystemVerilog && module.generatedDefinitionType == DefinitionGenerationType.none), @@ -465,6 +453,7 @@ class SynthModuleDefinition { final receiverIsSubModuleOutput = receiver.isOutput && (receiver.parentModule?.parent == module); + if (receiverIsSubModuleOutput) { final subModule = receiver.parentModule!; @@ -513,6 +502,7 @@ class SynthModuleDefinition { _collapseArrays(); _collapseAssignments(); _assignSubmodulePortMapping(); + _pruneUnused(); process(); _pickNames(); @@ -752,49 +742,59 @@ class SynthModuleDefinition { } /// Picks names of signals and sub-modules. + /// + /// Signal names are read from [Module.signalName] (for user-created + /// [Logic] objects) or kept as literal constants. Submodule instance + /// names and synthesizer artifacts are allocated from the shared + /// [Module] namespace via [Module.allocateSignalName], guaranteeing no + /// collisions across synthesizers. void _pickNames() { - // first ports get priority + // Name allocation order matters — earlier claims get the unsuffixed name + // when there are collisions. This matches production ROHD priority: + // 1. Ports (reserved by _initNamespace, claimed via signalName) + // 2. Reserved submodule instances + // 3. Reserved internal signals + // 4. Non-reserved submodule instances + // 5. Non-reserved internal signals for (final input in inputs) { - input.pickName(_synthInstantiationNameUniquifier); + input.pickName(); } for (final output in outputs) { - output.pickName(_synthInstantiationNameUniquifier); + output.pickName(); } for (final inOut in inOuts) { - inOut.pickName(_synthInstantiationNameUniquifier); + inOut.pickName(); } - // pick names of *reserved* submodule instances - final nonReservedSubmodules = []; + // Reserved submodule instances first (they assert their exact name). for (final submodule in subModuleInstantiations) { if (submodule.module.reserveName) { - submodule.pickName(_synthInstantiationNameUniquifier); + submodule.pickName(module); assert(submodule.module.name == submodule.name, 'Expect reserved names to retain their name.'); - } else { - nonReservedSubmodules.add(submodule); } } - // then *reserved* internal signals get priority + // Reserved internal signals next. final nonReservedSignals = []; for (final signal in internalSignals) { if (signal.isReserved) { - signal.pickName(_synthInstantiationNameUniquifier); + signal.pickName(); } else { nonReservedSignals.add(signal); } } - // then submodule instances - for (final submodule in nonReservedSubmodules - .where((element) => element.needsInstantiation)) { - submodule.pickName(_synthInstantiationNameUniquifier); + // Then non-reserved submodule instances. + for (final submodule in subModuleInstantiations) { + if (!submodule.module.reserveName && submodule.needsInstantiation) { + submodule.pickName(module); + } } - // then the rest of the internal signals + // Then the rest of the internal signals. for (final signal in nonReservedSignals) { - signal.pickName(_synthInstantiationNameUniquifier); + signal.pickName(); } } diff --git a/lib/src/synthesizers/utilities/synth_sub_module_instantiation.dart b/lib/src/synthesizers/utilities/synth_sub_module_instantiation.dart index 80a415a09..4f1c3e4f2 100644 --- a/lib/src/synthesizers/utilities/synth_sub_module_instantiation.dart +++ b/lib/src/synthesizers/utilities/synth_sub_module_instantiation.dart @@ -1,4 +1,4 @@ -// Copyright (C) 2021-2025 Intel Corporation +// Copyright (C) 2021-2026 Intel Corporation // SPDX-License-Identifier: BSD-3-Clause // // synth_sub_module_instantiation.dart @@ -11,7 +11,6 @@ import 'dart:collection'; import 'package:rohd/rohd.dart'; import 'package:rohd/src/synthesizers/utilities/utilities.dart'; -import 'package:rohd/src/utilities/uniquifier.dart'; /// Represents an instantiation of a module within another module. class SynthSubModuleInstantiation { @@ -25,13 +24,16 @@ class SynthSubModuleInstantiation { String get name => _name!; /// Selects a name for this module instance. Must be called exactly once. - void pickName(Uniquifier uniquifier) { + /// + /// Names are allocated from [parentModule]'s shared namespace via + /// [Module.allocateSignalName], ensuring no collision with signal names or + /// other submodule instances — even across multiple synthesizers. + void pickName(Module parentModule) { assert(_name == null, 'Should only pick a name once.'); - _name = uniquifier.getUniqueName( - initialName: module.uniqueInstanceName, + _name = parentModule.allocateSignalName( + module.uniqueInstanceName, reserved: module.reserveName, - nullStarter: 'm', ); } diff --git a/lib/src/utilities/signal_namer.dart b/lib/src/utilities/signal_namer.dart new file mode 100644 index 000000000..b7d9dc090 --- /dev/null +++ b/lib/src/utilities/signal_namer.dart @@ -0,0 +1,271 @@ +// Copyright (C) 2026 Intel Corporation +// SPDX-License-Identifier: BSD-3-Clause +// +// signal_namer.dart +// Collision-free signal naming within a module scope. +// +// 2026 April 10 +// Author: Desmond Kirkpatrick + +import 'package:collection/collection.dart'; +import 'package:meta/meta.dart'; +import 'package:rohd/rohd.dart'; +import 'package:rohd/src/utilities/sanitizer.dart'; +import 'package:rohd/src/utilities/uniquifier.dart'; + +/// Assigns collision-free names to [Logic] signals within a single module. +/// +/// Wraps a [Uniquifier] with a sparse Logic→String cache so that each +/// signal is named exactly once and every subsequent lookup is O(1). +/// +/// Port names are reserved at construction time. Internal signals are +/// named lazily on the first [nameOf] call. +@internal +class SignalNamer { + final Uniquifier _uniquifier; + + /// Sparse cache: only entries where the canonical name has been resolved. + /// Ports whose sanitized name == logic.name may be absent (fast-path + /// through [_portLogics] check). + final Map _names = {}; + + /// The set of port [Logic] objects, for O(1) port membership tests. + final Set _portLogics; + + SignalNamer._({ + required Uniquifier uniquifier, + required Map portRenames, + required Set portLogics, + }) : _uniquifier = uniquifier, + _portLogics = portLogics { + _names.addAll(portRenames); + } + + /// Creates a [SignalNamer] for the given module ports. + /// + /// Sanitized port names are reserved in the namespace. Ports whose + /// sanitized name differs from [Logic.name] are cached immediately. + factory SignalNamer.forModule({ + required Map inputs, + required Map outputs, + required Map inOuts, + }) { + final portRenames = {}; + final portLogics = {}; + final portNames = []; + + void collectPort(String rawName, Logic logic) { + final sanitized = Sanitizer.sanitizeSV(rawName); + portNames.add(sanitized); + portLogics.add(logic); + if (sanitized != logic.name) { + portRenames[logic] = sanitized; + } + } + + for (final entry in inputs.entries) { + collectPort(entry.key, entry.value); + } + for (final entry in outputs.entries) { + collectPort(entry.key, entry.value); + } + for (final entry in inOuts.entries) { + collectPort(entry.key, entry.value); + } + + // Claim each port name as reserved so that: + // (a) non-reserved signals can't steal them, and + // (b) a second reserved signal with the same name throws. + final uniquifier = Uniquifier(); + for (final name in portNames) { + uniquifier.getUniqueName(initialName: name, reserved: true); + } + + return SignalNamer._( + uniquifier: uniquifier, + portRenames: portRenames, + portLogics: portLogics, + ); + } + + /// Returns the canonical name for [logic]. + /// + /// The first call for a given [logic] allocates a collision-free name + /// via the underlying [Uniquifier]. Subsequent calls return the cached + /// result in O(1). + String nameOf(Logic logic) { + // Fast path: already named (port rename or previously-queried signal). + final cached = _names[logic]; + if (cached != null) { + return cached; + } + + // Port whose sanitized name == logic.name — already reserved. + if (_portLogics.contains(logic)) { + return logic.name; + } + + // First time seeing this internal signal — derive base name. + String baseName; + // Only treat as reserved for Uniquifier purposes if this is a true + // reserved internal signal (not a submodule port that happens to have + // Naming.reserved). + final isReservedInternal = logic.naming == Naming.reserved && !logic.isPort; + if (logic.naming == Naming.reserved || logic.isArrayMember) { + baseName = logic.name; + } else { + baseName = Sanitizer.sanitizeSV(logic.structureName); + } + + final name = _uniquifier.getUniqueName( + initialName: baseName, + reserved: isReservedInternal, + ); + _names[logic] = name; + return name; + } + + /// The base name that would be used for [logic] before uniquification. + static String baseName(Logic logic) => + (logic.naming == Naming.reserved || logic.isArrayMember) + ? logic.name + : Sanitizer.sanitizeSV(logic.structureName); + + /// Chooses the best name from a pool of merged [Logic] signals. + /// + /// When [constValue] is provided and [constNameDisallowed] is `false`, + /// the constant's value string is used directly as the name (no + /// uniquification). When [constNameDisallowed] is `true`, the constant + /// is excluded from the candidate pool and the normal priority applies. + /// + /// Priority (after constant handling): + /// 1. Port of this module (always wins — its name is already reserved). + /// 2. Reserved internal signal (exact name, throws on collision). + /// 3. Renameable signal. + /// 4. Preferred-available mergeable (base name not yet taken). + /// 5. Preferred-uniquifiable mergeable. + /// 6. Available-unpreferred mergeable. + /// 7. First unpreferred mergeable. + /// 8. Unnamed (prefer non-unpreferred base name). + /// + /// The winning name is allocated once and cached for the chosen [Logic]. + /// All other non-port [Logic]s in [candidates] are also cached to the + /// same name. + String nameOfBest( + Iterable candidates, { + Const? constValue, + bool constNameDisallowed = false, + }) { + // Constant whose literal value string is the name. + if (constValue != null && !constNameDisallowed) { + return constValue.value.toString(); + } + + // Classify using _portLogics membership (context-aware) rather than + // Logic.naming (context-independent), because submodule ports have + // Naming.reserved but should NOT be treated as reserved here. + Logic? port; + Logic? reserved; + Logic? renameable; + final preferredMergeable = []; + final unpreferredMergeable = []; + final unnamed = []; + + for (final logic in candidates) { + if (_portLogics.contains(logic)) { + port = logic; + } else if (logic.isPort) { + // Submodule port — treat as mergeable regardless of intrinsic naming, + // matching SynthModuleDefinition's namingOverride convention. + if (Naming.isUnpreferred(baseName(logic))) { + unpreferredMergeable.add(logic); + } else { + preferredMergeable.add(logic); + } + } else if (logic.naming == Naming.reserved) { + reserved = logic; + } else if (logic.naming == Naming.renameable) { + renameable = logic; + } else if (logic.naming == Naming.mergeable) { + if (Naming.isUnpreferred(baseName(logic))) { + unpreferredMergeable.add(logic); + } else { + preferredMergeable.add(logic); + } + } else { + unnamed.add(logic); + } + } + + // Port of this module — name already reserved in namespace. + if (port != null) { + return _nameAndCacheAll(port, candidates); + } + + // Reserved internal — must keep exact name (throws on collision). + if (reserved != null) { + return _nameAndCacheAll(reserved, candidates); + } + + // Renameable — preferred base, uniquified if needed. + if (renameable != null) { + return _nameAndCacheAll(renameable, candidates); + } + + // Preferred-available mergeable. + for (final logic in preferredMergeable) { + if (_uniquifier.isAvailable(baseName(logic))) { + return _nameAndCacheAll(logic, candidates); + } + } + + // Preferred-uniquifiable mergeable. + if (preferredMergeable.isNotEmpty) { + return _nameAndCacheAll(preferredMergeable.first, candidates); + } + + // Unpreferred mergeable — prefer available. + if (unpreferredMergeable.isNotEmpty) { + final best = unpreferredMergeable + .firstWhereOrNull((e) => _uniquifier.isAvailable(baseName(e))) ?? + unpreferredMergeable.first; + return _nameAndCacheAll(best, candidates); + } + + // Unnamed — prefer non-unpreferred base name. + if (unnamed.isNotEmpty) { + final best = + unnamed.firstWhereOrNull((e) => !Naming.isUnpreferred(baseName(e))) ?? + unnamed.first; + return _nameAndCacheAll(best, candidates); + } + + throw StateError('No Logic candidates to name.'); + } + + /// Names [chosen] via [nameOf], then caches the same name for all other + /// non-port [Logic]s in [all]. + String _nameAndCacheAll(Logic chosen, Iterable all) { + final name = nameOf(chosen); + for (final logic in all) { + if (!identical(logic, chosen) && !_portLogics.contains(logic)) { + _names[logic] = name; + } + } + return name; + } + + /// Allocates a collision-free name for a non-signal artifact (wire, + /// instance, etc.). + /// + /// When [reserved] is `true`, the exact [baseName] (after sanitization) + /// is claimed without modification; an exception is thrown if it collides. + String allocate(String baseName, {bool reserved = false}) => + _uniquifier.getUniqueName( + initialName: Sanitizer.sanitizeSV(baseName), + reserved: reserved, + ); + + /// Returns `true` if [name] has not yet been claimed in this namespace. + bool isAvailable(String name) => _uniquifier.isAvailable(name); +} diff --git a/test/naming_cases_test.dart b/test/naming_cases_test.dart new file mode 100644 index 000000000..fbc1d9536 --- /dev/null +++ b/test/naming_cases_test.dart @@ -0,0 +1,583 @@ +// Copyright (C) 2026 Intel Corporation +// SPDX-License-Identifier: BSD-3-Clause +// +// naming_cases_test.dart +// Systematic test of all signal-naming cases in the synthesis pipeline. +// +// 2026 April 10 +// Author: Desmond Kirkpatrick + +// ════════════════════════════════════════════════════════ +// NAMING CROSS-PRODUCT TABLE +// ════════════════════════════════════════════════════════ +// +// Axis 1 — Naming enum (set at Logic construction time): +// reserved Exact name required; collision → exception. +// renameable Keeps name, uniquified on collision; never merged. +// mergeable May merge with equivalent signals; any merged name chosen. +// unnamed No user name; system generates one. +// +// Axis 2 — Context role (per SynthModuleDefinition): +// this-port Port of module being synthesized +// (namingOverride → reserved). +// sub-port Port of a child submodule +// (namingOverride → mergeable). +// internal Non-port signal inside the module (no override). +// const Const object (separate path via constValue). +// +// Axis 3 — Name preference: +// preferred baseName does NOT start with '_' +// unpreferred baseName starts with '_' +// +// Axis 4 — Constant context (only for Const): +// allowed Literal value string used as name. +// disallowed Feeding expressionlessInput; +// must use a wire name. +// +// ────────────────────────────────────────────────────── +// Row Naming Context Pref? Test Valid? +// Effective class → Outcome +// ────────────────────────────────────────────────────── +// 1 reserved this-port pref T1 ✓ +// port (in _portLogics) → exact sanitized name +// 2 reserved this-port unpref T2 ✓ unusual +// port → exact _-prefixed port name +// 3 reserved sub-port pref T3 ✓ +// preferred mergeable → merged, uniquified +// 4 reserved sub-port unpref T4 ✓ +// unpreferred mergeable → low-priority merge +// 5 reserved internal pref T5 ✓ +// reserved internal → exact name, throw on clash +// 6 reserved internal unpref T6 ✓ unusual +// reserved internal → exact _-prefixed name +// 7 renameable this-port pref — can't happen* +// port → exact port name +// 8 renameable sub-port pref — can't happen* +// preferred mergeable → merged +// 9 renameable internal pref T9 ✓ +// renameable → base name, uniquified +// 10 renameable internal unpref T10 ✓ unusual +// renameable → uniquified _-prefixed +// 11 mergeable this-port pref T11 ✓ +// port → exact port name (Logic.port()) +// 12 mergeable this-port unpref T12 ✓ unusual +// port → exact _-prefixed port name +// 13 mergeable sub-port pref T3 ✓ (=row 3) +// preferred mergeable → best-available merge +// 14 mergeable sub-port unpref T4 ✓ (=row 4) +// unpreferred mergeable → low-priority merge +// 15 mergeable internal pref T15 ✓ +// preferred mergeable → prefer available name +// 16 mergeable internal unpref T16 ✓ +// unpreferred mergeable → low-priority merge +// 17 unnamed this-port — — ✗ impossible** +// port → exact port name +// 18 unnamed sub-port — — ✗ impossible** +// mergeable → merged +// 19 unnamed internal (unpf) T19 ✓ +// unnamed → generated _s name +// 20 —(Const) — — T20 ✓ +// const allowed → literal value e.g. 8'h42 +// 21 —(Const) — — T21 ✓ +// const disallowed → wire name (not literal) +// ────────────────────────────────────────────────────── +// +// * Rows 7-8: addInput/addOutput always create +// Logic with Naming.reserved, so a port can +// never have intrinsic Naming.renameable. +// The namingOverride makes it moot anyway. +// +// ** Rows 17-18: addInput/addOutput require a +// non-null, non-empty name. chooseName() only +// yields Naming.unnamed for null/empty names, +// so a port can never be unnamed. +// +// ✗ unnamed + reserved: Logic(naming: reserved) +// with null/empty name throws +// NullReservedNameException / +// EmptyReservedNameException at construction +// time. Never reaches synthesizer. +// +// Additional cross-cutting concerns: +// COL Collision between mergeables +// → uniquified suffix (_0) +// MG Merge: directly-connected signals +// share SynthLogic +// INST Submodule instance names: unique, +// don't collide with ports +// ST Structure element: structureName +// = "parent.field" → sanitized ("_") +// AR Array element: isArrayMember +// → uses logic.name (index-based) +// +// ════════════════════════════════════════════════════════ + +import 'package:rohd/rohd.dart'; +import 'package:rohd/src/synthesizers/utilities/utilities.dart'; +import 'package:test/test.dart'; + +// ── Leaf sub-modules ────────────────────────────── + +/// A leaf module whose `in0` is an "expressionless input" — +/// meaning any constant driving it must get a real wire name, not a literal. +class _ExpressionlessSub extends Module with SystemVerilog { + @override + List get expressionlessInputs => const ['in0']; + + _ExpressionlessSub(Logic a, Logic b) : super(name: 'exprsub') { + a = addInput('in0', a, width: a.width); + b = addInput('in1', b, width: b.width); + addOutput('out', width: a.width) <= a & b; + } +} + +/// A simple sub-module with preferred-name ports. +class _SimpleSub extends Module { + _SimpleSub(Logic x) : super(name: 'simplesub') { + x = addInput('x', x, width: x.width); + addOutput('y', width: x.width) <= ~x; + } +} + +/// A sub-module with an unpreferred-name port. +class _UnprefSub extends Module { + _UnprefSub(Logic a) : super(name: 'unprefsub') { + a = addInput('_uport', a, width: a.width); + addOutput('uout', width: a.width) <= ~a; + } +} + +// ── Main test module ────────────────────────────── +// One module that exercises every valid naming case in a minimal design. +// Each signal is tagged with the row number from the table above. + +class _AllNamingCases extends Module { + // Exposed for test inspection. + // Row 1 / Row 2: ports (accessed via mod.input / mod.output). + // Row 5: + late final Logic reservedInternal; + // Row 6: + late final Logic reservedInternalUnpref; + // Row 9: + late final Logic renameableInternal; + // Row 10: + late final Logic renameableInternalUnpref; + // Row 15: + late final Logic mergeablePref; + // Row 15 collision partner: + late final Logic mergeablePrefCollide; + // Row 16: + late final Logic mergeableUnpref; + // Row 19: + late final Logic unnamed; + // Row 20: + late final Logic constAllowed; + // Row 21: + late final Logic constDisallowed; + // MG: + late final Logic mergeTarget; + + // Structure/array elements (ST, AR): + late final LogicStructure structPort; + late final LogicArray arrayPort; + + _AllNamingCases() : super(name: 'allcases') { + // ── Row 1: reserved + this-port + preferred ────────────────── + final inp = addInput('inp', Logic(width: 8), width: 8); + final out = addOutput('out', width: 8); + + // ── Row 2: reserved + this-port + unpreferred ──────────────── + final uInp = addInput('_uinp', Logic(width: 8), width: 8); + + // ── Row 11: mergeable + this-port + preferred ──────────────── + // (This is the Logic.port() → connectIO path. addInput forces + // Naming.reserved regardless of the source's naming, so intrinsic + // mergeable is overridden to reserved. We test the port keeps its + // exact name.) + final mPortInp = addInput('mport', Logic(width: 8), width: 8); + + // ── Row 12: mergeable + this-port + unpreferred ────────────── + final mPortUnpref = addInput('_muprt', Logic(width: 8), width: 8); + + // ── Row 5: reserved + internal + preferred ─────────────────── + reservedInternal = Logic(name: 'resv', width: 8, naming: Naming.reserved) + ..gets(inp ^ Const(0x01, width: 8)); + + // ── Row 6: reserved + internal + unpreferred ───────────────── + reservedInternalUnpref = + Logic(name: '_resvu', width: 8, naming: Naming.reserved) + ..gets(inp ^ Const(0x02, width: 8)); + + // ── Row 9: renameable + internal + preferred ───────────────── + renameableInternal = Logic(name: 'ren', width: 8, naming: Naming.renameable) + ..gets(inp ^ Const(0x03, width: 8)); + + // ── Row 10: renameable + internal + unpreferred ────────────── + renameableInternalUnpref = + Logic(name: '_renu', width: 8, naming: Naming.renameable) + ..gets(inp ^ Const(0x04, width: 8)); + + // ── Row 15: mergeable + internal + preferred ───────────────── + mergeablePref = Logic(name: 'mname', width: 8, naming: Naming.mergeable) + ..gets(inp ^ Const(0x05, width: 8)); + + // ── COL: collision partner — same base name 'mname' ────────── + mergeablePrefCollide = + Logic(name: 'mname', width: 8, naming: Naming.mergeable) + ..gets(inp ^ Const(0x06, width: 8)); + + // ── Row 16: mergeable + internal + unpreferred ─────────────── + mergeableUnpref = Logic(name: '_hidden', width: 8, naming: Naming.mergeable) + ..gets(inp ^ Const(0x07, width: 8)); + + // ── Row 19: unnamed + internal ─────────────────────────────── + unnamed = Logic(width: 8)..gets(inp ^ Const(0x08, width: 8)); + + // ── Rows 3/13: sub-port preferred (via _SimpleSub.x / .y) ─── + // ── Row 4/14: sub-port unpreferred (via _UnprefSub._uport) ── + final sub = _SimpleSub(renameableInternal); + final subOut = sub.output('y'); + // Use a distinct expression so the submodule port doesn't merge with + // renameableInternal (which is renameable and would win). + final unpSub = _UnprefSub(inp ^ Const(0x0a, width: 8)); + + // ── MG: merge behavior — mergeTarget merges with subOut ────── + mergeTarget = Logic(name: 'mmerge', width: 8, naming: Naming.mergeable) + ..gets(subOut); + + // ── Row 20: constant with name allowed ─────────────────────── + constAllowed = + Const(0x42, width: 8).named('const_ok', naming: Naming.mergeable); + + // ── Row 21: constant with name disallowed (expressionlessInput) + constDisallowed = + Const(0x09, width: 8).named('const_wire', naming: Naming.mergeable); + // ignore: unused_local_variable + final exprSub = _ExpressionlessSub(constDisallowed, inp); + + // ── ST: structure element (structureName = "parent.field") ──── + structPort = _SimpleStruct(); + addInput('stIn', structPort, width: structPort.width); + + // ── AR: array element (isArrayMember, uses logic.name) ─────── + arrayPort = LogicArray([3], 8, name: 'arIn'); + addInputArray('arIn', arrayPort, dimensions: [3], elementWidth: 8); + + // Drive output to use all signals (prevents pruning). + out <= + mergeTarget | + mergeablePrefCollide | + mergeableUnpref | + unnamed | + constAllowed | + uInp | + mPortInp | + mPortUnpref | + reservedInternalUnpref | + renameableInternalUnpref | + unpSub.output('uout'); + } +} + +/// A minimal LogicStructure for testing structureName sanitization. +class _SimpleStruct extends LogicStructure { + final Logic field1; + final Logic field2; + + factory _SimpleStruct({String name = 'st'}) => _SimpleStruct._( + Logic(name: 'a', width: 4), + Logic(name: 'b', width: 4), + name: name, + ); + + _SimpleStruct._(this.field1, this.field2, {required super.name}) + : super([field1, field2]); + + @override + LogicStructure clone({String? name}) => + _SimpleStruct(name: name ?? this.name); +} + +// ── Helpers ─────────────────────────────────────── + +/// Collects a map from Logic → picked name for all SynthLogics. +Map _collectNames(SynthModuleDefinition def) { + final names = {}; + for (final sl in [ + ...def.inputs, + ...def.outputs, + ...def.inOuts, + ...def.internalSignals, + ]) { + try { + final n = sl.name; + for (final logic in sl.logics) { + names[logic] = n; + } + // ignore: avoid_catches_without_on_clauses + } catch (_) { + // name not picked (pruned/replaced) + } + } + return names; +} + +/// Finds a SynthLogic that contains [logic]. +SynthLogic? _findSynthLogic(SynthModuleDefinition def, Logic logic) { + for (final sl in [ + ...def.inputs, + ...def.outputs, + ...def.inOuts, + ...def.internalSignals, + ]) { + if (sl.logics.contains(logic)) { + return sl; + } + } + return null; +} + +// ── Tests ──────────────────────────────────────── + +void main() { + late _AllNamingCases mod; + late SynthModuleDefinition def; + late Map names; + + setUp(() async { + mod = _AllNamingCases(); + await mod.build(); + def = SynthModuleDefinition(mod); + names = _collectNames(def); + }); + + group('naming cases', () { + // ── Row 1: reserved + this-port + preferred ──────────────── + + test('T1: reserved preferred port keeps exact name', () { + expect(names[mod.input('inp')], 'inp'); + expect(names[mod.output('out')], 'out'); + }); + + // ── Row 2: reserved + this-port + unpreferred ────────────── + + test('T2: reserved unpreferred port keeps exact _-prefixed name', () { + expect(names[mod.input('_uinp')], '_uinp'); + }); + + // ── Rows 3/13: sub-port + preferred (reserved or mergeable) ─ + + test('T3: submodule preferred port gets a name in parent', () { + final subX = mod.subModules.whereType<_SimpleSub>().first.input('x'); + final n = names[subX]; + expect(n, isNotNull, reason: 'Submodule port must be named'); + // Treated as preferred mergeable — name should not start with _. + expect(n, isNot(startsWith('_')), + reason: 'Preferred submodule port name should not be unpreferred'); + }); + + // ── Row 4/14: sub-port + unpreferred ──────────────────────── + + test('T4: submodule unpreferred port gets an unpreferred name', () { + final subUPort = + mod.subModules.whereType<_UnprefSub>().first.input('_uport'); + final n = names[subUPort]; + expect(n, isNotNull, reason: 'Submodule port must be named'); + expect(n, startsWith('_'), + reason: 'Unpreferred submodule port should keep _-prefix'); + }); + + // ── Row 5: reserved + internal + preferred ────────────────── + + test('T5: reserved preferred internal keeps exact name', () { + expect(names[mod.reservedInternal], 'resv'); + }); + + // ── Row 6: reserved + internal + unpreferred ──────────────── + + test('T6: reserved unpreferred internal keeps exact _-prefixed name', () { + expect(names[mod.reservedInternalUnpref], '_resvu'); + }); + + // ── Row 9: renameable + internal + preferred ──────────────── + + test('T9: renameable preferred internal gets its name', () { + final n = names[mod.renameableInternal]; + expect(n, isNotNull); + expect(n, contains('ren')); + }); + + // ── Row 10: renameable + internal + unpreferred ───────────── + + test('T10: renameable unpreferred internal keeps _-prefix', () { + final n = names[mod.renameableInternalUnpref]; + expect(n, isNotNull); + expect(n, startsWith('_'), + reason: 'Unpreferred renameable should keep _-prefix'); + expect(n, contains('renu')); + }); + + // ── Row 11: mergeable + this-port + preferred ─────────────── + + test('T11: mergeable-origin port (Logic.port) keeps exact port name', () { + // addInput overrides naming to reserved; the port name is exact. + expect(names[mod.input('mport')], 'mport'); + }); + + // ── Row 12: mergeable + this-port + unpreferred ───────────── + + test('T12: mergeable-origin unpreferred port keeps exact name', () { + expect(names[mod.input('_muprt')], '_muprt'); + }); + + // ── Row 15: mergeable + internal + preferred ──────────────── + + test('T15: mergeable preferred internal gets its name', () { + final n = names[mod.mergeablePref]; + expect(n, isNotNull); + expect(n, contains('mname')); + }); + + // ── COL: name collision → uniquified suffix ───────────────── + + test('COL: collision between two mergeables gets uniquified', () { + final n1 = names[mod.mergeablePref]; + final n2 = names[mod.mergeablePrefCollide]; + expect(n1, isNot(n2), reason: 'Colliding names must be uniquified'); + expect({n1, n2}, containsAll(['mname', 'mname_0'])); + }); + + // ── Row 16: mergeable + internal + unpreferred ────────────── + + test('T16: mergeable unpreferred internal keeps _-prefix', () { + final n = names[mod.mergeableUnpref]; + expect(n, isNotNull); + expect(n, startsWith('_'), + reason: 'Unpreferred mergeable should keep _-prefix'); + }); + + // ── Row 19: unnamed + internal ────────────────────────────── + + test('T19: unnamed signal gets a generated name', () { + final n = names[mod.unnamed]; + expect(n, isNotNull, reason: 'Unnamed signal must still get a name'); + // chooseName() gives unnamed signals a name starting with '_s'. + expect(n, startsWith('_'), + reason: 'Unnamed signals get unpreferred generated names'); + }); + + // ── Row 20: constant with name allowed ────────────────────── + + test('T20: constant with name allowed uses literal value', () { + final sl = _findSynthLogic(def, mod.constAllowed); + expect(sl, isNotNull); + if (sl != null && !sl.constNameDisallowed) { + expect(sl.name, contains("8'h42"), + reason: 'Allowed constant should use value literal'); + } + }); + + // ── Row 21: constant with name disallowed ─────────────────── + + test('T21: constant with name disallowed uses wire name', () { + final sl = _findSynthLogic(def, mod.constDisallowed); + expect(sl, isNotNull); + if (sl != null) { + if (sl.constNameDisallowed) { + expect(sl.name, isNot(contains("8'h09")), + reason: 'Disallowed constant should not use value literal'); + expect(sl.name, isNotEmpty); + } + } + }); + + // ── MG: merge behavior ────────────────────────────────────── + + test('MG: merged signals share the same SynthLogic', () { + final sl = _findSynthLogic(def, mod.mergeTarget); + expect(sl, isNotNull); + if (sl != null && sl.logics.length > 1) { + expect(sl.name, isNotEmpty); + } + }); + + // ── INST: submodule instance naming ───────────────────────── + + test('INST: submodule instances get collision-free names', () { + final instNames = def.subModuleInstantiations + .where((s) => s.needsInstantiation) + .map((s) => s.name) + .toList(); + expect(instNames.toSet().length, instNames.length, + reason: 'Instance names must be unique'); + final portNames = {...mod.inputs.keys, ...mod.outputs.keys}; + for (final name in instNames) { + expect(portNames, isNot(contains(name)), + reason: 'Instance "$name" should not collide with a port'); + } + }); + + // ── ST: structure element naming ──────────────────────────── + + test('ST: structure element structureName is sanitized', () { + // structureName for field1 is "st.a" → sanitized to "st_a". + final stIn = mod.input('stIn'); + final n = names[stIn]; + expect(n, isNotNull); + // The port itself should keep its reserved name 'stIn'. + expect(n, 'stIn'); + }); + + // ── AR: array element naming ──────────────────────────────── + + test('AR: array port keeps its name', () { + // Array ports are registered via addInputArray with Naming.reserved. + final arIn = mod.input('arIn'); + final n = names[arIn]; + expect(n, isNotNull); + expect(n, 'arIn'); + }); + + // ── Impossible cases ──────────────────────────────────────── + + test('unnamed + reserved throws at construction time', () { + expect( + () => Logic(naming: Naming.reserved), + throwsA(isA()), + ); + expect( + () => Logic(name: '', naming: Naming.reserved), + throwsA(isA()), + ); + }); + + // ── Golden SV snapshot ────────────────────────────────────── + + test('golden SV output snapshot', () { + final sv = mod.generateSynth(); + + // Port declarations. + expect(sv, contains('input logic [7:0] inp')); + expect(sv, contains('output logic [7:0] out')); + expect(sv, contains('_uinp')); + expect(sv, contains('mport')); + expect(sv, contains('_muprt')); + + // Reserved internals. + expect(sv, contains('resv')); + expect(sv, contains('_resvu')); + + // Renameable internals. + expect(sv, contains('ren')); + expect(sv, contains('_renu')); + + // Constant literal (T20). + expect(sv, contains("8'h42")); + + // Submodule instantiations. + expect(sv, contains('simplesub')); + expect(sv, contains('exprsub')); + expect(sv, contains('unprefsub')); + }); + }); +} diff --git a/test/naming_consistency_test.dart b/test/naming_consistency_test.dart new file mode 100644 index 000000000..53f95e6d8 --- /dev/null +++ b/test/naming_consistency_test.dart @@ -0,0 +1,247 @@ +// Copyright (C) 2026 Intel Corporation +// SPDX-License-Identifier: BSD-3-Clause +// +// naming_consistency_test.dart +// Validates that both the SystemVerilog synthesizer and a base +// SynthModuleDefinition (used by the netlist synthesizer) produce +// consistent signal names via the shared Module.signalNamer. +// +// 2026 April 10 +// Author: Desmond Kirkpatrick + +import 'package:rohd/rohd.dart'; +import 'package:rohd/src/synthesizers/systemverilog/systemverilog_synth_module_definition.dart'; +import 'package:rohd/src/synthesizers/utilities/utilities.dart'; +import 'package:test/test.dart'; + +// ── Helper modules ────────────────────────────────────────────────── + +/// A simple module with ports, internal wires, and a sub-module. +class _Inner extends Module { + _Inner(Logic a, Logic b) : super(name: 'inner') { + a = addInput('a', a, width: a.width); + b = addInput('b', b, width: b.width); + addOutput('y', width: a.width) <= a & b; + } +} + +class _Outer extends Module { + _Outer(Logic a, Logic b) : super(name: 'outer') { + a = addInput('a', a, width: a.width); + b = addInput('b', b, width: b.width); + final inner = _Inner(a, b); + addOutput('y', width: a.width) <= inner.output('y'); + } +} + +/// A module with a constant assignment (exercises const naming). +class _ConstModule extends Module { + _ConstModule(Logic a) : super(name: 'constmod') { + a = addInput('a', a, width: 8); + final c = Const(0x42, width: 8).named('myConst', naming: Naming.mergeable); + addOutput('y', width: 8) <= a + c; + } +} + +/// A module with Naming.renameable and Naming.mergeable signals. +class _MixedNaming extends Module { + _MixedNaming(Logic a) : super(name: 'mixednaming') { + a = addInput('a', a, width: 8); + final r = Logic(name: 'renamed', width: 8, naming: Naming.renameable) + ..gets(a); + final m = Logic(name: 'merged', width: 8, naming: Naming.mergeable) + ..gets(r); + addOutput('y', width: 8) <= m; + } +} + +/// A module with a FlipFlop sub-module. +class _FlopOuter extends Module { + _FlopOuter(Logic clk, Logic d) : super(name: 'flopouter') { + clk = addInput('clk', clk); + d = addInput('d', d, width: 8); + addOutput('q', width: 8) <= flop(clk, d); + } +} + +/// Builds [SynthModuleDefinition]s from both bases and collects a +/// Logic→name mapping for all present SynthLogics. +/// +/// Returns maps from Logic to its resolved signal name. +Map _collectNames(SynthModuleDefinition def) { + final names = {}; + for (final sl in [ + ...def.inputs, + ...def.outputs, + ...def.inOuts, + ...def.internalSignals, + ]) { + // Skip SynthLogics whose name was never picked (replaced/pruned). + try { + final n = sl.name; + for (final logic in sl.logics) { + names[logic] = n; + } + // ignore: avoid_catches_without_on_clauses + } catch (_) { + // name not picked — skip + } + } + return names; +} + +void main() { + group('naming consistency', () { + test('SV and base SynthModuleDefinition agree on port names', () async { + final mod = _Outer(Logic(width: 8), Logic(width: 8)); + await mod.build(); + + // SV synthesizer path + final svDef = SystemVerilogSynthModuleDefinition(mod); + + // Base path (same as netlist synthesizer uses) + // Since signalNamer is late final, the second constructor reuses + // the same naming state — names must be consistent. + final baseDef = SynthModuleDefinition(mod); + + final svNames = _collectNames(svDef); + final baseNames = _collectNames(baseDef); + + // Every Logic present in both must have the same name. + for (final logic in svNames.keys) { + if (baseNames.containsKey(logic)) { + expect(baseNames[logic], svNames[logic], + reason: 'Name mismatch for ${logic.name} ' + '(${logic.runtimeType}, naming=${logic.naming})'); + } + } + + // Port names specifically must match. + for (final port in [...mod.inputs.values, ...mod.outputs.values]) { + expect(svNames[port], isNotNull, + reason: 'SV def should have port ${port.name}'); + expect(baseNames[port], isNotNull, + reason: 'Base def should have port ${port.name}'); + expect(svNames[port], baseNames[port], + reason: 'Port name must match for ${port.name}'); + } + }); + + test('constant naming is consistent', () async { + final mod = _ConstModule(Logic(width: 8)); + await mod.build(); + + final svDef = SystemVerilogSynthModuleDefinition(mod); + final baseDef = SynthModuleDefinition(mod); + + final svNames = _collectNames(svDef); + final baseNames = _collectNames(baseDef); + + for (final logic in svNames.keys) { + if (baseNames.containsKey(logic)) { + expect(baseNames[logic], svNames[logic], + reason: 'Name mismatch for ${logic.name}'); + } + } + }); + + test('mixed naming (renameable + mergeable) is consistent', () async { + final mod = _MixedNaming(Logic(width: 8)); + await mod.build(); + + final svDef = SystemVerilogSynthModuleDefinition(mod); + final baseDef = SynthModuleDefinition(mod); + + final svNames = _collectNames(svDef); + final baseNames = _collectNames(baseDef); + + for (final logic in svNames.keys) { + if (baseNames.containsKey(logic)) { + expect(baseNames[logic], svNames[logic], + reason: 'Name mismatch for ${logic.name}'); + } + } + }); + + test('flop module naming is consistent', () async { + final mod = _FlopOuter(Logic(), Logic(width: 8)); + await mod.build(); + + final svDef = SystemVerilogSynthModuleDefinition(mod); + final baseDef = SynthModuleDefinition(mod); + + final svNames = _collectNames(svDef); + final baseNames = _collectNames(baseDef); + + for (final logic in svNames.keys) { + if (baseNames.containsKey(logic)) { + expect(baseNames[logic], svNames[logic], + reason: 'Name mismatch for ${logic.name}'); + } + } + }); + + test('signalNamer is shared across multiple SynthModuleDefinitions', + () async { + final mod = _Outer(Logic(width: 8), Logic(width: 8)); + await mod.build(); + + // Build one def, then build another — same signalNamer instance. + final def1 = SynthModuleDefinition(mod); + final def2 = SynthModuleDefinition(mod); + + final names1 = _collectNames(def1); + final names2 = _collectNames(def2); + + for (final logic in names1.keys) { + if (names2.containsKey(logic)) { + expect(names2[logic], names1[logic], + reason: 'Shared namer should produce same name for ' + '${logic.name}'); + } + } + }); + + test('Module.signalName matches SynthLogic.name for ports', () async { + final mod = _Outer(Logic(width: 8), Logic(width: 8)); + await mod.build(); + + final def = SynthModuleDefinition(mod); + final synthNames = _collectNames(def); + + // Module.signalName uses SignalNamer.nameOf directly + for (final port in [...mod.inputs.values, ...mod.outputs.values]) { + final moduleName = mod.signalName(port); + final synthName = synthNames[port]; + expect(synthName, moduleName, + reason: 'SynthLogic.name and Module.signalName must agree ' + 'for port ${port.name}'); + } + }); + + test('submodule instance names are allocated from shared namespace', + () async { + // When building a single SynthModuleDefinition (as each synthesizer + // does), submodule instance names come from Module.allocateSignalName. + final mod = _Outer(Logic(width: 8), Logic(width: 8)); + await mod.build(); + + final def = SynthModuleDefinition(mod); + + final instNames = def.subModuleInstantiations + .where((s) => s.needsInstantiation) + .map((s) => s.name) + .toSet(); + + // The inner module instance should have a name + expect(instNames, isNotEmpty, + reason: 'Should have at least one submodule instance'); + + // All instance names should be obtainable from the module namespace + for (final name in instNames) { + expect(mod.isSignalNameAvailable(name), isFalse, + reason: 'Instance name "$name" should be claimed in namespace'); + } + }); + }); +} From 85f88cef0f472794689c9965b1be768fc5682b59 Mon Sep 17 00:00:00 2001 From: "Desmond A. Kirkpatrick" Date: Fri, 17 Apr 2026 08:36:09 -0700 Subject: [PATCH 2/8] dart 3.11 parameter_assignments pickiness --- analysis_options.yaml | 4 +++- lib/src/module.dart | 3 --- lib/src/signals/logic.dart | 1 - lib/src/signals/wire_net.dart | 1 - lib/src/utilities/simcompare.dart | 1 - lib/src/values/logic_value.dart | 3 --- 6 files changed, 3 insertions(+), 10 deletions(-) diff --git a/analysis_options.yaml b/analysis_options.yaml index 65d475023..2b2098177 100644 --- a/analysis_options.yaml +++ b/analysis_options.yaml @@ -129,7 +129,9 @@ linter: - overridden_fields - package_names - package_prefixed_library_names - - parameter_assignments + # parameter_assignments - disabled; ROHD idiomatically reassigns + # constructor parameters via addInput/addOutput. + # - parameter_assignments - prefer_adjacent_string_concatenation - prefer_asserts_in_initializer_lists - prefer_asserts_with_message diff --git a/lib/src/module.dart b/lib/src/module.dart index 09e11fdc7..188b78890 100644 --- a/lib/src/module.dart +++ b/lib/src/module.dart @@ -702,7 +702,6 @@ abstract class Module { } if (source is LogicStructure) { - // ignore: parameter_assignments source = source.packed; } @@ -739,7 +738,6 @@ abstract class Module { String name, LogicType source) { _checkForSafePortName(name); - // ignore: parameter_assignments source = _validateType(source, isOutput: false, name: name); if (source.isNet || (source is LogicStructure && source.hasNets)) { @@ -848,7 +846,6 @@ abstract class Module { throw PortTypeException(source, 'Typed inOuts must be nets.'); } - // ignore: parameter_assignments source = _validateType(source, isOutput: false, name: name); _inOutDrivers.add(source); diff --git a/lib/src/signals/logic.dart b/lib/src/signals/logic.dart index 88afba0d6..4c5f99e5e 100644 --- a/lib/src/signals/logic.dart +++ b/lib/src/signals/logic.dart @@ -377,7 +377,6 @@ class Logic { // If we are connecting a `LogicStructure` to this simple `Logic`, // then pack it first. if (other is LogicStructure) { - // ignore: parameter_assignments other = other.packed; } diff --git a/lib/src/signals/wire_net.dart b/lib/src/signals/wire_net.dart index 78e8b1beb..f93529b0f 100644 --- a/lib/src/signals/wire_net.dart +++ b/lib/src/signals/wire_net.dart @@ -189,7 +189,6 @@ class _WireNetBlasted extends _Wire implements _WireNet { other as _WireNet; if (other is! _WireNetBlasted) { - // ignore: parameter_assignments other = other.toBlasted(); } diff --git a/lib/src/utilities/simcompare.dart b/lib/src/utilities/simcompare.dart index 3a25f4074..d7850df4e 100644 --- a/lib/src/utilities/simcompare.dart +++ b/lib/src/utilities/simcompare.dart @@ -282,7 +282,6 @@ abstract class SimCompare { : 'logic'); if (adjust != null) { - // ignore: parameter_assignments signalName = adjust(signalName); } diff --git a/lib/src/values/logic_value.dart b/lib/src/values/logic_value.dart index 0cdc3c1df..81fc7304b 100644 --- a/lib/src/values/logic_value.dart +++ b/lib/src/values/logic_value.dart @@ -218,7 +218,6 @@ abstract class LogicValue implements Comparable { if (val.width == 1 && (!val.isValid || fill)) { if (!val.isValid) { - // ignore: parameter_assignments width ??= 1; } if (width == null) { @@ -243,7 +242,6 @@ abstract class LogicValue implements Comparable { if (val.length == 1 && (val == 'x' || val == 'z' || fill)) { if (val == 'x' || val == 'z') { - // ignore: parameter_assignments width ??= 1; } if (width == null) { @@ -269,7 +267,6 @@ abstract class LogicValue implements Comparable { if (val.length == 1 && (val.first == LogicValue.x || val.first == LogicValue.z || fill)) { if (!val.first.isValid) { - // ignore: parameter_assignments width ??= 1; } if (width == null) { From b7087c40467389ae38be40e2d4c599c0d532ebe7 Mon Sep 17 00:00:00 2001 From: "Desmond A. Kirkpatrick" Date: Fri, 17 Apr 2026 13:03:20 -0700 Subject: [PATCH 3/8] conflict resolved and dart format . works --- .../synthesizers/utilities/synth_logic.dart | 79 ------------------- 1 file changed, 79 deletions(-) diff --git a/lib/src/synthesizers/utilities/synth_logic.dart b/lib/src/synthesizers/utilities/synth_logic.dart index d0a5e5d5a..b5827295b 100644 --- a/lib/src/synthesizers/utilities/synth_logic.dart +++ b/lib/src/synthesizers/utilities/synth_logic.dart @@ -221,7 +221,6 @@ class SynthLogic { } /// Finds the best name from the collection of [Logic]s. -<<<<<<< central_naming /// /// Delegates to signal namer which handles constant value naming, priority /// selection, and uniquification via the module's shared namespace. @@ -231,84 +230,6 @@ class SynthLogic { constValue: _constLogic, constNameDisallowed: _constNameDisallowed, ); -======= - String _findName(Uniquifier uniquifier) { - // check for const - if (_constLogic != null) { - if (!_constNameDisallowed) { - return _constLogic!.value.toString(); - } else { - assert( - logics.length > 1, - 'If there is a constant, but the const name is not allowed, ' - 'there needs to be another option', - ); - } - } - - // check for reserved - if (_reservedLogic != null) { - return uniquifier.getUniqueName( - initialName: _reservedLogic!.name, - reserved: true, - ); - } - - // check for renameable - if (_renameableLogic != null) { - return uniquifier.getUniqueName( - initialName: _renameableLogic!.preferredSynthName, - ); - } - - // pick a preferred, available, mergeable name, if one exists - final unpreferredMergeableLogics = []; - final uniquifiableMergeableLogics = []; - for (final mergeableLogic in _mergeableLogics) { - if (Naming.isUnpreferred(mergeableLogic.preferredSynthName)) { - unpreferredMergeableLogics.add(mergeableLogic); - } else if (!uniquifier.isAvailable(mergeableLogic.preferredSynthName)) { - uniquifiableMergeableLogics.add(mergeableLogic); - } else { - return uniquifier.getUniqueName( - initialName: mergeableLogic.preferredSynthName, - ); - } - } - - // uniquify a preferred, mergeable name, if one exists - if (uniquifiableMergeableLogics.isNotEmpty) { - return uniquifier.getUniqueName( - initialName: uniquifiableMergeableLogics.first.preferredSynthName, - ); - } - - // pick an available unpreferred mergeable name, if one exists, otherwise - // uniquify an unpreferred mergeable name - if (unpreferredMergeableLogics.isNotEmpty) { - return uniquifier.getUniqueName( - initialName: unpreferredMergeableLogics - .firstWhereOrNull( - (element) => - uniquifier.isAvailable(element.preferredSynthName), - ) - ?.preferredSynthName ?? - unpreferredMergeableLogics.first.preferredSynthName, - ); - } - - // pick anything (unnamed) and uniquify as necessary (considering preferred) - // no need to prefer an available one here, since it's all unnamed - return uniquifier.getUniqueName( - initialName: _unnamedLogics - .firstWhereOrNull( - (element) => !Naming.isUnpreferred(element.preferredSynthName), - ) - ?.preferredSynthName ?? - _unnamedLogics.first.preferredSynthName, - ); - } ->>>>>>> main /// Creates an instance to represent [initialLogic] and any that merge /// into it. From 4a55214d9448376d8900a9348422f04f0985cd06 Mon Sep 17 00:00:00 2001 From: "Desmond A. Kirkpatrick" Date: Sat, 18 Apr 2026 14:18:31 -0700 Subject: [PATCH 4/8] properly assign naming spaces for instances vs signals --- lib/src/module.dart | 41 ++++++- lib/src/synthesizers/synthesizer.dart | 8 +- .../systemverilog_synthesizer.dart | 3 +- .../utilities/synth_module_definition.dart | 9 +- .../synth_sub_module_instantiation.dart | 10 +- test/instance_signal_name_collision_test.dart | 108 ++++++++++++++++++ test/naming_consistency_test.dart | 16 ++- 7 files changed, 166 insertions(+), 29 deletions(-) create mode 100644 test/instance_signal_name_collision_test.dart diff --git a/lib/src/module.dart b/lib/src/module.dart index 188b78890..8a6cd037b 100644 --- a/lib/src/module.dart +++ b/lib/src/module.dart @@ -68,25 +68,54 @@ abstract class Module { ); } + /// Separate namespace for submodule instance names. + /// + /// Instance names and signal names occupy different namespaces in + /// SystemVerilog (and most other HDLs), so they must be uniquified + /// independently to avoid false collisions. + @internal + late final Uniquifier instanceNameUniquifier = Uniquifier(); + /// Returns the collision-free signal name for [logic] within this module. String signalName(Logic logic) => signalNamer.nameOf(logic); - /// Allocates a collision-free signal name in this module's namespace. + /// Allocates a collision-free signal name in this module's signal namespace. /// - /// Used by synthesizers to name connection nets, submodule instances, - /// intermediate wires, and other artifacts that have no user-created - /// [Logic] object. The returned name is guaranteed not to collide with - /// any signal name or any previously allocated name. + /// Used by synthesizers to name connection nets, intermediate wires, and + /// other signal artifacts. The returned name is guaranteed not to collide + /// with any other signal name previously allocated in this module. /// /// When [reserved] is `true`, the exact [baseName] (after sanitization) is /// claimed without modification; an exception is thrown if it collides. String allocateSignalName(String baseName, {bool reserved = false}) => signalNamer.allocate(baseName, reserved: reserved); + /// Allocates a collision-free instance name in this module's instance + /// namespace. + /// + /// Instance names are kept separate from signal names because in + /// SystemVerilog (and other HDLs) they occupy distinct namespaces — a + /// signal and a submodule instance may legally share the same identifier + /// without collision. Mixing them into one uniquifier causes spurious + /// suffixing. + /// + /// When [reserved] is `true`, the exact [baseName] (after sanitization) is + /// claimed without modification; an exception is thrown if it collides. + String allocateInstanceName(String baseName, {bool reserved = false}) => + instanceNameUniquifier.getUniqueName( + initialName: Sanitizer.sanitizeSV(baseName), + reserved: reserved, + ); + /// Returns `true` if [name] has not yet been claimed as a signal name in - /// this module's namespace. + /// this module's signal namespace. bool isSignalNameAvailable(String name) => signalNamer.isAvailable(name); + /// Returns `true` if [name] has not yet been claimed as an instance name in + /// this module's instance namespace. + bool isInstanceNameAvailable(String name) => + instanceNameUniquifier.isAvailable(name); + /// An internal mapping of inOut names to their sources to this [Module]. late final Map _inOutSources = {}; diff --git a/lib/src/synthesizers/synthesizer.dart b/lib/src/synthesizers/synthesizer.dart index 2d7730208..ce3d2c900 100644 --- a/lib/src/synthesizers/synthesizer.dart +++ b/lib/src/synthesizers/synthesizer.dart @@ -27,13 +27,7 @@ abstract class Synthesizer { /// Synthesizes [module] into a [SynthesisResult], given the mapping provided /// by [getInstanceTypeOfModule]. - /// - /// Optionally a [lookupExistingResult] callback may be supplied which - /// allows the synthesizer to query already-generated `SynthesisResult`s - /// for child modules (useful when building parent output that needs - /// information from children). SynthesisResult synthesize( Module module, String Function(Module module) getInstanceTypeOfModule, - {SynthesisResult? Function(Module module)? lookupExistingResult, - Map? existingResults}); + {Map? existingResults}); } diff --git a/lib/src/synthesizers/systemverilog/systemverilog_synthesizer.dart b/lib/src/synthesizers/systemverilog/systemverilog_synthesizer.dart index b83acb9cc..d50daf45a 100644 --- a/lib/src/synthesizers/systemverilog/systemverilog_synthesizer.dart +++ b/lib/src/synthesizers/systemverilog/systemverilog_synthesizer.dart @@ -138,8 +138,7 @@ class SystemVerilogSynthesizer extends Synthesizer { @override SynthesisResult synthesize( Module module, String Function(Module module) getInstanceTypeOfModule, - {SynthesisResult? Function(Module module)? lookupExistingResult, - Map? existingResults}) { + {Map? existingResults}) { assert( module is! SystemVerilog || module.generatedDefinitionType != DefinitionGenerationType.none, diff --git a/lib/src/synthesizers/utilities/synth_module_definition.dart b/lib/src/synthesizers/utilities/synth_module_definition.dart index dac9075e8..97722a629 100644 --- a/lib/src/synthesizers/utilities/synth_module_definition.dart +++ b/lib/src/synthesizers/utilities/synth_module_definition.dart @@ -744,10 +744,11 @@ class SynthModuleDefinition { /// Picks names of signals and sub-modules. /// /// Signal names are read from [Module.signalName] (for user-created - /// [Logic] objects) or kept as literal constants. Submodule instance - /// names and synthesizer artifacts are allocated from the shared - /// [Module] namespace via [Module.allocateSignalName], guaranteeing no - /// collisions across synthesizers. + /// [Logic] objects) or kept as literal constants and are allocated from + /// [Module.allocateSignalName] (signal namespace). Submodule instance + /// names are allocated from [Module.allocateInstanceName] (instance + /// namespace). The two namespaces are independent, matching SystemVerilog + /// semantics where signal and instance identifiers do not collide. void _pickNames() { // Name allocation order matters — earlier claims get the unsuffixed name // when there are collisions. This matches production ROHD priority: diff --git a/lib/src/synthesizers/utilities/synth_sub_module_instantiation.dart b/lib/src/synthesizers/utilities/synth_sub_module_instantiation.dart index 4f1c3e4f2..4eaf83f57 100644 --- a/lib/src/synthesizers/utilities/synth_sub_module_instantiation.dart +++ b/lib/src/synthesizers/utilities/synth_sub_module_instantiation.dart @@ -25,13 +25,15 @@ class SynthSubModuleInstantiation { /// Selects a name for this module instance. Must be called exactly once. /// - /// Names are allocated from [parentModule]'s shared namespace via - /// [Module.allocateSignalName], ensuring no collision with signal names or - /// other submodule instances — even across multiple synthesizers. + /// Names are allocated from [parentModule]'s instance namespace via + /// [Module.allocateInstanceName], which is kept separate from the signal + /// namespace. In SystemVerilog (and other HDLs) instance names and signal + /// names occupy distinct namespaces, so they must be uniquified + /// independently to avoid spurious suffixing. void pickName(Module parentModule) { assert(_name == null, 'Should only pick a name once.'); - _name = parentModule.allocateSignalName( + _name = parentModule.allocateInstanceName( module.uniqueInstanceName, reserved: module.reserveName, ); diff --git a/test/instance_signal_name_collision_test.dart b/test/instance_signal_name_collision_test.dart new file mode 100644 index 000000000..0673e3522 --- /dev/null +++ b/test/instance_signal_name_collision_test.dart @@ -0,0 +1,108 @@ +// Copyright (C) 2026 Intel Corporation +// SPDX-License-Identifier: BSD-3-Clause +// +// instance_signal_name_collision_test.dart +// Regression test that demonstrates the bug present in the main branch where +// submodule instance names and signal names share a single Uniquifier. +// +// In SystemVerilog, signal identifiers and instance identifiers live in +// *separate* namespaces, so it is perfectly legal to have a signal called +// "inner" and a module instance also called "inner" in the same scope. +// +// When a single shared Uniquifier is used (main-branch behaviour), the second +// name to be allocated gets spuriously suffixed (e.g. "inner_0"), which +// produces incorrect generated SV. +// +// 2026 April 18 +// Author: Desmond Kirkpatrick + +import 'package:rohd/rohd.dart'; +import 'package:rohd/src/synthesizers/utilities/utilities.dart'; +import 'package:test/test.dart'; + +// ── Minimal repro modules ──────────────────────────────────────────────────── + +/// Leaf module whose default instance name is "inner". +class _Inner extends Module { + _Inner(Logic a) : super(name: 'inner') { + a = addInput('a', a, width: a.width); + addOutput('y', width: a.width) <= a; + } +} + +/// Parent module that: +/// • instantiates [_Inner] (default instance name: "inner") +/// • names an internal wire "inner" as well +/// +/// In SV the two identifiers live in different namespaces, so both should +/// be emitted as "inner" without any suffix. +class _CollidingParent extends Module { + _CollidingParent(Logic a) : super(name: 'colliding_parent') { + a = addInput('a', a, width: a.width); + + // Internal wire explicitly named "inner". + final inner = Logic(name: 'inner', width: a.width, naming: Naming.reserved) + ..gets(a); + + // Submodule whose uniqueInstanceName will also be "inner". + final sub = _Inner(inner); + + addOutput('y', width: a.width) <= sub.output('y'); + } +} + +// ── Test ───────────────────────────────────────────────────────────────────── + +void main() { + group('instance / signal name collision (main-branch bug)', () { + late _CollidingParent mod; + late SynthModuleDefinition def; + + setUpAll(() async { + mod = _CollidingParent(Logic(width: 8)); + await mod.build(); + def = SynthModuleDefinition(mod); + }); + + test('internal signal named "inner" retains its exact name', () { + // Find the SynthLogic for the reserved "inner" wire. + final sl = def.internalSignals.cast().firstWhere( + (s) => s!.logics.any((l) => l.name == 'inner'), + orElse: () => null, + ); + expect(sl, isNotNull, reason: 'Expected to find SynthLogic for "inner"'); + expect(sl!.name, 'inner', + reason: 'Signal "inner" must not be suffixed to "inner_0"'); + }); + + test('submodule instance named "inner" retains its exact name', () { + final inst = def.subModuleInstantiations + .where((s) => s.needsInstantiation) + .cast() + .firstWhere( + (s) => s!.module.name == 'inner', + orElse: () => null, + ); + expect(inst, isNotNull, reason: 'Expected submodule instance for inner'); + expect(inst!.name, 'inner', + reason: 'Instance "inner" must not be suffixed to "inner_0"'); + }); + + test('signal and instance may share the name "inner" without collision', () { + // Both should be "inner", not one of them "inner_0". + final sl = def.internalSignals.cast().firstWhere( + (s) => s!.logics.any((l) => l.name == 'inner'), + orElse: () => null, + ); + final inst = def.subModuleInstantiations + .where((s) => s.needsInstantiation) + .cast() + .firstWhere( + (s) => s!.module.name == 'inner', + orElse: () => null, + ); + expect(sl?.name, 'inner'); + expect(inst?.name, 'inner'); + }); + }); +} diff --git a/test/naming_consistency_test.dart b/test/naming_consistency_test.dart index 53f95e6d8..b569bd4d6 100644 --- a/test/naming_consistency_test.dart +++ b/test/naming_consistency_test.dart @@ -219,10 +219,12 @@ void main() { } }); - test('submodule instance names are allocated from shared namespace', + test('submodule instance names are allocated from the instance namespace', () async { - // When building a single SynthModuleDefinition (as each synthesizer - // does), submodule instance names come from Module.allocateSignalName. + // Instance names come from Module.allocateInstanceName, which is + // separate from the signal namespace (Module.allocateSignalName). + // A signal and a submodule instance may therefore share the same + // identifier without collision — matching SystemVerilog semantics. final mod = _Outer(Logic(width: 8), Logic(width: 8)); await mod.build(); @@ -237,10 +239,12 @@ void main() { expect(instNames, isNotEmpty, reason: 'Should have at least one submodule instance'); - // All instance names should be obtainable from the module namespace + // Instance names are claimed in the *instance* namespace, NOT the + // signal namespace. for (final name in instNames) { - expect(mod.isSignalNameAvailable(name), isFalse, - reason: 'Instance name "$name" should be claimed in namespace'); + expect(mod.isInstanceNameAvailable(name), isFalse, + reason: 'Instance name "$name" should be claimed in instance ' + 'namespace'); } }); }); From ed7be3696082ded32f01ccabaeb5e63b8efb1a02 Mon Sep 17 00:00:00 2001 From: "Desmond A. Kirkpatrick" Date: Sat, 18 Apr 2026 15:32:50 -0700 Subject: [PATCH 5/8] format issue --- test/instance_signal_name_collision_test.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/instance_signal_name_collision_test.dart b/test/instance_signal_name_collision_test.dart index 0673e3522..2cdfb2e3e 100644 --- a/test/instance_signal_name_collision_test.dart +++ b/test/instance_signal_name_collision_test.dart @@ -88,7 +88,8 @@ void main() { reason: 'Instance "inner" must not be suffixed to "inner_0"'); }); - test('signal and instance may share the name "inner" without collision', () { + test('signal and instance may share the name "inner" without collision', + () { // Both should be "inner", not one of them "inner_0". final sl = def.internalSignals.cast().firstWhere( (s) => s!.logics.any((l) => l.name == 'inner'), From ab09aed656059ee777755293f176bb354f417a84 Mon Sep 17 00:00:00 2001 From: "Desmond A. Kirkpatrick" Date: Sun, 19 Apr 2026 05:27:52 -0700 Subject: [PATCH 6/8] Controllable enforcement of signal vs instance name uniqueness. --- lib/src/module.dart | 37 +++++++++++++-- lib/src/synthesizers/synth_builder.dart | 3 -- lib/src/synthesizers/synthesizer.dart | 10 ---- lib/src/utilities/config.dart | 9 ++++ lib/src/utilities/signal_namer.dart | 47 +++++++++++++++---- test/instance_signal_name_collision_test.dart | 9 ++++ test/name_test.dart | 5 ++ 7 files changed, 96 insertions(+), 24 deletions(-) diff --git a/lib/src/module.dart b/lib/src/module.dart index 8a6cd037b..475f48c68 100644 --- a/lib/src/module.dart +++ b/lib/src/module.dart @@ -65,6 +65,9 @@ abstract class Module { inputs: _inputs, outputs: _outputs, inOuts: _inOuts, + isAvailableInOtherNamespace: (name) => + !Config.ensureUniqueSignalAndInstanceNames || + instanceNameUniquifier.isAvailable(name), ); } @@ -101,11 +104,39 @@ abstract class Module { /// /// When [reserved] is `true`, the exact [baseName] (after sanitization) is /// claimed without modification; an exception is thrown if it collides. - String allocateInstanceName(String baseName, {bool reserved = false}) => - instanceNameUniquifier.getUniqueName( - initialName: Sanitizer.sanitizeSV(baseName), + String allocateInstanceName(String baseName, {bool reserved = false}) { + final sanitizedBaseName = Sanitizer.sanitizeSV(baseName); + + if (!Config.ensureUniqueSignalAndInstanceNames) { + return instanceNameUniquifier.getUniqueName( + initialName: sanitizedBaseName, reserved: reserved, ); + } + + if (reserved) { + if (!instanceNameUniquifier.isAvailable(sanitizedBaseName, + reserved: true) || + !signalNamer.isAvailable(sanitizedBaseName)) { + throw UnavailableReservedNameException(sanitizedBaseName); + } + + return instanceNameUniquifier.getUniqueName( + initialName: sanitizedBaseName, + reserved: true, + ); + } + + var candidate = sanitizedBaseName; + var suffix = 0; + while (!instanceNameUniquifier.isAvailable(candidate) || + !signalNamer.isAvailable(candidate)) { + candidate = '${sanitizedBaseName}_$suffix'; + suffix++; + } + + return instanceNameUniquifier.getUniqueName(initialName: candidate); + } /// Returns `true` if [name] has not yet been claimed as a signal name in /// this module's signal namespace. diff --git a/lib/src/synthesizers/synth_builder.dart b/lib/src/synthesizers/synth_builder.dart index 3b3a6011c..f9d0a0d08 100644 --- a/lib/src/synthesizers/synth_builder.dart +++ b/lib/src/synthesizers/synth_builder.dart @@ -56,9 +56,6 @@ class SynthBuilder { } } - // Allow the synthesizer to prepare with knowledge of top module(s) - synthesizer.prepare(this.tops); - final modulesToParse = [...tops]; for (var i = 0; i < modulesToParse.length; i++) { final moduleI = modulesToParse[i]; diff --git a/lib/src/synthesizers/synthesizer.dart b/lib/src/synthesizers/synthesizer.dart index ce3d2c900..7b350e8b4 100644 --- a/lib/src/synthesizers/synthesizer.dart +++ b/lib/src/synthesizers/synthesizer.dart @@ -11,16 +11,6 @@ import 'package:rohd/rohd.dart'; /// An object capable of converting a module into some new output format abstract class Synthesizer { - /// Called by [SynthBuilder] before synthesis begins, with the top-level - /// module(s) being synthesized. - /// - /// Override this method to perform any initialization that requires - /// knowledge of the top module, such as resolving port names to [Logic] - /// objects, or computing global signal sets. - /// - /// The default implementation does nothing. - void prepare(List tops) {} - /// Determines whether [module] needs a separate definition or can just be /// described in-line. bool generatesDefinition(Module module); diff --git a/lib/src/utilities/config.dart b/lib/src/utilities/config.dart index 4aa2ca8c6..89eda836a 100644 --- a/lib/src/utilities/config.dart +++ b/lib/src/utilities/config.dart @@ -11,4 +11,13 @@ class Config { /// The version of the ROHD framework. static const String version = '0.6.8'; + + /// Controls whether synthesized signal names and instance names must be + /// unique across both namespaces. + /// + /// When `true`, central naming cross-checks both namespaces during + /// allocation to avoid collisions in generated output. + /// + /// When `false`, signal and instance names are uniquified independently. + static bool ensureUniqueSignalAndInstanceNames = true; } diff --git a/lib/src/utilities/signal_namer.dart b/lib/src/utilities/signal_namer.dart index b7d9dc090..7f98fdff3 100644 --- a/lib/src/utilities/signal_namer.dart +++ b/lib/src/utilities/signal_namer.dart @@ -23,6 +23,7 @@ import 'package:rohd/src/utilities/uniquifier.dart'; @internal class SignalNamer { final Uniquifier _uniquifier; + final bool Function(String name) _isAvailableInOtherNamespace; /// Sparse cache: only entries where the canonical name has been resolved. /// Ports whose sanitized name == logic.name may be absent (fast-path @@ -36,8 +37,10 @@ class SignalNamer { required Uniquifier uniquifier, required Map portRenames, required Set portLogics, + required bool Function(String name) isAvailableInOtherNamespace, }) : _uniquifier = uniquifier, - _portLogics = portLogics { + _portLogics = portLogics, + _isAvailableInOtherNamespace = isAvailableInOtherNamespace { _names.addAll(portRenames); } @@ -49,6 +52,7 @@ class SignalNamer { required Map inputs, required Map outputs, required Map inOuts, + bool Function(String name)? isAvailableInOtherNamespace, }) { final portRenames = {}; final portLogics = {}; @@ -85,9 +89,36 @@ class SignalNamer { uniquifier: uniquifier, portRenames: portRenames, portLogics: portLogics, + isAvailableInOtherNamespace: + isAvailableInOtherNamespace ?? ((_) => true), ); } + bool _isAvailable(String name, {bool reserved = false}) => + _uniquifier.isAvailable(name, reserved: reserved) && + _isAvailableInOtherNamespace(name); + + String _allocateUniqueName(String baseName, {bool reserved = false}) { + if (reserved) { + if (!_isAvailable(baseName, reserved: true)) { + throw UnavailableReservedNameException(baseName); + } + + _uniquifier.getUniqueName(initialName: baseName, reserved: true); + return baseName; + } + + var candidate = baseName; + var suffix = 0; + while (!_isAvailable(candidate)) { + candidate = '${baseName}_$suffix'; + suffix++; + } + + _uniquifier.getUniqueName(initialName: candidate); + return candidate; + } + /// Returns the canonical name for [logic]. /// /// The first call for a given [logic] allocates a collision-free name @@ -117,8 +148,8 @@ class SignalNamer { baseName = Sanitizer.sanitizeSV(logic.structureName); } - final name = _uniquifier.getUniqueName( - initialName: baseName, + final name = _allocateUniqueName( + baseName, reserved: isReservedInternal, ); _names[logic] = name; @@ -214,7 +245,7 @@ class SignalNamer { // Preferred-available mergeable. for (final logic in preferredMergeable) { - if (_uniquifier.isAvailable(baseName(logic))) { + if (_isAvailable(baseName(logic))) { return _nameAndCacheAll(logic, candidates); } } @@ -227,7 +258,7 @@ class SignalNamer { // Unpreferred mergeable — prefer available. if (unpreferredMergeable.isNotEmpty) { final best = unpreferredMergeable - .firstWhereOrNull((e) => _uniquifier.isAvailable(baseName(e))) ?? + .firstWhereOrNull((e) => _isAvailable(baseName(e))) ?? unpreferredMergeable.first; return _nameAndCacheAll(best, candidates); } @@ -261,11 +292,11 @@ class SignalNamer { /// When [reserved] is `true`, the exact [baseName] (after sanitization) /// is claimed without modification; an exception is thrown if it collides. String allocate(String baseName, {bool reserved = false}) => - _uniquifier.getUniqueName( - initialName: Sanitizer.sanitizeSV(baseName), + _allocateUniqueName( + Sanitizer.sanitizeSV(baseName), reserved: reserved, ); /// Returns `true` if [name] has not yet been claimed in this namespace. - bool isAvailable(String name) => _uniquifier.isAvailable(name); + bool isAvailable(String name) => _isAvailable(name); } diff --git a/test/instance_signal_name_collision_test.dart b/test/instance_signal_name_collision_test.dart index 2cdfb2e3e..6ee10de92 100644 --- a/test/instance_signal_name_collision_test.dart +++ b/test/instance_signal_name_collision_test.dart @@ -18,6 +18,7 @@ import 'package:rohd/rohd.dart'; import 'package:rohd/src/synthesizers/utilities/utilities.dart'; +import 'package:rohd/src/utilities/config.dart'; import 'package:test/test.dart'; // ── Minimal repro modules ──────────────────────────────────────────────────── @@ -57,13 +58,21 @@ void main() { group('instance / signal name collision (main-branch bug)', () { late _CollidingParent mod; late SynthModuleDefinition def; + late bool previousSetting; setUpAll(() async { + previousSetting = Config.ensureUniqueSignalAndInstanceNames; + Config.ensureUniqueSignalAndInstanceNames = false; + mod = _CollidingParent(Logic(width: 8)); await mod.build(); def = SynthModuleDefinition(mod); }); + tearDownAll(() { + Config.ensureUniqueSignalAndInstanceNames = previousSetting; + }); + test('internal signal named "inner" retains its exact name', () { // Find the SynthLogic for the reserved "inner" wire. final sl = def.internalSignals.cast().firstWhere( diff --git a/test/name_test.dart b/test/name_test.dart index 2742c0ec8..c863c04f5 100644 --- a/test/name_test.dart +++ b/test/name_test.dart @@ -136,6 +136,11 @@ void main() { final nameTypes = [nameType1, nameType2]; // skip ones that actually *should* cause a failure + // + // Note: SystemVerilog allows using the same identifier for a signal + // and an instance because they are different namespaces. However, + // Icarus Verilog rejects that pattern, so ROHD treats those as + // conflicts for simulator compatibility. final shouldConflict = [ { NameType.internalModuleDefinition, From 520d2809fdfa844260aa7614bdf55d8655330b09 Mon Sep 17 00:00:00 2001 From: "Desmond A. Kirkpatrick" Date: Sun, 19 Apr 2026 06:58:00 -0700 Subject: [PATCH 7/8] Refactored to Namer class. No external API changes for ROHD --- lib/src/module.dart | 93 +---- lib/src/synthesizers/synthesizer.dart | 3 +- .../systemverilog_synthesizer.dart | 3 +- .../synthesizers/utilities/synth_logic.dart | 37 +- .../utilities/synth_module_definition.dart | 9 +- .../synth_sub_module_instantiation.dart | 6 +- lib/src/utilities/config.dart | 9 - lib/src/utilities/namer.dart | 349 ++++++++++++++++++ lib/src/utilities/signal_namer.dart | 18 +- test/instance_signal_name_collision_test.dart | 8 +- test/naming_consistency_test.dart | 23 +- test/naming_namespace_test.dart | 180 +++++++++ 12 files changed, 596 insertions(+), 142 deletions(-) create mode 100644 lib/src/utilities/namer.dart create mode 100644 test/naming_namespace_test.dart diff --git a/lib/src/module.dart b/lib/src/module.dart index 475f48c68..02e02ad63 100644 --- a/lib/src/module.dart +++ b/lib/src/module.dart @@ -15,8 +15,8 @@ import 'package:rohd/rohd.dart'; import 'package:rohd/src/collections/traverseable_collection.dart'; import 'package:rohd/src/diagnostics/inspector_service.dart'; import 'package:rohd/src/utilities/config.dart'; +import 'package:rohd/src/utilities/namer.dart'; import 'package:rohd/src/utilities/sanitizer.dart'; -import 'package:rohd/src/utilities/signal_namer.dart'; import 'package:rohd/src/utilities/timestamper.dart'; import 'package:rohd/src/utilities/uniquifier.dart'; @@ -52,101 +52,22 @@ abstract class Module { /// An internal mapping of input names to their sources to this [Module]. late final Map _inputSources = {}; - // ─── Canonical naming (SignalNamer) ───────────────────────────── + // ─── Central naming (Namer) ───────────────────────────────────── - /// Lazily-constructed namer that owns the [Uniquifier] and the - /// sparse Logic→String cache. Initialized on first access. + /// Central namer that owns both the signal and instance namespaces. + /// Initialized lazily on first access (after build). @internal - late final SignalNamer signalNamer = _createSignalNamer(); + late final Namer namer = _createNamer(); - SignalNamer _createSignalNamer() { + Namer _createNamer() { assert(hasBuilt, 'Module must be built before canonical names are bound.'); - return SignalNamer.forModule( + return Namer.forModule( inputs: _inputs, outputs: _outputs, inOuts: _inOuts, - isAvailableInOtherNamespace: (name) => - !Config.ensureUniqueSignalAndInstanceNames || - instanceNameUniquifier.isAvailable(name), ); } - /// Separate namespace for submodule instance names. - /// - /// Instance names and signal names occupy different namespaces in - /// SystemVerilog (and most other HDLs), so they must be uniquified - /// independently to avoid false collisions. - @internal - late final Uniquifier instanceNameUniquifier = Uniquifier(); - - /// Returns the collision-free signal name for [logic] within this module. - String signalName(Logic logic) => signalNamer.nameOf(logic); - - /// Allocates a collision-free signal name in this module's signal namespace. - /// - /// Used by synthesizers to name connection nets, intermediate wires, and - /// other signal artifacts. The returned name is guaranteed not to collide - /// with any other signal name previously allocated in this module. - /// - /// When [reserved] is `true`, the exact [baseName] (after sanitization) is - /// claimed without modification; an exception is thrown if it collides. - String allocateSignalName(String baseName, {bool reserved = false}) => - signalNamer.allocate(baseName, reserved: reserved); - - /// Allocates a collision-free instance name in this module's instance - /// namespace. - /// - /// Instance names are kept separate from signal names because in - /// SystemVerilog (and other HDLs) they occupy distinct namespaces — a - /// signal and a submodule instance may legally share the same identifier - /// without collision. Mixing them into one uniquifier causes spurious - /// suffixing. - /// - /// When [reserved] is `true`, the exact [baseName] (after sanitization) is - /// claimed without modification; an exception is thrown if it collides. - String allocateInstanceName(String baseName, {bool reserved = false}) { - final sanitizedBaseName = Sanitizer.sanitizeSV(baseName); - - if (!Config.ensureUniqueSignalAndInstanceNames) { - return instanceNameUniquifier.getUniqueName( - initialName: sanitizedBaseName, - reserved: reserved, - ); - } - - if (reserved) { - if (!instanceNameUniquifier.isAvailable(sanitizedBaseName, - reserved: true) || - !signalNamer.isAvailable(sanitizedBaseName)) { - throw UnavailableReservedNameException(sanitizedBaseName); - } - - return instanceNameUniquifier.getUniqueName( - initialName: sanitizedBaseName, - reserved: true, - ); - } - - var candidate = sanitizedBaseName; - var suffix = 0; - while (!instanceNameUniquifier.isAvailable(candidate) || - !signalNamer.isAvailable(candidate)) { - candidate = '${sanitizedBaseName}_$suffix'; - suffix++; - } - - return instanceNameUniquifier.getUniqueName(initialName: candidate); - } - - /// Returns `true` if [name] has not yet been claimed as a signal name in - /// this module's signal namespace. - bool isSignalNameAvailable(String name) => signalNamer.isAvailable(name); - - /// Returns `true` if [name] has not yet been claimed as an instance name in - /// this module's instance namespace. - bool isInstanceNameAvailable(String name) => - instanceNameUniquifier.isAvailable(name); - /// An internal mapping of inOut names to their sources to this [Module]. late final Map _inOutSources = {}; diff --git a/lib/src/synthesizers/synthesizer.dart b/lib/src/synthesizers/synthesizer.dart index 7b350e8b4..687bbab03 100644 --- a/lib/src/synthesizers/synthesizer.dart +++ b/lib/src/synthesizers/synthesizer.dart @@ -18,6 +18,5 @@ abstract class Synthesizer { /// Synthesizes [module] into a [SynthesisResult], given the mapping provided /// by [getInstanceTypeOfModule]. SynthesisResult synthesize( - Module module, String Function(Module module) getInstanceTypeOfModule, - {Map? existingResults}); + Module module, String Function(Module module) getInstanceTypeOfModule); } diff --git a/lib/src/synthesizers/systemverilog/systemverilog_synthesizer.dart b/lib/src/synthesizers/systemverilog/systemverilog_synthesizer.dart index d50daf45a..062647ac3 100644 --- a/lib/src/synthesizers/systemverilog/systemverilog_synthesizer.dart +++ b/lib/src/synthesizers/systemverilog/systemverilog_synthesizer.dart @@ -137,8 +137,7 @@ class SystemVerilogSynthesizer extends Synthesizer { @override SynthesisResult synthesize( - Module module, String Function(Module module) getInstanceTypeOfModule, - {Map? existingResults}) { + Module module, String Function(Module module) getInstanceTypeOfModule) { assert( module is! SystemVerilog || module.generatedDefinitionType != DefinitionGenerationType.none, diff --git a/lib/src/synthesizers/utilities/synth_logic.dart b/lib/src/synthesizers/utilities/synth_logic.dart index b5827295b..ad88bd6cc 100644 --- a/lib/src/synthesizers/utilities/synth_logic.dart +++ b/lib/src/synthesizers/utilities/synth_logic.dart @@ -11,11 +11,25 @@ import 'package:collection/collection.dart'; import 'package:meta/meta.dart'; import 'package:rohd/rohd.dart'; import 'package:rohd/src/synthesizers/utilities/utilities.dart'; +import 'package:rohd/src/utilities/namer.dart'; import 'package:rohd/src/utilities/sanitizer.dart'; /// Represents a logic signal in the generated code within a module. @internal class SynthLogic { + /// Controls whether two constants with the same value driving separate + /// module inputs are merged into a single signal declaration. + /// + /// When `true` (the default), identical constants are collapsed to one + /// declaration — desirable for simulation-oriented output such as + /// SystemVerilog, where a single `assign wire = VALUE;` feeds all + /// downstream consumers. + /// + /// When `false`, each constant input keeps its own declaration. This is + /// useful for netlist/visualization outputs where seeing every individual + /// constant connection is more informative than an optimized fan-out net. + static bool mergeConstantInputs = true; + /// All [Logic]s represented, regardless of type. List get logics => UnmodifiableListView([ if (_reservedLogic != null) _reservedLogic!, @@ -225,7 +239,7 @@ class SynthLogic { /// Delegates to signal namer which handles constant value naming, priority /// selection, and uniquification via the module's shared namespace. String _findName() => - parentSynthModuleDefinition.module.signalNamer.nameOfBest( + parentSynthModuleDefinition.module.namer.signalNameOfBest( logics, constValue: _constLogic, constNameDisallowed: _constNameDisallowed, @@ -274,7 +288,12 @@ class SynthLogic { } /// Indicates whether two constants can be merged. + /// + /// Merging is only performed when [SynthLogic.mergeConstantInputs] is + /// `true`. Set it to `false` to keep each constant input as its own + /// declaration (e.g. for netlist/visualization output). static bool _constantsMergeable(SynthLogic a, SynthLogic b) => + SynthLogic.mergeConstantInputs && a.isConstant && b.isConstant && a._constLogic!.value == b._constLogic!.value && @@ -336,7 +355,7 @@ class SynthLogic { @override String toString() => '${_name == null ? 'null' : '"$name"'}, ' - 'logics contained: ${logics.map((e) => e.preferredSynthName).toList()}'; + 'logics contained: ${logics.map(Namer.baseName).toList()}'; /// Provides a definition for a range in SV from a width. static String _widthToRangeDef(int width, {bool forceRange = false}) { @@ -483,17 +502,3 @@ class SynthLogicArrayElement extends SynthLogic { ' parentArray=($parentArray), element ${logic.arrayIndex}, logic: $logic' ' logics contained: ${logics.map((e) => e.name).toList()}'; } - -extension on Logic { - /// Returns the preferred name for this [Logic] while generating in the synth - /// stack. - String get preferredSynthName => naming == Naming.reserved - // if reserved, keep the exact name - ? name - : isArrayMember - // arrays nicely name their elements already - ? name - // sanitize to remove any `.` in struct names - // the base `name` will be returned if not a structure. - : Sanitizer.sanitizeSV(structureName); -} diff --git a/lib/src/synthesizers/utilities/synth_module_definition.dart b/lib/src/synthesizers/utilities/synth_module_definition.dart index 97722a629..73b4e95c3 100644 --- a/lib/src/synthesizers/utilities/synth_module_definition.dart +++ b/lib/src/synthesizers/utilities/synth_module_definition.dart @@ -743,12 +743,11 @@ class SynthModuleDefinition { /// Picks names of signals and sub-modules. /// - /// Signal names are read from [Module.signalName] (for user-created + /// Signal names are read from `Namer.signalNameOf `(for user-created /// [Logic] objects) or kept as literal constants and are allocated from - /// [Module.allocateSignalName] (signal namespace). Submodule instance - /// names are allocated from [Module.allocateInstanceName] (instance - /// namespace). The two namespaces are independent, matching SystemVerilog - /// semantics where signal and instance identifiers do not collide. + /// `Namer.allocateSignalName` (signal namespace). Submodule instance + /// names are allocated from `Namer.allocateInstanceName` (instance + /// namespace). Both namespaces are managed by the module's `Namer`. void _pickNames() { // Name allocation order matters — earlier claims get the unsuffixed name // when there are collisions. This matches production ROHD priority: diff --git a/lib/src/synthesizers/utilities/synth_sub_module_instantiation.dart b/lib/src/synthesizers/utilities/synth_sub_module_instantiation.dart index 4eaf83f57..0cee7f1c9 100644 --- a/lib/src/synthesizers/utilities/synth_sub_module_instantiation.dart +++ b/lib/src/synthesizers/utilities/synth_sub_module_instantiation.dart @@ -25,15 +25,15 @@ class SynthSubModuleInstantiation { /// Selects a name for this module instance. Must be called exactly once. /// - /// Names are allocated from [parentModule]'s instance namespace via - /// [Module.allocateInstanceName], which is kept separate from the signal + /// Names are allocated from [parentModule]'s `Namer`'s instance namespace + /// via `Namer.allocateInstanceName`], which is kept separate from the signal /// namespace. In SystemVerilog (and other HDLs) instance names and signal /// names occupy distinct namespaces, so they must be uniquified /// independently to avoid spurious suffixing. void pickName(Module parentModule) { assert(_name == null, 'Should only pick a name once.'); - _name = parentModule.allocateInstanceName( + _name = parentModule.namer.allocateInstanceName( module.uniqueInstanceName, reserved: module.reserveName, ); diff --git a/lib/src/utilities/config.dart b/lib/src/utilities/config.dart index 89eda836a..4aa2ca8c6 100644 --- a/lib/src/utilities/config.dart +++ b/lib/src/utilities/config.dart @@ -11,13 +11,4 @@ class Config { /// The version of the ROHD framework. static const String version = '0.6.8'; - - /// Controls whether synthesized signal names and instance names must be - /// unique across both namespaces. - /// - /// When `true`, central naming cross-checks both namespaces during - /// allocation to avoid collisions in generated output. - /// - /// When `false`, signal and instance names are uniquified independently. - static bool ensureUniqueSignalAndInstanceNames = true; } diff --git a/lib/src/utilities/namer.dart b/lib/src/utilities/namer.dart new file mode 100644 index 000000000..f03f708fa --- /dev/null +++ b/lib/src/utilities/namer.dart @@ -0,0 +1,349 @@ +// Copyright (C) 2026 Intel Corporation +// SPDX-License-Identifier: BSD-3-Clause +// +// namer.dart +// Central collision-free naming for signals and instances within a module. +// +// 2026 April 10 +// Author: Desmond Kirkpatrick + +import 'package:collection/collection.dart'; +import 'package:meta/meta.dart'; +import 'package:rohd/rohd.dart'; +import 'package:rohd/src/utilities/sanitizer.dart'; +import 'package:rohd/src/utilities/uniquifier.dart'; + +/// Central namer that manages collision-free names for both signals and +/// submodule instances within a single module scope. +/// +/// Signal names and instance names occupy separate namespaces (matching +/// SystemVerilog semantics), but can optionally be cross-checked via +/// [uniquifySignalAndInstanceNames] for simulator compatibility. +/// +/// Port names are reserved at construction time. Internal signal names +/// are assigned lazily on the first [signalNameOf] call. Instance names +/// are allocated explicitly via [allocateInstanceName]. +@internal +class Namer { + /// Controls whether signal names and instance names must be unique + /// across both namespaces. + /// + /// When `true` (the default), allocations cross-check both namespaces + /// so that no identifier appears as both a signal and an instance name. + /// This is necessary for simulators like Icarus Verilog that reject + /// duplicate identifiers even across namespace boundaries. + /// + /// When `false`, signal and instance names are uniquified independently, + /// matching strict SystemVerilog semantics where instance and signal + /// identifiers occupy separate namespaces. + static bool uniquifySignalAndInstanceNames = true; + + // ─── Signal namespace ─────────────────────────────────────────── + + final Uniquifier _signalUniquifier; + + /// Sparse cache: only entries where the canonical name has been resolved. + /// Ports whose sanitized name == logic.name may be absent (fast-path + /// through [_portLogics] check). + final Map _signalNames = {}; + + /// The set of port [Logic] objects, for O(1) port membership tests. + final Set _portLogics; + + // ─── Instance namespace ───────────────────────────────────────── + + final Uniquifier _instanceUniquifier = Uniquifier(); + + // ─── Construction ─────────────────────────────────────────────── + + Namer._({ + required Uniquifier signalUniquifier, + required Map portRenames, + required Set portLogics, + }) : _signalUniquifier = signalUniquifier, + _portLogics = portLogics { + _signalNames.addAll(portRenames); + } + + /// Creates a [Namer] for the given module ports. + /// + /// Sanitized port names are reserved in the signal namespace. Ports + /// whose sanitized name differs from [Logic.name] are cached immediately. + factory Namer.forModule({ + required Map inputs, + required Map outputs, + required Map inOuts, + }) { + final portRenames = {}; + final portLogics = {}; + final portNames = []; + + void collectPort(String rawName, Logic logic) { + final sanitized = Sanitizer.sanitizeSV(rawName); + portNames.add(sanitized); + portLogics.add(logic); + if (sanitized != logic.name) { + portRenames[logic] = sanitized; + } + } + + for (final entry in inputs.entries) { + collectPort(entry.key, entry.value); + } + for (final entry in outputs.entries) { + collectPort(entry.key, entry.value); + } + for (final entry in inOuts.entries) { + collectPort(entry.key, entry.value); + } + + final uniquifier = Uniquifier(); + for (final name in portNames) { + uniquifier.getUniqueName(initialName: name, reserved: true); + } + + return Namer._( + signalUniquifier: uniquifier, + portRenames: portRenames, + portLogics: portLogics, + ); + } + + // ─── Signal availability / allocation ─────────────────────────── + + bool _isSignalAvailable(String name, {bool reserved = false}) => + _signalUniquifier.isAvailable(name, reserved: reserved) && + (!uniquifySignalAndInstanceNames || + _instanceUniquifier.isAvailable(name)); + + String _allocateUniqueSignalName(String baseName, {bool reserved = false}) { + if (reserved) { + if (!_isSignalAvailable(baseName, reserved: true)) { + throw UnavailableReservedNameException(baseName); + } + + _signalUniquifier.getUniqueName(initialName: baseName, reserved: true); + return baseName; + } + + var candidate = baseName; + var suffix = 0; + while (!_isSignalAvailable(candidate)) { + candidate = '${baseName}_$suffix'; + suffix++; + } + + _signalUniquifier.getUniqueName(initialName: candidate); + return candidate; + } + + /// Returns `true` if [name] has not yet been claimed in the signal + /// namespace. + bool isSignalNameAvailable(String name) => _isSignalAvailable(name); + + /// Allocates a collision-free name in the signal namespace. + /// + /// When [reserved] is `true`, the exact [baseName] (after sanitization) + /// is claimed without modification; an exception is thrown if it collides. + String allocateSignalName(String baseName, {bool reserved = false}) => + _allocateUniqueSignalName( + Sanitizer.sanitizeSV(baseName), + reserved: reserved, + ); + + // ─── Instance availability / allocation ───────────────────────── + + bool _isInstanceAvailable(String name, {bool reserved = false}) => + _instanceUniquifier.isAvailable(name, reserved: reserved) && + (!uniquifySignalAndInstanceNames || _signalUniquifier.isAvailable(name)); + + /// Returns `true` if [name] has not yet been claimed in the instance + /// namespace. + bool isInstanceNameAvailable(String name) => + _instanceUniquifier.isAvailable(name); + + /// Allocates a collision-free instance name. + /// + /// When [reserved] is `true`, the exact [baseName] (after sanitization) + /// is claimed without modification; an exception is thrown if it collides. + String allocateInstanceName(String baseName, {bool reserved = false}) { + final sanitizedBaseName = Sanitizer.sanitizeSV(baseName); + + if (!uniquifySignalAndInstanceNames) { + return _instanceUniquifier.getUniqueName( + initialName: sanitizedBaseName, + reserved: reserved, + ); + } + + if (reserved) { + if (!_isInstanceAvailable(sanitizedBaseName, reserved: true)) { + throw UnavailableReservedNameException(sanitizedBaseName); + } + + return _instanceUniquifier.getUniqueName( + initialName: sanitizedBaseName, + reserved: true, + ); + } + + var candidate = sanitizedBaseName; + var suffix = 0; + while (!_isInstanceAvailable(candidate)) { + candidate = '${sanitizedBaseName}_$suffix'; + suffix++; + } + + return _instanceUniquifier.getUniqueName(initialName: candidate); + } + + // ─── Signal naming (Logic → String) ───────────────────────────── + + /// Returns the canonical name for [logic]. + /// + /// The first call for a given [logic] allocates a collision-free name + /// via the underlying [Uniquifier]. Subsequent calls return the cached + /// result in O(1). + String signalNameOf(Logic logic) { + final cached = _signalNames[logic]; + if (cached != null) { + return cached; + } + + if (_portLogics.contains(logic)) { + return logic.name; + } + + String base; + final isReservedInternal = logic.naming == Naming.reserved && !logic.isPort; + if (logic.naming == Naming.reserved || logic.isArrayMember) { + base = logic.name; + } else { + base = Sanitizer.sanitizeSV(logic.structureName); + } + + final name = _allocateUniqueSignalName( + base, + reserved: isReservedInternal, + ); + _signalNames[logic] = name; + return name; + } + + /// The base name that would be used for [logic] before uniquification. + static String baseName(Logic logic) => + (logic.naming == Naming.reserved || logic.isArrayMember) + ? logic.name + : Sanitizer.sanitizeSV(logic.structureName); + + /// Chooses the best name from a pool of merged [Logic] signals. + /// + /// When [constValue] is provided and [constNameDisallowed] is `false`, + /// the constant's value string is used directly as the name (no + /// uniquification). When [constNameDisallowed] is `true`, the constant + /// is excluded from the candidate pool and the normal priority applies. + /// + /// Priority (after constant handling): + /// 1. Port of this module (always wins — its name is already reserved). + /// 2. Reserved internal signal (exact name, throws on collision). + /// 3. Renameable signal. + /// 4. Preferred-available mergeable (base name not yet taken). + /// 5. Preferred-uniquifiable mergeable. + /// 6. Available-unpreferred mergeable. + /// 7. First unpreferred mergeable. + /// 8. Unnamed (prefer non-unpreferred base name). + /// + /// The winning name is allocated once and cached for the chosen [Logic]. + /// All other non-port [Logic]s in [candidates] are also cached to the + /// same name. + String signalNameOfBest( + Iterable candidates, { + Const? constValue, + bool constNameDisallowed = false, + }) { + if (constValue != null && !constNameDisallowed) { + return constValue.value.toString(); + } + + Logic? port; + Logic? reserved; + Logic? renameable; + final preferredMergeable = []; + final unpreferredMergeable = []; + final unnamed = []; + + for (final logic in candidates) { + if (_portLogics.contains(logic)) { + port = logic; + } else if (logic.isPort) { + if (Naming.isUnpreferred(baseName(logic))) { + unpreferredMergeable.add(logic); + } else { + preferredMergeable.add(logic); + } + } else if (logic.naming == Naming.reserved) { + reserved = logic; + } else if (logic.naming == Naming.renameable) { + renameable = logic; + } else if (logic.naming == Naming.mergeable) { + if (Naming.isUnpreferred(baseName(logic))) { + unpreferredMergeable.add(logic); + } else { + preferredMergeable.add(logic); + } + } else { + unnamed.add(logic); + } + } + + if (port != null) { + return _nameAndCacheAll(port, candidates); + } + + if (reserved != null) { + return _nameAndCacheAll(reserved, candidates); + } + + if (renameable != null) { + return _nameAndCacheAll(renameable, candidates); + } + + for (final logic in preferredMergeable) { + if (_isSignalAvailable(baseName(logic))) { + return _nameAndCacheAll(logic, candidates); + } + } + + if (preferredMergeable.isNotEmpty) { + return _nameAndCacheAll(preferredMergeable.first, candidates); + } + + if (unpreferredMergeable.isNotEmpty) { + final best = unpreferredMergeable + .firstWhereOrNull((e) => _isSignalAvailable(baseName(e))) ?? + unpreferredMergeable.first; + return _nameAndCacheAll(best, candidates); + } + + if (unnamed.isNotEmpty) { + final best = + unnamed.firstWhereOrNull((e) => !Naming.isUnpreferred(baseName(e))) ?? + unnamed.first; + return _nameAndCacheAll(best, candidates); + } + + throw StateError('No Logic candidates to name.'); + } + + /// Names [chosen] via [signalNameOf], then caches the same name for all + /// other non-port [Logic]s in [all]. + String _nameAndCacheAll(Logic chosen, Iterable all) { + final name = signalNameOf(chosen); + for (final logic in all) { + if (!identical(logic, chosen) && !_portLogics.contains(logic)) { + _signalNames[logic] = name; + } + } + return name; + } +} diff --git a/lib/src/utilities/signal_namer.dart b/lib/src/utilities/signal_namer.dart index 7f98fdff3..1f217489c 100644 --- a/lib/src/utilities/signal_namer.dart +++ b/lib/src/utilities/signal_namer.dart @@ -22,6 +22,19 @@ import 'package:rohd/src/utilities/uniquifier.dart'; /// named lazily on the first [nameOf] call. @internal class SignalNamer { + /// Controls whether synthesized signal names and instance names must be + /// unique across both namespaces. + /// + /// When `true` (the default), central naming cross-checks both namespaces + /// during allocation so that no identifier appears as both a signal and an + /// instance name. This is necessary for simulators like Icarus Verilog + /// that reject duplicate identifiers even across namespace boundaries. + /// + /// When `false`, signal and instance names are uniquified independently, + /// matching strict SystemVerilog semantics where instance and signal + /// identifiers occupy separate namespaces. + static bool uniquifySignalAndInstanceNames = true; + final Uniquifier _uniquifier; final bool Function(String name) _isAvailableInOtherNamespace; @@ -89,14 +102,13 @@ class SignalNamer { uniquifier: uniquifier, portRenames: portRenames, portLogics: portLogics, - isAvailableInOtherNamespace: - isAvailableInOtherNamespace ?? ((_) => true), + isAvailableInOtherNamespace: isAvailableInOtherNamespace ?? (_) => true, ); } bool _isAvailable(String name, {bool reserved = false}) => _uniquifier.isAvailable(name, reserved: reserved) && - _isAvailableInOtherNamespace(name); + (!uniquifySignalAndInstanceNames || _isAvailableInOtherNamespace(name)); String _allocateUniqueName(String baseName, {bool reserved = false}) { if (reserved) { diff --git a/test/instance_signal_name_collision_test.dart b/test/instance_signal_name_collision_test.dart index 6ee10de92..c369f83e4 100644 --- a/test/instance_signal_name_collision_test.dart +++ b/test/instance_signal_name_collision_test.dart @@ -18,7 +18,7 @@ import 'package:rohd/rohd.dart'; import 'package:rohd/src/synthesizers/utilities/utilities.dart'; -import 'package:rohd/src/utilities/config.dart'; +import 'package:rohd/src/utilities/namer.dart'; import 'package:test/test.dart'; // ── Minimal repro modules ──────────────────────────────────────────────────── @@ -61,8 +61,8 @@ void main() { late bool previousSetting; setUpAll(() async { - previousSetting = Config.ensureUniqueSignalAndInstanceNames; - Config.ensureUniqueSignalAndInstanceNames = false; + previousSetting = Namer.uniquifySignalAndInstanceNames; + Namer.uniquifySignalAndInstanceNames = false; mod = _CollidingParent(Logic(width: 8)); await mod.build(); @@ -70,7 +70,7 @@ void main() { }); tearDownAll(() { - Config.ensureUniqueSignalAndInstanceNames = previousSetting; + Namer.uniquifySignalAndInstanceNames = previousSetting; }); test('internal signal named "inner" retains its exact name', () { diff --git a/test/naming_consistency_test.dart b/test/naming_consistency_test.dart index b569bd4d6..c79221baa 100644 --- a/test/naming_consistency_test.dart +++ b/test/naming_consistency_test.dart @@ -4,7 +4,7 @@ // naming_consistency_test.dart // Validates that both the SystemVerilog synthesizer and a base // SynthModuleDefinition (used by the netlist synthesizer) produce -// consistent signal names via the shared Module.signalNamer. +// consistent signal names via the shared Module.namer. // // 2026 April 10 // Author: Desmond Kirkpatrick @@ -100,7 +100,7 @@ void main() { final svDef = SystemVerilogSynthModuleDefinition(mod); // Base path (same as netlist synthesizer uses) - // Since signalNamer is late final, the second constructor reuses + // Since namer is late final, the second constructor reuses // the same naming state — names must be consistent. final baseDef = SynthModuleDefinition(mod); @@ -181,12 +181,11 @@ void main() { } }); - test('signalNamer is shared across multiple SynthModuleDefinitions', - () async { + test('namer is shared across multiple SynthModuleDefinitions', () async { final mod = _Outer(Logic(width: 8), Logic(width: 8)); await mod.build(); - // Build one def, then build another — same signalNamer instance. + // Build one def, then build another — same namer instance. final def1 = SynthModuleDefinition(mod); final def2 = SynthModuleDefinition(mod); @@ -202,27 +201,27 @@ void main() { } }); - test('Module.signalName matches SynthLogic.name for ports', () async { + test('Namer.signalNameOf matches SynthLogic.name for ports', () async { final mod = _Outer(Logic(width: 8), Logic(width: 8)); await mod.build(); final def = SynthModuleDefinition(mod); final synthNames = _collectNames(def); - // Module.signalName uses SignalNamer.nameOf directly + // Module.namer.signalNameOf uses Namer directly for (final port in [...mod.inputs.values, ...mod.outputs.values]) { - final moduleName = mod.signalName(port); + final moduleName = mod.namer.signalNameOf(port); final synthName = synthNames[port]; expect(synthName, moduleName, - reason: 'SynthLogic.name and Module.signalName must agree ' + reason: 'SynthLogic.name and Module.namer.signalNameOf must agree ' 'for port ${port.name}'); } }); test('submodule instance names are allocated from the instance namespace', () async { - // Instance names come from Module.allocateInstanceName, which is - // separate from the signal namespace (Module.allocateSignalName). + // Instance names come from Module.namer.allocateInstanceName, which is + // separate from the signal namespace (Module.namer.allocateSignalName). // A signal and a submodule instance may therefore share the same // identifier without collision — matching SystemVerilog semantics. final mod = _Outer(Logic(width: 8), Logic(width: 8)); @@ -242,7 +241,7 @@ void main() { // Instance names are claimed in the *instance* namespace, NOT the // signal namespace. for (final name in instNames) { - expect(mod.isInstanceNameAvailable(name), isFalse, + expect(mod.namer.isInstanceNameAvailable(name), isFalse, reason: 'Instance name "$name" should be claimed in instance ' 'namespace'); } diff --git a/test/naming_namespace_test.dart b/test/naming_namespace_test.dart new file mode 100644 index 000000000..32e55629d --- /dev/null +++ b/test/naming_namespace_test.dart @@ -0,0 +1,180 @@ +// Copyright (C) 2026 Intel Corporation +// SPDX-License-Identifier: BSD-3-Clause +// +// naming_namespace_test.dart +// Tests for constant naming via nameOfBest, the tryMerge guard for +// constNameDisallowed, and separate instance/signal namespaces. +// +// 2026 April +// Author: Desmond Kirkpatrick + +import 'package:rohd/rohd.dart'; +import 'package:rohd/src/utilities/namer.dart'; +import 'package:test/test.dart'; + +/// A simple submodule whose instance name can collide with a signal name. +class _Inner extends Module { + _Inner(Logic a, {super.name = 'inner'}) { + a = addInput('a', a); + addOutput('b') <= ~a; + } +} + +/// Top module that has a signal named the same as a submodule instance. +class _InstanceSignalCollision extends Module { + _InstanceSignalCollision({String instanceName = 'inner'}) + : super(name: 'top') { + final a = addInput('a', Logic()); + final o = addOutput('o'); + + // Create a signal whose name matches the submodule instance name. + final sig = Logic(name: instanceName); + sig <= ~a; + + final sub = _Inner(sig, name: instanceName); + o <= sub.output('b'); + } +} + +/// Top module with two submodule instances that have the same name. +class _DuplicateInstances extends Module { + _DuplicateInstances() : super(name: 'top') { + final a = addInput('a', Logic()); + final o = addOutput('o'); + + final sub1 = _Inner(a, name: 'blk'); + final sub2 = _Inner(sub1.output('b'), name: 'blk'); + o <= sub2.output('b'); + } +} + +/// Module that uses a constant in a connection chain, exercising constant +/// naming through nameOfBest. +class _ConstantNamingModule extends Module { + _ConstantNamingModule() : super(name: 'const_mod') { + final a = addInput('a', Logic()); + final o = addOutput('o'); + + // A constant "1" drives one input of the AND gate. + o <= a & Const(1); + } +} + +/// Module with a mux where one input is a constant, exercising the +/// constNameDisallowed path — the mux output cannot use the constant's +/// literal as its name because it also carries non-constant values. +class _ConstNameDisallowedModule extends Module { + _ConstNameDisallowedModule() : super(name: 'const_disallow') { + final a = addInput('a', Logic()); + final sel = addInput('sel', Logic()); + final o = addOutput('o'); + + // mux output can be the constant OR a, so the constant name is disallowed. + o <= mux(sel, Const(1), a); + } +} + +void main() { + tearDown(() async { + await Simulator.reset(); + // Restore default. + Namer.uniquifySignalAndInstanceNames = true; + }); + + group('constant naming via nameOfBest', () { + test('constant value appears as literal in SV output', () async { + final dut = _ConstantNamingModule(); + await dut.build(); + final sv = dut.generateSynth(); + + // The constant "1" should appear as a literal 1'h1 in the output, + // not as a declared signal. + expect(sv, contains("1'h1")); + }); + + test('constNameDisallowed falls through to signal naming', () async { + final dut = _ConstNameDisallowedModule(); + await dut.build(); + final sv = dut.generateSynth(); + + // The output assignment should NOT use the raw constant literal + // as a wire name; a proper signal name should be used instead. + // The constant still appears as a literal in the mux expression. + expect(sv, contains("1'h1")); + // The output 'o' should be assigned from something. + expect(sv, contains('o')); + }); + }); + + group('separate instance and signal namespaces', () { + test( + 'signal and instance with same name do not collide ' + 'when namespaces are independent', () async { + Namer.uniquifySignalAndInstanceNames = false; + final dut = _InstanceSignalCollision(); + await dut.build(); + final sv = dut.generateSynth(); + + // With independent namespaces, the signal keeps its name 'inner' + // and the instance also keeps 'inner' — no spurious _0 suffix. + expect(sv, contains(RegExp(r'logic\s+inner[,;\s]'))); + expect(sv, isNot(contains('inner_0'))); + }); + + test( + 'signal and instance get suffixed when ' + 'ensureUniqueSignalAndInstanceNames is true', () async { + Namer.uniquifySignalAndInstanceNames = true; + final dut = _InstanceSignalCollision(); + await dut.build(); + final sv = dut.generateSynth(); + + // With cross-namespace checking enabled, the signal 'inner' is + // allocated first (during signal naming); when the instance tries + // to claim 'inner', it sees the signal namespace has it, so the + // instance OR signal gets a suffix. + expect(sv, contains('inner_0')); + }); + + test( + 'signal and instance do not spuriously suffix when ' + 'ensureUniqueSignalAndInstanceNames is false', () async { + Namer.uniquifySignalAndInstanceNames = false; + final dut = _InstanceSignalCollision(); + await dut.build(); + final sv = dut.generateSynth(); + + // With independent namespaces, no spurious suffixing. + expect(sv, isNot(contains('inner_0'))); + }); + + test('duplicate instance names get uniquified', () async { + final dut = _DuplicateInstances(); + await dut.build(); + final sv = dut.generateSynth(); + + // Two instances named 'blk' — one should be 'blk', the other 'blk_0'. + expect(sv, contains('blk')); + expect(sv, contains(RegExp(r'blk_\d'))); + }); + }); + + group('instance namespace independence', () { + test('allocateInstanceName is independent from allocateSignalName', + () async { + final dut = _InstanceSignalCollision(); + await dut.build(); + + // After build, the signal namer has 'inner' claimed. + // With independent namespaces, instance namespace should also accept + // 'inner' without conflict. + Namer.uniquifySignalAndInstanceNames = false; + + // The instance namespace should show 'inner' as available before + // any instance allocation. + // (After synthesis, names are already allocated, so we just verify + // the module built without error.) + expect(dut.generateSynth(), isNotEmpty); + }); + }); +} From 61d031928feb5156f1ab8bf697571bc9077b665e Mon Sep 17 00:00:00 2001 From: "Desmond A. Kirkpatrick" Date: Sun, 19 Apr 2026 20:14:20 -0700 Subject: [PATCH 8/8] signal registry --- test/signal_registry_test.dart | 142 +++++++++++++++++++++++++++++++++ 1 file changed, 142 insertions(+) create mode 100644 test/signal_registry_test.dart diff --git a/test/signal_registry_test.dart b/test/signal_registry_test.dart new file mode 100644 index 000000000..152b6091a --- /dev/null +++ b/test/signal_registry_test.dart @@ -0,0 +1,142 @@ +// Copyright (C) 2026 Intel Corporation +// SPDX-License-Identifier: BSD-3-Clause +// +// signal_registry_test.dart +// Tests for Module canonical naming (SynthesisNameRegistry). +// +// 2026 April 14 + +import 'package:rohd/rohd.dart'; +import 'package:test/test.dart'; + +// ──────────────────────────────────────────────────────────────── +// Simple test modules +// ──────────────────────────────────────────────────────────────── + +class _GateMod extends Module { + _GateMod(Logic a, Logic b) : super(name: 'gatetestmodule') { + a = addInput('a', a); + b = addInput('b', b); + final aBar = addOutput('a_bar'); + final aAndB = addOutput('a_and_b'); + aBar <= ~a; + aAndB <= a & b; + } +} + +class _Counter extends Module { + _Counter(Logic en, Logic reset, {int width = 8}) : super(name: 'counter') { + en = addInput('en', en); + reset = addInput('reset', reset); + final val = addOutput('val', width: width); + final nextVal = Logic(name: 'nextVal', width: width); + nextVal <= val + 1; + Sequential.multi([ + SimpleClockGenerator(10).clk, + reset, + ], [ + If(reset, then: [ + val < 0, + ], orElse: [ + If(en, then: [val < nextVal]), + ]), + ]); + } +} + +void main() { + tearDown(() async { + await Simulator.reset(); + }); + + group('signalName basics', () { + test('returns port names after build', () async { + final mod = _GateMod(Logic(), Logic()); + await mod.build(); + + expect(mod.namer.signalNameOf(mod.input('a')), equals('a')); + expect(mod.namer.signalNameOf(mod.input('b')), equals('b')); + expect(mod.namer.signalNameOf(mod.output('a_bar')), equals('a_bar')); + expect(mod.namer.signalNameOf(mod.output('a_and_b')), equals('a_and_b')); + }); + + test('returns internal signal names', () async { + final mod = _Counter(Logic(), Logic()); + await mod.build(); + + expect(mod.namer.signalNameOf(mod.input('en')), equals('en')); + expect(mod.namer.signalNameOf(mod.input('reset')), equals('reset')); + expect(mod.namer.signalNameOf(mod.output('val')), equals('val')); + }); + + test('agrees with signalName after synth', () async { + final mod = _Counter(Logic(), Logic()); + await mod.build(); + + for (final entry in mod.inputs.entries) { + expect( + mod.namer.signalNameOf(entry.value), + isNotNull, + reason: 'signalName should work for input ${entry.key}', + ); + } + for (final entry in mod.outputs.entries) { + expect( + mod.namer.signalNameOf(entry.value), + isNotNull, + reason: 'signalName should work for output ${entry.key}', + ); + } + }); + }); + + group('allocateSignalName', () { + test('avoids collision with existing names', () async { + final mod = _Counter(Logic(), Logic()); + await mod.build(); + + final allocated = mod.namer.allocateSignalName('en'); + expect(allocated, isNot(equals('en')), + reason: 'Should not collide with existing port name'); + expect(allocated, contains('en'), + reason: 'Should be based on the requested name'); + }); + + test('successive allocations are unique', () async { + final mod = _Counter(Logic(), Logic()); + await mod.build(); + + final a = mod.namer.allocateSignalName('wire'); + final b = mod.namer.allocateSignalName('wire'); + expect(a, isNot(equals(b)), reason: 'Each allocation should be unique'); + }); + }); + + group('sparse storage', () { + test('identity names not stored in renames', () async { + final mod = _GateMod(Logic(), Logic()); + await mod.build(); + + expect(mod.namer.signalNameOf(mod.input('a')), equals('a')); + expect(mod.input('a').name, equals('a')); + }); + }); + + group('determinism', () { + test('same module produces identical canonical names', () async { + Future> buildAndGetNames() async { + final mod = _Counter(Logic(), Logic()); + await mod.build(); + return { + for (final sig in mod.signals) sig.name: mod.namer.signalNameOf(sig), + }; + } + + final names1 = await buildAndGetNames(); + await Simulator.reset(); + final names2 = await buildAndGetNames(); + + expect(names1, equals(names2)); + }); + }); +}