From ea9aacc6b4e3a18e6864334436960df30d906355 Mon Sep 17 00:00:00 2001
From: Shubham Padkonde <shubhampadkonde12@gmail.com>
Date: Mon, 21 Sep 2026 09:13:00 +0530
Subject: [PATCH] Reserve definition parameter names in the module namespace

SystemVerilog.definitionParameters names were emitted straight into the
module header without ever passing through the module's Namer, so they
did not participate in the shared namespace that ports, signals and
instances share. A parameter could therefore take a name that was
already in use, and the generated module declared the same identifier
twice:

  module Colliding #(
  parameter int a = 3
  input logic [7:0] a,
  ...

Reserve the parameter names alongside the port names when the Namer for
a module is built. A signal or instance that would have picked the same
name is now uniquified away from it, and a parameter that collides with
a name which cannot move -- a port, or a reserved internal signal --
throws UnavailableReservedNameException during build instead of
producing an illegal module.

Fixes #611

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 lib/src/utilities/namer.dart   | 15 ++++++
 test/sv_param_naming_test.dart | 83 ++++++++++++++++++++++++++++++++++
 2 files changed, 98 insertions(+)
 create mode 100755 test/sv_param_naming_test.dart

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 = <Logic>{
       ...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 <shubhampadkonde12@gmail.com>
+
+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<SystemVerilogParameterDefinition> 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<String> 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<UnavailableReservedNameException>()));
+      });
+    }
+
+    test('such as a reserved internal signal', () {
+      expect(
+          () async => synthOf(ParameterizedMod(Logic(width: 8), 'myInternal',
+              internalNaming: Naming.reserved)),
+          throwsA(isA<UnavailableReservedNameException>()));
+    });
+  });
+
+  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;'));
+  });
+}