Repository: intel/rohd (dart)

Issue:
An `If.block` with only one `Else` in it does not flag an error
### Describe the bug

If an `If.block` has only one `Else` with nothing else, it will simulate and generate without error, even though the generated verilog looks like this (note missing `()`s).

ROHD:
```dart
    Combinational([
      If.block([
        Else([b < a])
      ])
    ]);
```

SystemVerilog:
```SystemVerilog
//  combinational
always_comb begin
  if begin
      b = a;
  end 

end
```

### To Reproduce

Construct a module such as this one:
```dart
class OnlyElseMod extends Module {
  OnlyElseMod(Logic a) {
    a = addInput('a', a);
    final b = addOutput('b');
    Combinational([
      If.block([
        Else([b < a])
      ])
    ]);
  }
}
```

### Expected behavior

Raise exception

### Actual behavior

No exception

### Additional: Dart SDK info

_No response_

### Additional: pubspec.yaml

_No response_

### Additional: Context

_No response_

Pull request:
<!-- Please make sure you check out the contribution guidelines before submitting a pull request! -->

## Description & Motivation

Fixes a set of bugs where `If`s can be constructed in an illegal way without an `Exception` being thrown.

## Related Issue(s)

Fix #382 

## Testing

Added tests for different wrong usages of `Else` in an `If.block`.

## Backwards-compatibility

> Is this a breaking change that will not be backwards-compatible? If yes, how so?

No

## Documentation

> Does the change require any updates to documentation? If so, where? Are they included?

Updated doc comment clarifying requirements.

Patch:
diff --git a/lib/src/exceptions/conditionals/conditional_exceptions.dart b/lib/src/exceptions/conditionals/conditional_exceptions.dart
index b03b403f5..1a550f4c4 100644
--- a/lib/src/exceptions/conditionals/conditional_exceptions.dart
+++ b/lib/src/exceptions/conditionals/conditional_exceptions.dart
@@ -1,6 +1,7 @@
 /// Copyright (C) 2022 Intel Corporation
 /// SPDX-License-Identifier: BSD-3-Clause
 
+export 'invalid_conditional_exception.dart';
 export 'mapped_signal_already_assigned_exception.dart';
 export 'signal_redriven_exception.dart';
 export 'uninitialized_signal_exception.dart';
diff --git a/lib/src/exceptions/conditionals/invalid_conditional_exception.dart b/lib/src/exceptions/conditionals/invalid_conditional_exception.dart
new file mode 100644
index 000000000..77bad9052
--- /dev/null
+++ b/lib/src/exceptions/conditionals/invalid_conditional_exception.dart
@@ -0,0 +1,19 @@
+// Copyright (C) 2023 Intel Corporation
+// SPDX-License-Identifier: BSD-3-Clause
+//
+// invalid_conditional_exception.dart
+// An exception thrown when a conditional is built in an invalid way.
+//
+// 2023 June 13
+// Author: Max Korbel <max.korbel@intel.com>
+
+import 'package:rohd/rohd.dart';
+import 'package:rohd/src/exceptions/exceptions.dart';
+
+/// An exception that is thrown when a [Conditional] has been constructed in
+/// an invalid way.
+class InvalidConditionalException extends RohdException {
+  /// Creates a new [InvalidConditionalException] with a [message] explaining
+  /// why the conditional was invalid.
+  InvalidConditionalException(super.message);
+}
diff --git a/lib/src/modules/conditional.dart b/lib/src/modules/conditional.dart
index 7db664d91..306121f81 100644
--- a/lib/src/modules/conditional.dart
+++ b/lib/src/modules/conditional.dart
@@ -1219,7 +1219,9 @@ class If extends Conditional {
   ///
   /// The first item should be an [Iff], and if an [Else] is included it must
   /// be the last item.  Any other items should be [ElseIf].  It is okay to
-  /// make thefirst item an [ElseIf], it will act just like an [Iff].
+  /// make the first item an [ElseIf], it will act just like an [Iff]. If an
+  /// [Else] is included, it cannot be the only element (it must be preceded
+  /// by an [Iff] or [ElseIf]).
   final List<Iff> iffs;
 
   /// If [condition] is high, then [then] executes, otherwise [orElse] is
@@ -1240,7 +1242,21 @@ class If extends Conditional {
 
   /// Checks the conditions for [iffs] in order and executes the first one
   /// whose condition is enabled.
-  If.block(this.iffs);
+  If.block(this.iffs) {
+    for (final iff in iffs) {
+      if (iff is Else) {
+        if (iff != iffs.last) {
+          throw InvalidConditionalException(
+              'Else must come last in an IfBlock.');
+        }
+
+        if (iff == iffs.first) {
+          throw InvalidConditionalException(
+              'Else cannot be the first in an IfBlock.');
+        }
+      }
+    }
+  }
 
   @override
   void execute(Set<Logic> drivenSignals, [void Function(Logic)? guard]) {
@@ -1318,9 +1334,6 @@ class If extends Conditional {
     final padding = Conditional.calcPadding(indent);
     final verilog = StringBuffer();
     for (final iff in iffs) {
-      if (iff is Else && iff != iffs.last) {
-        throw Exception('Else must come last in an IfBlock.');
-      }
       final header = iff == iffs.first
           ? 'if'
           : iff is Else


Test patch:
diff --git a/test/conditionals_test.dart b/test/conditionals_test.dart
index 2deba13e0..829c8ee0f 100644
--- a/test/conditionals_test.dart
+++ b/test/conditionals_test.dart
@@ -455,6 +455,35 @@ void main() {
         });
       }
     });
+
+    group('bad if blocks', () {
+      test('IfBlock with only else fails', () async {
+        expect(
+            () => If.block([
+                  Else([]),
+                ]),
+            throwsException);
+      });
+
+      test('IfBlock with else in the middle fails', () {
+        expect(
+            () => If.block([
+                  ElseIf(Logic(), []),
+                  Else([]),
+                  ElseIf(Logic(), []),
+                ]),
+            throwsException);
+      });
+
+      test('IfBlock with else at the start fails', () {
+        expect(
+            () => If.block([
+                  Else([]),
+                  ElseIf(Logic(), []),
+                ]),
+            throwsException);
+      });
+    });
   });
 
   group('simcompare', () {