Skip to content

Commit

Permalink
[CI] Use Mill for testing (#4466)
Browse files Browse the repository at this point in the history
* Fix scalacOptions for Chisel to use plugin
* Improve error reporting to support and suggest -Xcheckinit
  • Loading branch information
jackkoenig authored Oct 15, 2024
1 parent 7dabc73 commit e20cb96
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 17 deletions.
15 changes: 8 additions & 7 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,10 @@ jobs:
$CHISEL_FIRTOOL_PATH/firtool -version >> $GITHUB_STEP_SUMMARY
echo \`\`\` >> $GITHUB_STEP_SUMMARY
- name: Test
run: sbt ++${{ inputs.scala }} test
run: |
./mill firrtl[_].test
./mill svsim[_].test
./mill chisel[_].test
- name: Binary compatibility
# TODO either make this also check the plugin or decide that we don't
# support binary compatibility for the plugin
Expand Down Expand Up @@ -155,7 +158,6 @@ jobs:
with:
distribution: 'adopt'
java-version: '11'
cache: 'sbt'
- name: Install CIRCT
id: install-circt
if: ${{ inputs.circt }}
Expand All @@ -170,7 +172,7 @@ jobs:
dir=$(dirname $(which firtool))
echo "CHISEL_FIRTOOL_PATH=$dir" >> "$GITHUB_ENV"
- name: Integration Tests
run: sbt integrationTests/test
run: ./mill integrationTests[_].test

# Currently just a sanity check that the benchmarking flow works
benchmark:
Expand All @@ -190,7 +192,7 @@ jobs:
run: ./mill benchmark.runJmh

std:
name: Standard Library Tests
name: Standard Library
runs-on: ubuntu-22.04
strategy:
matrix:
Expand All @@ -207,9 +209,8 @@ jobs:
with:
distribution: 'adopt'
java-version: '11'
cache: 'sbt'
- name: Unit Tests
run: sbt ++${{ inputs.scala }} standardLibrary/test
- name: Compile
run: ./mill stdlib[_].compile

website:
name: Build Mdoc & Website
Expand Down
3 changes: 2 additions & 1 deletion build.sc
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,8 @@ object chisel extends Cross[Chisel](v.scalaCrossVersions)
trait Chisel extends common.ChiselModule with CrossSbtModule with ScalafmtModule {
override def millSourcePath = super.millSourcePath / os.up

override def scalacOptions = v.commonOptions
// Make sure to include super.scalacOptions for Chisel Plugin
override def scalacOptions = T(super.scalacOptions() ++ v.commonOptions)

def svsimModule = svsim(crossScalaVersion)

Expand Down
14 changes: 9 additions & 5 deletions core/src/main/scala/chisel3/ModuleImpl.scala
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,12 @@ package experimental {
def desiredName: String = simpleClassName(this.getClass)

/** Legalized name of this module. */
final lazy val name =
final lazy val name = {
def err(msg: String, cause: Throwable = null) = {
val msg = s"Error: desiredName of ${this.getClass.getName} is null. "
s"Did you evaluate 'name' before all values needed by desiredName were available? $msg."
throwException(msg, cause)
}
try {
// PseudoModules are not "true modules" and thus should share
// their original modules names without uniquification
Expand All @@ -671,12 +676,11 @@ package experimental {
}
} catch {
case e: NullPointerException =>
throwException(
s"Error: desiredName of ${this.getClass.getName} is null. Did you evaluate 'name' before all values needed by desiredName were available?",
e
)
err("Try adding -Xcheckinit to your scalacOptions to get a more useful stack trace", e)
case UninitializedFieldError(msg) => err(msg)
case t: Throwable => throw t
}
}

/** Returns a FIRRTL ModuleName that references this object
*
Expand Down
18 changes: 14 additions & 4 deletions core/src/main/scala/chisel3/internal/Builder.scala
Original file line number Diff line number Diff line change
Expand Up @@ -813,13 +813,23 @@ private[chisel3] object Builder extends LazyLogging {

// Helper for reasonable errors when clock or reset value not yet initialized
private def getDelayed[A](field: String, dc: Delayed[A]): A = {
val result = dc.value
if (result == null) {
// TODO add SourceInfo and change to Builder.exception
// TODO add SourceInfo and change to Builder.exception.
def err(extra: String) = {
throwException(
s"The implicit $field is null which means the code that sets its definition has not yet executed."
s"The implicit $field is null which means the code that sets its definition has not yet executed. $extra."
)
}
val result =
try {
dc.value
} catch {
// If user uses -Xcheckinit there will be additional information.
case UninitializedFieldError(msg) => err(msg)
}
// If user is not using -Xcheckinit, suggest that they do.
if (result == null) {
err("Try adding -Xcheckinit to your scalacOptions to get a more useful stack trace")
}
result
}

Expand Down

0 comments on commit e20cb96

Please sign in to comment.