Reserve definition parameter names in the module namespace

[Shubham-Padkonde]

Description & Motivation

SystemVerilog.definitionParameters are emitted straight into the module header by _verilogParameters, without ever passing through the module's Namer. Ports, internal signals and submodule instances all share one namespace managed by that Namer/Uniquifier; parameters were simply not part of it. So a parameter could take a name that was already in use, and the generated module declared the same identifier twice.

On main at d22fe80, a module with input a, output b and an internal signal myInternal:

parameter named	generated SystemVerilog
a	parameter int a = 3 alongside input logic [7:0] a,
b	parameter int b = 3 alongside output logic [7:0] b
myInternal	parameter int myInternal = 3 alongside logic [7:0] myInternal;

All three are illegal SystemVerilog — a parameter and a port/variable cannot share an identifier in the same scope.

The fix reserves the parameter names in the shared namespace when the Namer for a module is built, right after the port names. That gives the three outcomes the namespace already implies for everything else:

collision	after this change
parameter vs. port (a, b)	throws UnavailableReservedNameException at build — a port name cannot move
parameter vs. renameable internal signal	signal is uniquified to myInternal_0; parameter int myInternal = 3 stands, and the module is legal
parameter vs. reserved internal signal	throws UnavailableReservedNameException — the user asked for both names explicitly, so neither can move
parameter that collides with nothing	unchanged

This also means a duplicate parameter name within one module now throws rather than emitting the declaration twice.

I put the reservation in Namer.forModule rather than in the SystemVerilog synthesis result because that is where the namespace is established, so instances and signals allocated later automatically route around the parameter names instead of needing a second collision check at emit time.

Related Issue(s)

Fixes #611

Testing

New test/sv_param_naming_test.dart covers all four rows of the table above:

00:00 +5: All tests passed!

The three throwing cases fail on the unpatched tree (they generate the illegal module instead of throwing).

Full suite, dart test -j 3, clean main versus this branch:

04:34 +959 ~51 -772: Some tests failed.    <- main
04:39 +964 ~51 -772: Some tests failed.    <- this branch

The +5 is exactly the new tests here; skips and failures are identical. All 772 failures are Icarus Verilog missing from my machine — 743 report ProcessException ... Command: iverilog -g2012 ... and no other exception type appears in either log. CONTRIBUTING notes iverilog is required for the complete suite, and CI has it.

dart format --output=none --set-exit-if-changed reports no changes, and dart analyze over the package finds no issues.

Notably, sv_param_passthrough_test.dart keeps passing: its parameters are A, B, C against ports a, b, and SystemVerilog identifiers are case-sensitive, so they do not collide.

Backwards-compatibility

This turns two cases that previously produced silently-illegal SystemVerilog into build-time exceptions, so a design that was relying on such a name today will now fail loudly. That is the enforcement #611 asks for, and the output it replaces would not have compiled anyway.

The renameable-signal case is not a breaking change in the same way, but it does change generated signal names (myInternal becomes myInternal_0) for designs that had this collision.

I reused UnavailableReservedNameException rather than introducing a new exception type, since that is already how reserved-name conflicts surface elsewhere in the namer. If you would prefer a dedicated exception that names the parameter and module explicitly, that is an easy change — the message today is the generic Unable to use reserved name "a" because something else already has this name.

Documentation

Yes, and included: the Namer.forModule doc comment now explains that definition parameters are reserved along with ports and what happens on a collision. No user-guide page covers definitionParameters naming, so nothing outside the API docs needed updating.

---

Disclosure: this change was written by Claude Code (Claude Opus 5) working as my agent, at my direction. The generated SystemVerilog and test output quoted above come from real runs in my local environment; I am accountable for the content of this PR and happy to iterate on review feedback.

🤖 Generated with Claude Code