schoeberl / chisel-book

Digital Design with Chisel
727 stars 135 forks source link

Add missing period in when{}.otherwise{} snippets #61

Closed valdaarhun closed 6 months ago

valdaarhun commented 6 months ago

In some places, the when{}.otherwise{} chain is missing a period. This PR adds the missing period in those snippets.

schoeberl commented 6 months ago

The dot is usually not needed. Why add it?

valdaarhun commented 6 months ago

Thank you for the prompt reply.

The dot is usually not needed. Why add it?

Yes, you're right. I tinkered around with a few examples and it works without the period.

At the time of opening this PR, there was some confusion on my part. I had gone through chisel's documentation and tried hunting for resources (such as these slides) where the dot isn't used but I wasn't able to find any example that uses this construct without the dot.

I guess after reading this line in the book, I incorrectly assumed that the statement must be true for the otherwise part as well. There are a few other examples (such as Combinational.scala) in the book that use the dot for the otherwise part as well. That added to the confusion.

I'll close this PR.

schoeberl commented 6 months ago

Thank you for pointing this out. Yes, I should be more consistent in the book. I will fix this. The issue is that it usually works without the '.' and therefore looks better. However, there are cases where the Scala compiler cannot infer that dot.

valdaarhun commented 6 months ago

The issue is that it usually works without the '.' and therefore looks better.

Right.

However, there are cases where the Scala compiler cannot infer that dot.

I see. I wonder why that is. Could it be a bug in Chisel? If it fails to infer the dot in some cases, is it safer to omit the dot?

schoeberl commented 6 months ago

It is not a bug in Chisel, it is a Scala feature that one can use methods as infix notation.

valdaarhun commented 5 months ago

Got it. It makes sense to me now. Thanks!