Improve SSA error debuggability with hierarchy (#635)

Author: mkorbel1

lib/src/module.dart

@@ -353,7 +353,7 @@ abstract class Module {
 
   /// Converts a [hierarchy] (like used in [_checkValidHierarchy]) into a string
   /// that can be used for error messages.
-  static String _hierarchyListToString(List<Module> hierarchy) =>
+  static String _hierarchyListToString(Iterable<Module> hierarchy) =>
       hierarchy.map((e) => e.name).join('.');
 
   /// Adds a [Module] to this as a subModule.
@@ -1107,6 +1107,11 @@ abstract class Module {
     return hier.toString();
   }
 
+  /// Returns the hierarchical name of this [Module] with the parent [hierarchy]
+  /// included, separated by `.`s, e.g. `top.mid.leaf`.  Because it depends on
+  /// [hierarchy], this is only valid after [build] has been called.
+  String get hierarchicalName => _hierarchyListToString(hierarchy());
+
   /// Returns a synthesized version of this [Module].
   ///
   /// Currently returns one long file in SystemVerilog, but in the future

lib/src/modules/conditionals/always.dart

@@ -1,4 +1,4 @@
-// Copyright (C) 2021-2024 Intel Corporation
+// Copyright (C) 2021-2025 Intel Corporation
 // SPDX-License-Identifier: BSD-3-Clause
 //
 // always.dart
@@ -135,8 +135,12 @@ abstract class Always extends Module with SystemVerilog {
       }
 
       // share the registration information down
-      conditional.updateAssignmentMaps(
-          assignedReceiverToOutputMap, assignedDriverToInputMap);
+      conditional.updateRegistration(
+        assignedReceiverToOutputMap: assignedReceiverToOutputMap,
+        assignedDriverToInputMap: assignedDriverToInputMap,
+        parentConditional: null,
+        parentAlways: this,
+      );
     }
   }
 

lib/src/modules/conditionals/conditional.dart

@@ -9,6 +9,7 @@
 
 import 'package:meta/meta.dart';
 import 'package:rohd/rohd.dart';
+import 'package:rohd/src/modules/conditionals/always.dart';
 import 'package:rohd/src/modules/conditionals/ssa.dart';
 
 /// Represents an some logical assignments or actions that will only happen
@@ -28,20 +29,67 @@ abstract class Conditional {
   /// This is used for things like [Sequential]'s pre-tick values.
   Map<Logic, LogicValue> _driverValueOverrideMap = {};
 
+  /// The [Conditional] that contains this [Conditional], if there is one.
+  ///
+  /// This is only initialized after it's been included inside an [Always].
+  late final Conditional? _parentConditional;
+
+  /// The [Always] parent of this [Conditional], if there is one.
+  ///
+  /// This is only initialized after it's been included inside an [Always].
+  late final Always _parentAlways;
+
+  /// A string representing the hierarchical path to this [Conditional],
+  /// including the module path to the [_parentAlways] and the names of
+  /// [Conditional]s within that [Always] down to `this` one.
+  ///
+  /// If this [Conditional] is not yet registered within an [Always], or if the
+  /// [_parentAlways] has not yet been built, this will only include the runtime
+  /// type of this [Conditional].
+  @protected
+  @internal
+  String get hierarchyString => [
+        if (_isRegistered && _parentAlways.hasBuilt)
+          _parentConditional?.hierarchyString ?? _parentAlways.hierarchicalName,
+        runtimeType
+      ].join('.');
+
+  /// Indicates whether [updateRegistration] has been called on this
+  /// [Conditional] already.
+  bool _isRegistered = false;
+
+  /// Updates registration information for the [Conditional], passed down from
+  /// the parent [Always] or [Conditional].
+  ///
   /// Updates the values of [_assignedReceiverToOutputMap] and
   /// [_assignedDriverToInputMap] and passes them down to all sub-[Conditional]s
   /// as well.
   @internal
-  void updateAssignmentMaps(
-    Map<Logic, Logic> assignedReceiverToOutputMap,
-    Map<Logic, Logic> assignedDriverToInputMap,
-  ) {
+  void updateRegistration({
+    required Map<Logic, Logic> assignedReceiverToOutputMap,
+    required Map<Logic, Logic> assignedDriverToInputMap,
+    required Conditional? parentConditional,
+    required Always parentAlways,
+  }) {
+    if (_isRegistered) {
+      throw InvalidConditionalException('Conditional $this is already included'
+          ' as part of another block: $hierarchyString');
+    }
+
     _assignedReceiverToOutputMap = assignedReceiverToOutputMap;
     _assignedDriverToInputMap = assignedDriverToInputMap;
+    _parentConditional = parentConditional;
+    _parentAlways = parentAlways;
     for (final conditional in conditionals) {
-      conditional.updateAssignmentMaps(
-          assignedReceiverToOutputMap, assignedDriverToInputMap);
+      conditional.updateRegistration(
+        assignedReceiverToOutputMap: assignedReceiverToOutputMap,
+        assignedDriverToInputMap: assignedDriverToInputMap,
+        parentConditional: this,
+        parentAlways: parentAlways,
+      );
     }
+
+    _isRegistered = true;
   }
 
   /// Updates the value of [_driverValueOverrideMap] and passes it down to all
@@ -204,7 +252,7 @@ abstract class Conditional {
         receiverOutput.put(LogicValue.x);
       }
     } on WriteAfterReadException catch (e) {
-      throw e.cloneWithAddedPath('  at (driving X) $this');
+      throw e.cloneWithAddedPath('  at (driving X) $this  [$hierarchyString]');
     }
 
     drivenSignals?.addAll(receivers);

lib/src/modules/conditionals/conditional_assign.dart

@@ -66,7 +66,7 @@ class ConditionalAssign extends Conditional {
         _receiverOutput.put(currentValue);
       }
     } on WriteAfterReadException catch (e) {
-      throw e.cloneWithAddedPath('  at $this');
+      throw e.cloneWithAddedPath('  at $this  [$hierarchyString]');
     }
 
     if (drivenSignals != null &&

test/comb_mod_test.dart

@@ -245,6 +245,8 @@ void main() {
           } on Exception catch (e) {
             expect(e, isA<WriteAfterReadException>());
             expect(e.toString(), contains('internalReadEn '));
+            expect(
+                e.toString(), contains('singlereadandwriterf.rf_read.If.Case'));
           }
         }
       });

test/conditionals_test.dart

@@ -1,4 +1,4 @@
-// Copyright (C) 2021-2023 Intel Corporation
+// Copyright (C) 2021-2025 Intel Corporation
 // SPDX-License-Identifier: BSD-3-Clause
 //
 // conditionals_test.dart
@@ -700,8 +700,8 @@ void main() {
 
     test('should return exception if a conditional is used multiple times.',
         () async {
-      expect(
-          () => MultipleConditionalModule(Logic(), Logic()), throwsException);
+      expect(() => MultipleConditionalModule(Logic(), Logic()),
+          throwsA(isA<InvalidConditionalException>()));
     });
   });
 

test/module_test.dart

@@ -386,4 +386,15 @@ void main() {
       }
     });
   });
+
+  test('hierarchicalName', () async {
+    final topInput = Logic();
+    final top = FlexibleModule(name: 'top')..addInput('a', topInput);
+    final mid = FlexibleModule(name: 'mid')..addInput('a', top.input('a'));
+    final sub = FlexibleModule(name: 'sub')..addInput('a', mid.input('a'));
+
+    await top.build();
+
+    expect(sub.hierarchicalName, 'top.mid.sub');
+  });
 }