ucb-bar / chisel2-deprecated

chisel.eecs.berkeley.edu
388 stars 90 forks source link

(Relevant to Enum:) Uppercase identifiers assumed to be stable identifiers in extractor/unapply context #694

Open sdtwigg opened 8 years ago

sdtwigg commented 8 years ago

Any time you use scala extractor syntax (sometimes called doing an unapply), identifiers that start with an uppercase letter are assumed to be stable (already bound to a value) identifiers. identifiers that do not are assumed to be unbound and thus able to be set as part of the extraction.

Looking specifically at the Enum case (which I'm going to unroll into a List of Int so you can play with these examples in the REPL more easily)...

val a :: b :: c :: d :: Nil = List(1, 2, 3, 4)

After this a == 1 && b == 2 && c == 3 && d == 4 as expected.

In contrast, the following will complain that A, B, C, and D values do not exist:

val A :: B :: C :: D :: Nil = List(1, 2, 3, 4)

scalac expected that something like this had happened:

val A = 1; val B = 2; val C = 3; val D = 4;
val A :: B :: C :: D :: x :: Nil = List(1, 2, 3, 4, 6)

where now the only new identifier is x == 6. (Note: val A :: B :: C :: D :: Nil = List(0, 0, 0, 0) will fail with a scala.MatchError)

The takeaway is that, this sort of code using Chisel will not compile as expected:

val STATE_A :: STATE_B :: STATE_C :: Nil = Enum(UInt(), 3)

since it expects already existing STATE_A, STATE_B, STATE_C identifiers.

The Enum documentation should be amended to explicitly note this issue (as Verilog developers are very, very used to using uppercase identifiers as constant/localparam definitions).

PS: Another other time that extractor/unapply syntax comes up is in match-case, e.g. the following is functionally identical to the above expression:

val x = List(1,2,3,4,6) match {case A :: B :: C :: D :: x :: Nil => x}

PPS: This also applies to the Enum in Chisel3