Add isOneOf method to ChiselEnum

[chiplet]

Contributor Checklist

• Did you add Scaladoc to every public function/method?
• Did you add at least one test demonstrating the PR?
• Did you delete any extraneous printlns/debugging code?
• Did you specify the type of improvement?
• Did you add appropriate documentation in docs/src?
• Did you state the API impact?
• Did you specify the code generation impact?
• Did you request a desired merge strategy?
• Did you add text to be included in the Release Notes for this change?

Type of Improvement

• new feature/API

API Impact

• Adds user-facing isOneOf method to ChiselEnum value type (EnumType).

Backend Code Generation Impact

• No impact

Desired Merge Strategy

• Squash: The PR will be squashed and merged (choose this if you have no preference.

Release Notes

• Added isOneOf method to ChiselEnum to reduce boilerplate when checking if the current value is in a set of given values.

Reviewer Checklist (only modified by reviewer)

• Did you add the appropriate labels?
• Did you mark the proper milestone (3.2.x, 3.3.x, 3.4.x, 3.5.0) ?
• Did you review?
• Did you check whether all relevant Contributor checkboxes have been checked?
• Did you mark as Please Merge?

[CLAassistant]

All committers have signed the CLA.

[ekiwi]

This is a great start and thanks for picking up this issue, @chiplet !

The next step would be to add some tests for the new method here: https://github.com/chipsalliance/chisel3/blob/master/src/test/scala/chiselTests/StrongEnum.scala

If you are using IntelliJ, you should see a green arrow next to each test that lets you run it. If you are using sbt directly, the sbt testOnly command can be used to execute individual tests. In you case something like: sbt testOnly chiselTests.StrongEnumSpec should execute all tests in the StrongEnumSpec class.

Please let me know if you have any questions. And please let me know if you need me to run the continuous integration tests. Github now requires us to give first-time contributors the OK to run CI tests. Apparently they had some problems with people mining bitcoins :(

[chiplet]

I've now added a test for the new method. The most use cases should be covered, but I'm not sure about how isOneOf should behave with the non increasing enum example found in the tests.

object NonIncreasingEnum extends ChiselEnum {
   val x = Value(2.U)
   val y = Value(2.U)
}

Should NonIncreasingEnum.x.isOneOf(NonIncreasingEnum.y) evaluate to true or false?

@ekiwi You could try running the CI tests now 👍

[chiplet]

When starting to develop the tests I forgot the stop() call which resulted in the tester creating around 1.5GB large dump.vcd file under test_run_dir for each run. Should there be e.g. some sort of a timeout to prevent this?

[mwachs5]

can you add some description of this to the website docs for ChiselEnum?

[ekiwi]

Should NonIncreasingEnum.x.isOneOf(NonIncreasingEnum.y) evaluate to true or false?

I think it should be true.

They way to think about this, is that x and y are aliases and thus it should be possible to use them interchangeably.

[chiplet]

Should NonIncreasingEnum.x.isOneOf(NonIncreasingEnum.y) evaluate to true or false?

I think it should be true.

They way to think about this, is that x and y are aliases and thus it should be possible to use them interchangeably.

Upon closer inspection there's a test that disallows instantiating non-increasing enums so NonIncreasingEnum.x.isOneOf(NonIncreasingEnum.y) doesn't even compile.

[chiplet]

can you add some description of this to the website docs for ChiselEnum?

You mean in this file /docs/src/explanations/chisel-enum.md right?

I think currently the ChiselEnum naming and organization are quite confusing. I've seen the names StrongEnum, ChiselEnum and EnumFactory floating around and it was very unclear to me where to find the documentation for ChiselEnum when I was starting to use it. For example, when trying to search for ChiselEnum the API docs (like this) there are currently no results. The same applies when searching for StrongEnum. EnumFactory is the only class that can be found from the API doc, but even it doesn't contain any scaladoc comments. Maybe it's worth opening a new issue regarding this.

[ekiwi]

Maybe it's worth opening a new issue regarding this.

I think the only thing that needs to be solved in this issue regarding documentation would be to add some simple javadoc comment to the two methods. (/** this method does ... */).

Besides that it would be great if you could rebase your commit on the latest master branch and resolve any conflicts.

Looking good!

[chiplet]

I've added the related documentation and rebased to latest upstream commit so AFAIK this should be ready to merge.

[ekiwi]

This is an awesome first contribution! 🚢

It will be auto-merged once the CI passes. Thanks!