Skip to content

Conversation

@jackkoenig
Copy link
Contributor

Previously, we tried to avoid this by deprecating any imports from the firrtl package, but this missed things. Instead, we deprecate every public class, object, and trait.

Also delete the previous attempt using the DeprecateSFCComponent in the compiler plugin. It is now redundant.

I tried to write a [POSIX] sed script to do this but I was really struggling with having it take any number of abstract|final|case|sealed so instead I asked an AI and it generated a stupid but effective way to do it:

deprecate.sed:

/^sealed abstract class/i \
@deprecated("All APIs in package firrtl are deprecated.", "Chisel 7.0.0")

/^abstract class/i \
@deprecated("All APIs in package firrtl are deprecated.", "Chisel 7.0.0")

/^class/i \
@deprecated("All APIs in package firrtl are deprecated.", "Chisel 7.0.0")

/^final class/i \
@deprecated("All APIs in package firrtl are deprecated.", "Chisel 7.0.0")

/^case class/i \
@deprecated("All APIs in package firrtl are deprecated.", "Chisel 7.0.0")

/^final case class/i \
@deprecated("All APIs in package firrtl are deprecated.", "Chisel 7.0.0")

/^trait/i \
@deprecated("All APIs in package firrtl are deprecated.", "Chisel 7.0.0")

/^sealed trait/i \
@deprecated("All APIs in package firrtl are deprecated.", "Chisel 7.0.0")

/^object/i \
@deprecated("All APIs in package firrtl are deprecated.", "Chisel 7.0.0")

/^case object/i \
@deprecated("All APIs in package firrtl are deprecated.", "Chisel 7.0.0")
find firrtl/src/main/scala -name '*.scala' | xargs sed -i '' -f deprecate.sed

Naturally the initial version missed a bunch of things so I expanded it till it seemed to work.

This does not get things inside of package objects, and I don't think you can deprecate them, so instead I handled those manually, it was only like 6 things.

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 deprecation

Desired Merge Strategy

  • Squash

Release Notes

If there is anything you cannot do without accessing APIs in package firrtl directly, please open an issue on the Chisel Github repository to explain your use case so we can see about adding APIs to Chisel directly.

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.6.x, 5.x, or 6.x depending on impact, API modification or big change: 7.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) and clean up the commit message.
    • Merge: Ensure that contributor has cleaned up their commit history, then merge with Create a merge commit.

Previously, we tried to avoid this by deprecating any imports from the
firrtl package, but this missed things. Instead, we deprecate every
public class, object, and trait.

Also delete the previous attempt using the DeprecateSFCComponent in the
compiler plugin. It is now redundant.
@jackkoenig jackkoenig requested a review from seldridge April 9, 2025 18:47
@jackkoenig jackkoenig added the Deprecation Deprecates an API, will be included in release notes label Apr 9, 2025
@jackkoenig jackkoenig added this to the 7.0 milestone Apr 9, 2025
Copy link
Member

@seldridge seldridge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Nice to see the compiler plugin go.

@jackkoenig jackkoenig merged commit 990d37a into main Apr 9, 2025
17 of 18 checks passed
@jackkoenig jackkoenig deleted the jackkoenig/deprecate-firrtl-redux branch April 9, 2025 20:17
tymcauley added a commit to tymcauley/fixedpoint that referenced this pull request Apr 18, 2025
This update includes this Chisel PR, which deprecated everything in the
`firrtl` package: chipsalliance/chisel#4878

Bump `firtool` version to latest as well.
milovanovic pushed a commit to ucb-bar/fixedpoint that referenced this pull request Dec 15, 2025
* Update Chisel from 6.5.0 to 7.0.0

`UnknownWidth` became a `case object` in this Chisel PR:
chipsalliance/chisel#4242

The `connectFromBits` method was removed in this Chisel PR:
chipsalliance/chisel#4168

The `connectFromBits` API was replaced with the `_fromUInt` API in this
Chisel PR: chipsalliance/chisel#4782

Wrapping unary negation (`unary_-%`) was deprecated in this PR:
chipsalliance/chisel#4829

The sbt update is necessary to maintain compatibility with the latest
Scala compiler version.

* Migrate from Chisel testers to ChiselSim

The `chisel3.testers` package was removed here:
chipsalliance/chisel#4746

It doesn't look like ChiselSim has the same `Boolean` result from
`simulate`, so we must check for simulation errors by catching
exceptions.

* Replace ChiselRunners trait with direct use of ChiselSim

* Update to latest scalafmt

* lint: Apply latest scalafmt settings

* Update to latest scalatest version

* Fix missing sbt command in GitHub CI

For background: actions/setup-java#712

Also updated JDK to 21 (latest LTS), updated some GitHub actions to
their latest versions, and using install-circt GitHub action to get
firtool.

* Fix out-of-range literals in tests

An 8-bit signed integer with a binary point at 2 effectively has a 6-bit
signed number for the value to the left of the decimal point. That means
it can represent values in the range [-2^5, 2^5-1], or [-32, 31]. After
chipsalliance/chisel#4786 was merged, the `55`
and `56` literals are now correctly flagged as being out-of-range for
this type.

I've subtracted 32 from these out-of-range values (changing the MSB from
1 to 0), and the tests now pass.

* Update Chisel 7 snapshot from 639-5df5515f to 678-dca5fc11

This update includes this Chisel PR, which deprecated everything in the
`firrtl` package: chipsalliance/chisel#4878

Bump `firtool` version to latest as well.

* Update Chisel version to 7.0.0-RC1

Also update CI versions of firtool and verilator to latest.

* Update Chisel version to 7.5.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Deprecation Deprecates an API, will be included in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants