diff --git a/lib/src/utilities/namer.dart b/lib/src/utilities/namer.dart index d5f20e783..354dd17eb 100644 --- a/lib/src/utilities/namer.dart +++ b/lib/src/utilities/namer.dart @@ -51,6 +51,15 @@ class Namer { /// /// Port names are reserved in the shared namespace. Port names are /// guaranteed sanitary by [Module]'s `_checkForSafePortName`. + /// + /// Any [SystemVerilog.definitionParameters] are reserved as well, since a + /// generated parameter declaration shares the module's namespace with its + /// ports, signals and instances. Reserving them means an internal signal + /// or instance that would otherwise pick the same name is uniquified away + /// from it, and a parameter that collides with a name which cannot move -- + /// a port, or another reserved name -- throws an + /// [UnavailableReservedNameException] rather than generating a module with + /// two declarations of the same identifier. factory Namer.forModule(Module module) { final portLogics = { ...module.inputs.values, @@ -63,6 +72,12 @@ class Namer { uniquifier.getUniqueName(initialName: logic.name, reserved: true); } + if (module is SystemVerilog) { + for (final parameter in module.definitionParameters ?? const []) { + uniquifier.getUniqueName(initialName: parameter.name, reserved: true); + } + } + return Namer._(uniquifier: uniquifier, portLogics: portLogics); } diff --git a/test/sv_param_naming_test.dart b/test/sv_param_naming_test.dart new file mode 100755 index 000000000..790ab0089 --- /dev/null +++ b/test/sv_param_naming_test.dart @@ -0,0 +1,83 @@ +// Copyright (C) 2026 Intel Corporation +// SPDX-License-Identifier: BSD-3-Clause +// +// sv_param_naming_test.dart +// Unit tests for SystemVerilog parameter names sharing the module namespace +// +// 2026 September 21 +// Author: Shubham Padkonde + +import 'package:rohd/rohd.dart'; +import 'package:test/test.dart'; + +/// A module with one definition parameter and one internal signal, so a +/// parameter name can be made to collide with a port, an internal signal, or +/// nothing at all. +class ParameterizedMod extends Module with SystemVerilog { + @override + final List definitionParameters; + + ParameterizedMod( + Logic a, + String parameterName, { + Naming internalNaming = Naming.renameable, + super.name = 'parameterized_mod', + }) : definitionParameters = [ + SystemVerilogParameterDefinition(parameterName, + type: 'int', defaultValue: '3'), + ] { + a = addInput('a', a, width: 8); + + final internal = + Logic(name: 'myInternal', width: 8, naming: internalNaming); + internal <= a; + + addOutput('b', width: 8) <= internal; + } + + @override + String? definitionVerilog(String definitionType) => null; +} + +Future synthOf(ParameterizedMod mod) async { + await mod.build(); + return mod.generateSynth(); +} + +void main() { + group('a definition parameter cannot take a name that cannot move', () { + for (final portName in ['a', 'b']) { + test('such as the port $portName', () { + expect( + () async => + synthOf(ParameterizedMod(Logic(width: 8), portName)), + throwsA(isA())); + }); + } + + test('such as a reserved internal signal', () { + expect( + () async => synthOf(ParameterizedMod(Logic(width: 8), 'myInternal', + internalNaming: Naming.reserved)), + throwsA(isA())); + }); + }); + + test('a renameable signal is moved out of a parameter name', () async { + final sv = await synthOf(ParameterizedMod(Logic(width: 8), 'myInternal')); + + expect(sv, contains('parameter int myInternal = 3')); + + // The signal must no longer be declared as plain `myInternal`, or the + // module would declare that identifier twice. + expect(sv, isNot(contains('logic [7:0] myInternal;'))); + expect(sv, contains('myInternal_0')); + }); + + test('a parameter that collides with nothing is left alone', () async { + final sv = await synthOf(ParameterizedMod(Logic(width: 8), 'WIDTH')); + + expect(sv, contains('parameter int WIDTH = 3')); + expect(sv, contains('logic [7:0] myInternal;')); + }); +}