-
Notifications
You must be signed in to change notification settings - Fork 647
[Scala3][WIP] Scala3 testing I #5083
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
e68ee03
c25938a
9ecf4c7
26f4642
8da5e1d
9e1153e
6475b20
ac62454
d42f190
c3dec70
49a7fbf
c60d031
f12ce6d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,7 @@ import chisel3.layer.{block, Convention, Layer, LayerConfig} | |
| import chisel3.simulator._ | ||
| import org.scalatest.funspec.AnyFunSpec | ||
| import org.scalatest.matchers.must.Matchers | ||
| import org.scalatest.matchers.should.Matchers.convertToAnyShouldWrapper | ||
| // import org.scalatest.matchers.should.Matchers.convertToStringShouldWrapperForVerb | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: please don't commit commented out code.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah this is a tricky one. the scalatest API is subtly different between Scala 2 and 3 leading to an either-or situation as to which
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please check that ScalaTest doesn't already have a way to deal with this--it would be pretty bush league for such a popular library to screw this up. If they dont then we can polyfill it ourselves by adding a type alias for one or the other in the version-specific sources (e.g. in
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This still needs resolution |
||
| import svsim._ | ||
|
|
||
| class VerilatorSimulator(val workspacePath: String) extends Simulator[verilator.Backend] { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -123,12 +123,12 @@ object ChiselStageSpec { | |
| } | ||
|
|
||
| class RecoverableErrorFakeSourceInfo extends RawModule { | ||
| implicit val info = SourceLine("Foo", 3, 10) | ||
| implicit val info: SourceLine = SourceLine("Foo", 3, 10) | ||
| 3.U >> -1 | ||
| } | ||
|
|
||
| class ErrorCaughtByFirtool extends RawModule { | ||
| implicit val info = SourceLine("Foo", 3, 10) | ||
| implicit val info: SourceLine = SourceLine("Foo", 3, 10) | ||
|
Comment on lines
+126
to
+131
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These should be the more generic type |
||
| val w = Wire(UInt(8.W)) | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,7 +20,7 @@ class AdderTreeTester(bitWidth: Int, numsToAdd: List[Int]) extends Module { | |
| val dut = Module(new AdderTree(genType, numsToAdd.size)) | ||
| dut.io.numIn := VecInit(numsToAdd.map(x => x.asUInt(bitWidth.W))) | ||
| val sumCorrect = dut.io.numOut === (numsToAdd.reduce(_ + _) % (1 << bitWidth)).asUInt(bitWidth.W) | ||
| assert(sumCorrect) | ||
| chisel3.assert(sumCorrect) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this necessary? |
||
| stop() | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -374,7 +374,7 @@ class BlackBoxSpec extends AnyFlatSpec with Matchers with ChiselSim with FileChe | |
| object A extends layer.Layer(layer.LayerConfig.Extract()) | ||
|
|
||
| sealed trait NoIo { this: BlackBox @nowarn("cat=deprecation") => | ||
| final val io = IO(new Bundle {}) | ||
| final val io = chisel3.IO(new Bundle {}) | ||
| } | ||
|
|
||
| // No known layers | ||
|
|
@@ -419,7 +419,7 @@ class BlackBoxSpec extends AnyFlatSpec with Matchers with ChiselSim with FileChe | |
| class Bar extends BlackBox { | ||
| final val io = IO { | ||
| new Bundle { | ||
| val a = Output(probe.Probe(Bool(), layers.Verification)) | ||
| val a = Output(probe.Probe(Bool(), chisel3.layers.Verification)) | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -430,7 +430,7 @@ class BlackBoxSpec extends AnyFlatSpec with Matchers with ChiselSim with FileChe | |
| class Baz extends BlackBox(knownLayers = Seq(A)) { | ||
| final val io = IO { | ||
| new Bundle { | ||
| val a = Output(probe.Probe(Bool(), layers.Verification)) | ||
| val a = Output(probe.Probe(Bool(), chisel3.layers.Verification)) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm confused by why these are needed |
||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -75,7 +75,7 @@ class BulkConnectSpec extends AnyPropSpec with Matchers { | |
| val io: MyBundle = IO(Flipped(new MyBundle)) | ||
|
|
||
| @nowarn("cat=deprecation") | ||
| val bb = Module(new BlackBox { | ||
| val bb = Module[BlackBox { def io: MyBundle }](new BlackBox { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should not be necessary due to Selectable right? Or did we decide not to apply that to modules? |
||
| val io: MyBundle = IO(Flipped(new MyBundle)) | ||
| }) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,8 +34,8 @@ class ModuleWithChoice[T <: Data]( | |
| default: => FixedIOBaseModule[T] | ||
| )(alternateImpls: Seq[(Case, () => FixedIOBaseModule[T])]) | ||
| extends Module { | ||
| val inst = ModuleChoice(default)(alternateImpls) | ||
| val io = IO(inst.cloneType) | ||
| val inst: T = ModuleChoice[T](default, alternateImpls) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is wrong, the API in |
||
| val io: T = IO(chiselTypeOf(inst)) | ||
| io <> inst | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,7 +33,7 @@ class SimLogSpec extends AnyFlatSpec with Matchers with FileCheck with ChiselSim | |
| class MyModule extends Module { | ||
| val in = IO(Input(UInt(8.W))) | ||
| val fd = SimLog.file("logfile.log") | ||
| fd.printf("in = %d\n", in) | ||
| // fd.printf("in = %d\n", in) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this shouldn't be commented out? |
||
| } | ||
| ChiselStage | ||
| .emitCHIRRTL(new MyModule) | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm surprised this file has so many lines added. Can you clarify what is going on? |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,6 @@ | |
| package chiselTests.naming | ||
|
|
||
| import chisel3._ | ||
| import chisel3.aop.Select | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If these aren't used, I'm all for deleting (and we should eventually enable scalafix to do it for us), but why did this need to be deleted to run the test with Scala 3? |
||
| import chisel3.experimental.{noPrefix, prefix, skipPrefix, AffectsChiselPrefix} | ||
| import chisel3.testing.scalatest.FileCheck | ||
| import circt.stage.ChiselStage | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better for both of these would be:
It's not that this is fundamentally wrong, but people do look at the tests to get inspiration or see best practice sometimes and this cast is not good practice.