MixedVec subclasses immutable.IndexedSeq

[mwachs5]

In the change to scala 2.13, collection.IndexedSeq is not enough to be considered a Seq

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 request a desired merge strategy?
• Did you add text to be included in the Release Notes for this change?

Type of Improvement

• API modification

Desired Merge Strategy

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

Release Notes

Change MixedVec to inherit from collection.immutable.IndexedSeq, so that it can work for Seq[Data].

Reviewer Checklist (only modified by reviewer)

• Did you add the appropriate labels? (Select the most appropriate one based on the "Type of Improvement")
• Did you mark the proper milestone (Bug fix: 3.5.x, 3.6.x, or 5.x depending on impact, API modification or big change: 6.0)?
• Did you review?
• Did you check whether all relevant Contributor checkboxes have been checked?
• Did you do one of the following when ready to merge:
  • Squash: You/ the contributor Enable auto-merge (squash), clean up the commit message, and label with Please Merge.
  • Merge: Ensure that contributor has cleaned up their commit history, then merge with Create a merge commit.

[jackkoenig]

@mwachs5 an integration test failed. Annoying that this change technically can break some code but the fix is usually trivial, you should just be able to remove the .toSeq (might also need to call splat :_*).

[mwachs5]

you should just be able to remove the .toSeq (might also need to call splat :_*).

Interestingly neither of these work. The first gives me:

[error] /scratch/megan/chisel/integration-tests/src/test/scala/chiselTest/MixedVecIntegrationSpec.scala:86:17: ambiguous reference to overloaded definition,
[error] both macro method apply in object VecInit of type [T <: chisel3.Data](elt0: T, elts: T*): chisel3.Vec[T]
[error] and  macro method apply in object VecInit of type [T <: chisel3.Data](elts: Seq[T]): chisel3.Vec[T]
[error] match argument types (chisel3.util.MixedVec[chisel3.UInt])
[error]   val vecWire = VecInit(wire)

The second gives me:

error] /scratch/megan/chisel/integration-tests/src/test/scala/chiselTest/MixedVecIntegrationSpec.scala:86:29: Sequence argument type annotation `: _*` cannot be used here:
[error] the corresponding parameter has type chisel3.UInt which is not a repeated parameter type
[error]   val vecWire = VecInit(wire: _*)

I have confirmed that wire is of type MixedVec[UInt] ... what am I missing here?

This also does not work:

 val vecWire = VecInit[UInt](wire)

This does work:

  val vecWire = VecInit(wire : Seq[UInt])

It's as if a MixedVec is also seen as a UInt? Not sure why that is the case...

[mwachs5]

Actually, what is this test even attempting to show? It doesn't seem like it should be legal to have a VecInit of a MixedVec with different element sizes. Shouldn't VecInit be checking that all elements have the same chisel type?

I guess not, it is legal as the scaladoc says:

  /** Creates a new [[Vec]] composed of elements of the input Seq of [[Data]]
    * nodes.
    *
    * @note input elements should be of the same type (this is checked at the
    * FIRRTL level, but not at the Scala / Chisel level)
    * @note the width of all output elements is the width of the largest input
    * element
    * @note output elements are connected from the input elements
    */

[mwachs5]

I guess this is just a limitation for VectInit for anything that is both a Seq and Data. Vecs have the same problem (do you want a Vec of Vecs-with-one-element or a Vec of the elements of the Vecs...)

Should VecInit force you to have at least 2 elements to use the :_* version