Repository: intel/rohd (dart)

Issue:
LogicValue radixStrings should use underscore as separator, similar to Dart 3.6
### Motivation

Dart has moved to allowing underscores in digit strings to make them more readable.
LogicValue radixStrings were using spaces as separators, not a great choice.
It woud be nice to match Dart with underscores but also allow for other separators upon request.


### Desired solution

Add an optional seperatorChar for ofRadixString and toRadixString, defaulting to '_'.

### Alternatives considered

_No response_

### Additional details

_No response_

Pull request:
<!-- Please make sure you check out the contribution guidelines before submitting a pull request! -->

## Description & Motivation

Added an option to LogicValue.toRadixString and .ofRadixString to use '_' as a default separator, but allow
for the user to specify a different separator.

## Related Issue(s)

#534 

## Testing

Converted tests to use _ as separator after testing with sepChar argument.

## Backwards-compatibility

> Is this a breaking change that will not be backwards-compatible? If yes, how so?

Yes.  Previously output radixStrings will not parse by default as they will have spaces.

## Documentation

> Does the change require any updates to documentation? If so, where? Are they included?

Documentation in the class code is now up to date with this change.

Patch:
diff --git a/lib/src/exceptions/logic_value/logic_value_construction_exception.dart b/lib/src/exceptions/logic_value/logic_value_construction_exception.dart
index 603c518f6..639afe27d 100644
--- a/lib/src/exceptions/logic_value/logic_value_construction_exception.dart
+++ b/lib/src/exceptions/logic_value/logic_value_construction_exception.dart
@@ -2,7 +2,7 @@
 // SPDX-License-Identifier: BSD-3-Clause
 //
 // logic_value_construction_exception.dart
-// An exception that thrown when a signal failes to `put`.
+// An exception that is thrown when a signal fails to `put`.
 //
 // 2023 May 1
 // Author: Max Korbel <max.korbel@intel.com>
diff --git a/lib/src/exceptions/logic_value/logic_value_conversion_exception.dart b/lib/src/exceptions/logic_value/logic_value_conversion_exception.dart
new file mode 100644
index 000000000..a8d9622f3
--- /dev/null
+++ b/lib/src/exceptions/logic_value/logic_value_conversion_exception.dart
@@ -0,0 +1,19 @@
+// Copyright (C) 2024 Intel Corporation
+// SPDX-License-Identifier: BSD-3-Clause
+//
+// logic_value_conversion_exception.dart
+// An exception that is thrown when a conversion from a [LogicValue] fails
+// (such as to a [String]).
+//
+// 2024 December 26
+// Author: Desmond Kirkpatrick <desmond.a.kirkpatrick@intel.com>
+
+import 'package:rohd/rohd.dart';
+
+/// An exception that is thrown when a [LogicValue] cannot be
+/// properly converted.
+class LogicValueConversionException extends RohdException {
+  /// Creates an exception for when conversion of a `LogicValue` fails.
+  LogicValueConversionException(String message)
+      : super('Failed to convert `LogicValue`: $message');
+}
diff --git a/lib/src/exceptions/logic_value/logic_value_exceptions.dart b/lib/src/exceptions/logic_value/logic_value_exceptions.dart
index 4a00c1c18..a6f7818fb 100644
--- a/lib/src/exceptions/logic_value/logic_value_exceptions.dart
+++ b/lib/src/exceptions/logic_value/logic_value_exceptions.dart
@@ -4,4 +4,5 @@
 export 'invalid_truncation_exception.dart';
 export 'invalid_value_operation_exception.dart';
 export 'logic_value_construction_exception.dart';
+export 'logic_value_conversion_exception.dart';
 export 'value_width_mismatch_exception.dart';
diff --git a/lib/src/values/logic_value.dart b/lib/src/values/logic_value.dart
index cb7d2061e..cc6f456f6 100644
--- a/lib/src/values/logic_value.dart
+++ b/lib/src/values/logic_value.dart
@@ -111,7 +111,7 @@ abstract class LogicValue implements Comparable<LogicValue> {
                 ? _LogicValueEnum.x
                 : this == LogicValue.z
                     ? _LogicValueEnum.z
-                    : throw Exception('Failed to convert.');
+                    : throw LogicValueConversionException('Failed to convert.');
   }
 
   /// Creates a [LogicValue] of [val] using [of], but attempts to infer the
@@ -604,6 +604,9 @@ abstract class LogicValue implements Comparable<LogicValue> {
     }
   }
 
+  /// Legal characters in a radixString representation.
+  static const String radixStringChars = "'0123456789aAbBcCdDeEfFqohzZxX";
+
   /// Reverse a string (helper function)
   static String _reverse(String inString) =>
       String.fromCharCodes(inString.runes.toList().reversed);
@@ -616,15 +619,18 @@ abstract class LogicValue implements Comparable<LogicValue> {
   /// construct a [LogicValue].
   ///
   /// Here is the number 1492 printed as a radix string:
-  /// - Binary: `15'b101 1101 0100`
-  /// - Quaternary: `15'q11 3110`
+  /// - Binary: `15'b101_1101_0100`
+  /// - Quaternary: `15'q11_3110`
   /// - Octal: `15'o2724`
   /// - Decimal: `10'd1492`
   /// - Hex: `15'h05d4`
   ///
-  /// Spaces are output according to [chunkSize] starting from the LSB(right).
-  ///  - [chunkSize] = default:  `61'h2 9ebc 5f06 5bf7`
-  ///  - [chunkSize] = 10: `61'h29e bc5f065bf7`
+  /// Separators are output according to [chunkSize] starting from the
+  /// LSB(right), default is 4 characters.  The default separator is '_'.
+  /// [sepChar] can be set to another character, but not in [radixStringChars],
+  /// otherwise it will throw an exception.
+  ///  - [chunkSize] = default: `61'h2_9ebc_5f06_5bf7`
+  ///  - [chunkSize] = 10: `61'h29e_bc5f065bf7`
   ///
   /// Leading 0s are omitted in the output string:
   /// - `25'h1`
@@ -632,23 +638,27 @@ abstract class LogicValue implements Comparable<LogicValue> {
   /// When a [LogicValue] has 'x' or 'z' bits, then the radix characters those
   /// bits overlap will be expanded into binary form with '<' '>' bracketing
   /// them as follows:
-  ///   - `35'h7 ZZZZ Z<zzz0><100z>Z`
+  ///   - `35'h7_ZZZZ_Z<zzz0><100z>Z`
   /// Such a [LogicValue] cannot be converted to a Decimal (10) radix string
   /// and will throw an exception.
   ///
   /// If the leading bits are 'z', then the output radix character is 'Z' no
   /// matter what the length. When leading, 'Z' indicates one or more 'z'
   /// bits to fill the first radix character.
-  /// - `9'bz zzzz zzz = 9'hZZZ`
+  /// - `9'bz_zzzz_zzzz = 9'hZZZ`
   ///
-  String toRadixString({int radix = 2, int chunkSize = 4}) {
+  String toRadixString(
+      {int radix = 2, int chunkSize = 4, String sepChar = '_'}) {
+    if (radixStringChars.contains(sepChar)) {
+      throw LogicValueConversionException('separation character invalid');
+    }
     final radixStr = switch (radix) {
       2 => "'b",
       4 => "'q",
       8 => "'o",
       10 => "'d",
       16 => "'h",
-      _ => throw Exception('Unsupported radix: $radix')
+      _ => throw LogicValueConversionException('Unsupported radix: $radix')
     };
     final String reversedStr;
     if (isValid) {
@@ -657,7 +667,8 @@ abstract class LogicValue implements Comparable<LogicValue> {
       reversedStr = _reverse(radixString);
     } else {
       if (radix == 10) {
-        throw Exception('Cannot support decimal strings with invalid bits');
+        throw LogicValueConversionException(
+            'Cannot support decimal strings with invalid bits');
       }
       final span = (math.log(radix) / math.log(2)).ceil();
       final extendedStr =
@@ -688,10 +699,10 @@ abstract class LogicValue implements Comparable<LogicValue> {
     final spaceString = _reverse(reversedStr
         .replaceAllMapped(
             RegExp('((>(.){$chunkSize}<)|([a-zA-Z0-9])){$chunkSize}'),
-            (match) => '${match.group(0)} ')
-        .replaceAll(' <', '<'));
+            (match) => '${match.group(0)}$sepChar')
+        .replaceAll('$sepChar<', '<'));
 
-    final fullString = spaceString[0] == ' '
+    final fullString = spaceString[0] == sepChar
         ? spaceString.substring(1, spaceString.length)
         : spaceString;
     return '$width$radixStr$fullString';
@@ -716,6 +727,10 @@ abstract class LogicValue implements Comparable<LogicValue> {
   /// radix format.  Space-separation is for ease of reading and is often
   /// in chunks of 4 digits.
   ///
+  /// If the format of then length/radix-encoded string is not completely parsed
+  /// an exception will be thrown.  This can be caused by illegal characters
+  /// in the string or too long of a value string.
+  ///
   ///  Strings created by [toRadixString] are parsed by [ofRadixString].
   ///
   /// If the LogicValue width is not encoded as round number of radix
@@ -725,13 +740,21 @@ abstract class LogicValue implements Comparable<LogicValue> {
   ///  - 10'h2AA
   ///  - 11'h4AA
   ///  - 12'hAAA
-  static LogicValue ofRadixString(String valueString) {
+  static LogicValue ofRadixString(String valueString, {String sepChar = '_'}) {
+    if (radixStringChars.contains(sepChar)) {
+      throw LogicValueConstructionException('separation character invalid');
+    }
     if (RegExp(r'^\d+').firstMatch(valueString) != null) {
-      final formatStr = RegExp(r"^(\d+)'([bqodh])([0-9aAbBcCdDeEfFzZxX<> ]*)")
-          .firstMatch(valueString);
+      final formatStr =
+          RegExp("^(\\d+)'([bqodh])([0-9aAbBcCdDeEfFzZxX<>$sepChar]*)")
+              .firstMatch(valueString);
       if (formatStr != null) {
+        if (valueString.length != formatStr.group(0)!.length) {
+          throw LogicValueConstructionException('radix string stopped '
+              'parsing at character position ${formatStr.group(0)!.length}');
+        }
         final specifiedLength = int.parse(formatStr.group(1)!);
-        final compressedStr = formatStr.group(3)!.replaceAll(' ', '');
+        final compressedStr = formatStr.group(3)!.replaceAll(sepChar, '');
         // Extract radix
         final radixString = formatStr.group(2)!;
         final radix = switch (radixString) {
@@ -740,7 +763,8 @@ abstract class LogicValue implements Comparable<LogicValue> {
           'o' => 8,
           'd' => 10,
           'h' => 16,
-          _ => throw Exception('Unsupported radix: $radixString'),
+          _ => throw LogicValueConstructionException(
+              'Unsupported radix: $radixString'),
         };
         final span = (math.log(radix) / math.log(2)).ceil();
 
@@ -771,23 +795,30 @@ abstract class LogicValue implements Comparable<LogicValue> {
           while (cnt < binaryChunk.length - 1 && binaryChunk[cnt++] == '0') {}
           shorter = cnt - 1;
         } else {
-          final leadChar = compressedStr[0];
-          if (RegExp('[xXzZ]').hasMatch(leadChar)) {
-            shorter = span - 1;
-          } else {
-            if (radix == 10) {
-              shorter = binaryLength -
-                  BigInt.parse(compressedStr, radix: 10)
-                      .toRadixString(2)
-                      .length;
+          if (compressedStr.isNotEmpty) {
+            final leadChar = compressedStr[0];
+            if (RegExp('[xXzZ]').hasMatch(leadChar)) {
+              shorter = span - 1;
             } else {
-              shorter = span -
-                  BigInt.parse(leadChar, radix: radix).toRadixString(2).length;
+              if (radix == 10) {
+                shorter = binaryLength -
+                    BigInt.parse(compressedStr, radix: 10)
+                        .toRadixString(2)
+                        .length;
+              } else {
+                shorter = span -
+                    BigInt.parse(leadChar, radix: radix)
+                        .toRadixString(2)
+                        .length;
+              }
             }
+          } else {
+            shorter = 0;
           }
         }
         if (binaryLength - shorter > specifiedLength) {
-          throw Exception('ofRadixString: cannot represent '
+          throw LogicValueConstructionException(
+              'ofRadixString: cannot represent '
               '$compressedStr in $specifiedLength');
         }
         final noBinariesStr = reversedStr.replaceAll(fullBinaries, '0');
@@ -802,10 +833,15 @@ abstract class LogicValue implements Comparable<LogicValue> {
             .map((m) => List.generate(span, (s) => m.$2.start * span + s))
             .expand((ze) => ze);
 
-        final intValue = BigInt.parse(
-                _reverse(noBinariesStr.replaceAll(RegExp('[xXzZ]'), '0')),
-                radix: radix)
-            .toUnsigned(specifiedLength);
+        final BigInt intValue;
+        if (noBinariesStr.isNotEmpty) {
+          intValue = BigInt.parse(
+                  _reverse(noBinariesStr.replaceAll(RegExp('[xXzZ]'), '0')),
+                  radix: radix)
+              .toUnsigned(specifiedLength);
+        } else {
+          intValue = BigInt.zero;
+        }
         final logicValList = List<LogicValue>.from(
             LogicValue.ofString(intValue.toRadixString(2))
                 .zeroExtend(specifiedLength)
@@ -856,7 +892,8 @@ abstract class LogicValue implements Comparable<LogicValue> {
         }
         return logicValList.rswizzle();
       } else {
-        throw Exception('Invalid LogicValue string $valueString');
+        throw LogicValueConstructionException(
+            'Invalid LogicValue string $valueString');
       }
     }
     return LogicValue.zero;
@@ -918,7 +955,7 @@ abstract class LogicValue implements Comparable<LogicValue> {
 
   String _bitString() {
     if (width != 1) {
-      throw Exception(
+      throw LogicValueConversionException(
           'Cannot convert value of width $width to a single bit value.');
     }
     return this == LogicValue.x
@@ -1071,13 +1108,15 @@ abstract class LogicValue implements Comparable<LogicValue> {
 
   /// Converts a valid logical value to a boolean.
   ///
-  /// Throws an exception if the value is invalid.
+  /// Throws a LogicValueConversionException if the value is invalid.
   bool toBool() {
     if (!isValid) {
-      throw Exception('Cannot convert value "$this" to bool');
+      throw LogicValueConversionException(
+          'Cannot convert value "$this" to bool');
     }
     if (width != 1) {
-      throw Exception('Only single bit values can be converted to a bool,'
+      throw LogicValueConversionException(
+          'Only single bit values can be converted to a bool,'
           ' but found width $width in $this');
     }
     return this == LogicValue.one;


Test patch:
diff --git a/test/logic_value_test.dart b/test/logic_value_test.dart
index ba3fc796b..c8f94be3f 100644
--- a/test/logic_value_test.dart
+++ b/test/logic_value_test.dart
@@ -2067,8 +2067,8 @@ void main() {
     });
 
     test('radixString leading zero', () {
-      final lv = LogicValue.ofRadixString("10'b00 0010 0111");
-      expect(lv.toRadixString(), equals("10'b10 0111"));
+      final lv = LogicValue.ofRadixString("10'b00_0010_0111");
+      expect(lv.toRadixString(), equals("10'b10_0111"));
       expect(lv.toRadixString(radix: 4), equals("10'q213"));
       expect(lv.toRadixString(radix: 8), equals("10'o47"));
       expect(lv.toRadixString(radix: 10), equals("10'd39"));
@@ -2079,9 +2079,59 @@ void main() {
       }
     });
 
+    test('radixString round trip with alternate separation character', () {
+      final lv = LogicValue.ofRadixString("10'b00.0010.0111", sepChar: '.');
+
+      for (final i in [2, 4, 8, 10, 16]) {
+        expect(
+            LogicValue.ofRadixString(lv.toRadixString(radix: i, sepChar: '.'),
+                sepChar: '.'),
+            equals(lv));
+      }
+      try {
+        lv.toRadixString(sepChar: 'q');
+      } on Exception catch (e) {
+        expect(e, isA<LogicValueConversionException>());
+      }
+      try {
+        lv.toRadixString(radix: 14);
+      } on Exception catch (e) {
+        expect(e, isA<LogicValueConversionException>());
+      }
+    });
+    test('radixString space separators', () {
+      final lv = LogicValue.ofRadixString("10'b10 0010 0111", sepChar: ' ');
+      expect(lv.toInt(), equals(551));
+    });
+    test('radixString bad separator', () {
+      try {
+        LogicValue.ofRadixString("10'b10 0010_0111");
+      } on Exception catch (e) {
+        expect(e, isA<LogicValueConstructionException>());
+      }
+    });
+
+    test('radixString illegal separator', () {
+      try {
+        LogicValue.ofRadixString("10'b10q0010q0111", sepChar: 'q');
+      } on Exception catch (e) {
+        expect(e, isA<LogicValueConstructionException>());
+      }
+    });
+
+    test('radixString bad length', () {
+      try {
+        LogicValue.ofRadixString("10'b10_0010_0111_0000");
+      } on Exception catch (e) {
+        expect(e, isA<LogicValueConstructionException>());
+      }
+      // Try the shortest possible input
+      LogicValue.ofRadixString("10'b");
+    });
+
     test('radixString leading Z', () {
-      final lv = LogicValue.ofRadixString("10'bzz zzz1 1011");
-      expect(lv.toRadixString(), equals("10'bzz zzz1 1011"));
+      final lv = LogicValue.ofRadixString("10'bzz_zzz1_1011");
+      expect(lv.toRadixString(), equals("10'bzz_zzz1_1011"));
       expect(lv.toRadixString(radix: 4), equals("10'qZZ<z1>23"));
       expect(lv.toRadixString(radix: 8), equals("10'oZZ<z11>3"));
       expect(lv.toRadixString(radix: 16), equals("10'hZ<zzz1>b"));
@@ -2091,8 +2141,8 @@ void main() {
       }
     });
     test('radixString small leading radix character', () {
-      final lv = LogicValue.ofRadixString("10'b10 1010 0111");
-      expect(lv.toRadixString(radix: 4), equals("10'q2 2213"));
+      final lv = LogicValue.ofRadixString("10'b10_1010_0111");
+      expect(lv.toRadixString(radix: 4), equals("10'q2_2213"));
       expect(lv.toRadixString(radix: 8), equals("10'o1247"));
       expect(lv.toRadixString(radix: 10), equals("10'd679"));
       expect(lv.toRadixString(radix: 16), equals("10'h2A7"));