Fix bugs related to module naming and instantiation

lib/src/module.dart

@@ -407,6 +407,9 @@ abstract class Module {
     } else {
       if (!dontAddSignal && !isOutput(signal) && subModule == null) {
         _addInternalSignal(signal);
+        for (final dstConnection in signal.dstConnections) {
+          await _traceInputForModuleContents(dstConnection);
+        }
       }
       if (signal.srcConnection != null) {
         await _traceOutputForModuleContents(signal.srcConnection!);

lib/src/synthesizers/systemverilog.dart

@@ -298,19 +298,21 @@ class _SynthModuleDefinition {
   @override
   String toString() => "module name: '${module.name}'";
 
-  late final Uniquifier _synthLogicNameUniquifier;
+  /// Used to uniquify any identifiers, including signal names
+  /// and module instances.
+  late final Uniquifier _synthInstantiationNameUniquifier;
+
   String _getUniqueSynthLogicName(String? initialName, bool portName) {
     if (portName && initialName == null) {
       throw Exception('Port name cannot be null.');
     }
-    return _synthLogicNameUniquifier.getUniqueName(
+    return _synthInstantiationNameUniquifier.getUniqueName(
         initialName: initialName, reserved: portName);
   }
 
-  final Uniquifier _synthSubModuleInstantiationNameUniquifier = Uniquifier();
   String _getUniqueSynthSubModuleInstantiationName(
           String? initialName, bool reserved) =>
-      _synthSubModuleInstantiationNameUniquifier.getUniqueName(
+      _synthInstantiationNameUniquifier.getUniqueName(
           initialName: initialName, nullStarter: 'm', reserved: reserved);
 
   _SynthLogic? _getSynthLogic(Logic? logic, bool allowPortName) {
@@ -328,7 +330,7 @@ class _SynthModuleDefinition {
   }
 
   _SynthModuleDefinition(this.module) {
-    _synthLogicNameUniquifier = Uniquifier(
+    _synthInstantiationNameUniquifier = Uniquifier(
         reservedNames: {...module.inputs.keys, ...module.outputs.keys});
 
     // start by traversing output signals

test/multimodule5_test.dart

@@ -0,0 +1,39 @@
+/// Copyright (C) 2022 Intel Corporation
+/// SPDX-License-Identifier: BSD-3-Clause
+///
+/// multimodule5_test.dart
+/// Unit tests for a hierarchy of multiple modules and multiple instantiation
+/// (another type)
+///
+/// 2022 November 22
+/// Author: Max Korbel <max.korbel@intel.com>
+///
+
+import 'package:rohd/rohd.dart';
+import 'package:rohd/src/modules/passthrough.dart';
+import 'package:test/test.dart';
+
+class TopModule extends Module {
+  TopModule(Logic inPort) {
+    inPort = addInput('inPort', inPort);
+
+    final internalNet = Logic(name: 'internalNet');
+    final outPort = addOutput('outPort');
+
+    Combinational([internalNet < inPort]);
+    Combinational([outPort < internalNet]);
+
+    Passthrough(internalNet);
+  }
+}
+
+void main() {
+  test('multimodules5', () async {
+    final mod = TopModule(Logic());
+    await mod.build();
+
+    final sv = mod.generateSynth();
+
+    expect(sv, contains('Passthrough'));
+  });
+}

test/name_test.dart

@@ -6,9 +6,11 @@
 ///
 /// 2022 March 7
 /// Author: Max Korbel <max.korbel@intel.com>
+///
 
 // ignore_for_file: avoid_positional_boolean_parameters
 import 'package:rohd/rohd.dart';
+import 'package:rohd/src/utilities/simcompare.dart';
 import 'package:test/test.dart';
 
 class TopModule extends Module {
@@ -23,18 +25,138 @@ class TopModule extends Module {
 }
 
 class SpeciallyNamedModule extends Module {
-  SpeciallyNamedModule(Logic a, bool reserveDefName, bool reserveInstanceName)
-      : super(
-          name: 'specialNameInstance',
+  SpeciallyNamedModule(
+    Logic a,
+    bool reserveDefName,
+    bool reserveInstanceName, {
+    super.name = 'specialInstanceName',
+    super.definitionName = 'specialName',
+  }) : super(
           reserveName: reserveInstanceName,
-          definitionName: 'specialName',
           reserveDefinitionName: reserveDefName,
         ) {
     addInput('a', a, width: a.width);
   }
 }
 
+class RenameableModule extends Module {
+  final String inputPortName;
+  final String outputPortName;
+  RenameableModule(
+    Logic inputPort, {
+    this.outputPortName = 'outputPort',
+    String internalSignalName = 'internalSignal',
+    String internalModuleInstanceName = 'internalModuleInstanceName',
+    String internalModuleDefinitionName = 'internalModuleDefinitionName',
+    super.definitionName = 'moduleDefinitionName',
+    super.name = 'moduleInstanceName',
+    super.reserveDefinitionName = true,
+    super.reserveName = true,
+  }) : inputPortName = inputPort.name {
+    inputPort = addInput(inputPort.name, inputPort);
+    final outputPort = addOutput(outputPortName);
+
+    final internalSignal = Logic(name: internalSignalName);
+
+    Combinational([internalSignal < ~inputPort]);
+    Combinational([outputPort < internalSignal]);
+
+    SpeciallyNamedModule(
+      ~internalSignal,
+      true,
+      false,
+      name: internalModuleInstanceName,
+      definitionName: internalModuleDefinitionName,
+    );
+  }
+}
+
+enum NameType {
+  inputPort,
+  outputPort,
+  internalSignal,
+  internalModuleInstance,
+  internalModuleDefinition,
+  topDefinitionName,
+  topName
+}
+
 void main() {
+  tearDown(Simulator.reset);
+
+  group('signal and module naming conflicts', () {
+    Future<void> runTest(RenameableModule mod) async {
+      await mod.build();
+
+      final vectors = [
+        Vector({mod.inputPortName: 0}, {mod.outputPortName: 1}),
+        Vector({mod.inputPortName: 1}, {mod.outputPortName: 0}),
+      ];
+
+      await SimCompare.checkFunctionalVector(mod, vectors);
+      final simResult = SimCompare.iverilogVector(
+        mod.generateSynth(),
+        mod.definitionName,
+        vectors,
+      );
+      expect(simResult, equals(true));
+    }
+
+    Future<void> runTestGen(Map<NameType, String> names) async =>
+        runTest(RenameableModule(
+          Logic(name: names[NameType.inputPort]),
+          outputPortName: names[NameType.outputPort]!,
+          internalSignalName: names[NameType.internalSignal]!,
+          internalModuleInstanceName: names[NameType.internalModuleInstance]!,
+          internalModuleDefinitionName:
+              names[NameType.internalModuleDefinition]!,
+          definitionName: names[NameType.topDefinitionName],
+          name: names[NameType.topName]!,
+        ));
+
+    for (var i = 0; i < NameType.values.length; i++) {
+      for (var j = i + 1; j < NameType.values.length; j++) {
+        final nameType1 = NameType.values[i];
+        final nameType2 = NameType.values[j];
+        final nameTypes = [nameType1, nameType2];
+
+        // skip ones that actually *should* cause a failure
+        final skips = [
+          [NameType.internalModuleDefinition, NameType.topDefinitionName],
+          [NameType.inputPort, NameType.outputPort]
+        ];
+
+        var doSkip = false;
+        for (final skip in skips) {
+          if (nameTypes.contains(skip[0]) && nameTypes.contains(skip[1])) {
+            doSkip = true;
+            break;
+          }
+        }
+        if (doSkip) {
+          continue;
+        }
+
+        test('${nameType1.name} == ${nameType2.name}', () async {
+          final testMap = Map.fromEntries(List.generate(NameType.values.length,
+              (k) => MapEntry(NameType.values[k], 'uniqueName$k')));
+          testMap[nameType1] = 'conflictingName';
+          testMap[nameType2] = testMap[nameType1]!;
+          await runTestGen(testMap);
+        });
+      }
+    }
+
+    test('input port name != internal signal name', () async {
+      await runTest(
+          RenameableModule(Logic(name: 'apple'), internalSignalName: 'apple'));
+    });
+    test('output port name != internal signal name', () async {
+      await runTest(RenameableModule(Logic(),
+          internalSignalName: 'apple', outputPortName: 'apple'));
+    });
+  });
+
   group('definition name', () {
     test('respected with no conflicts', () async {
       final mod = SpeciallyNamedModule(Logic(), false, false);
@@ -60,8 +182,9 @@ void main() {
       final mod = TopModule(Logic(), false, false);
       await mod.build();
       final sv = mod.generateSynth();
-      expect(sv, contains('specialNameInstance('));
-      expect(sv, contains('specialNameInstance_0('));
+
+      expect(sv, contains('specialInstanceName('));
+      expect(sv, contains('specialInstanceName_0('));
     });
     test('reserved throws exception with conflicts', () async {
       final mod = TopModule(Logic(), false, true);