Chisel chokes on using elements of an unbound Aggregate as elements of a Record

[jackkoenig]

Type of issue: Bug Report

Please provide the steps to reproduce the problem:

Consider the following Chisel:

//> using scala "2.13.12"
//> using dep "org.chipsalliance::chisel:6.4.0"
//> using plugin "org.chipsalliance:::chisel-plugin:6.4.0"
//> using options "-unchecked", "-deprecation", "-language:reflectiveCalls", "-feature", "-Xcheckinit", "-Xfatal-warnings", "-Ywarn-dead-code", "-Ywarn-unused", "-Ymacro-annotations"

import chisel3._
// _root_ disambiguates from package chisel3.util.circt if user imports chisel3.util._
import _root_.circt.stage.ChiselStage
import scala.collection.immutable.VectorMap

class MyRecord extends Record {
  val a = UInt(8.W)
  val b = Vec(2, UInt(8.W))
  val elements = VectorMap("a" -> a) ++ b.zipWithIndex.map { case (e, i) => i.toString -> e }
}

class Foo extends Module {
  val in = IO(Input(new MyRecord))
  val out = IO(Output(new MyRecord))

  out := in
}

object Main extends App {
  println(
    ChiselStage.emitCHIRRTL(new Foo)
  )
}

What is the current behavior?

This emits:

circuit Foo :
  module Foo :
    input clock : Clock
    input reset : UInt<1>
    input in : { [ILit(1)] : UInt<8>, [ILit(0)] : UInt<8>, a : UInt<8>}
    output out : { [ILit(1)] : UInt<8>, [ILit(0)] : UInt<8>, a : UInt<8>}

    connect out, in

The input in : { [ILit(1)] : UInt<8>, [ILit(0)] : UInt<8>, a : UInt<8>} is nonsensical.

What is the expected behavior?

It should either error or emit valid FIRRTL

Note there are other ways that this same bug can manifest. Trying to dontTouch in or out throws an exception pointing to Chisel internals.

Please tell us about your environment:

Other Information

What is the use case for changing the behavior?

[mwachs5]

is this only for Vecs or does it manifest for other Bundles and Record you might try to do this to

[jackkoenig]

is this only for Vecs or does it manifest for other Bundles and Record you might try to do this to

I assumed it would apply to all Aggregates, but it seems to be only for Vecs. The issue is that the ref for the children of Vecs is unconditionally pointing to the Vec within which they are defined[1] (before binding), whereas for Records it is set during binding[2].

Regardless, I think we should ban this because it's a pretty sharp edge even when it "works". The user may reasonably see b as a thing that they can use, connect to it to just connect to those fields. They will expect it to act like a view, but it will not. Things will work if you just iterate on the elements of b, but will fail if you try to do anything with b itself.