Skip to content
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

expose typeEquivalent #1402

Merged
merged 6 commits into from
Apr 10, 2020
Merged

expose typeEquivalent #1402

merged 6 commits into from
Apr 10, 2020

Conversation

sequencer
Copy link
Member

Related issue:

Type of change: other enhancement

Impact: API addition (no impact on existing code)

Development Phase: proposal

Release Notes
This PR expose typeEquivalent method to DataMirror for compile time reflection to Data Type, it can help chiseltester with more strict peek/poke constraint.

@sequencer sequencer requested a review from a team as a code owner April 7, 2020 05:57
@edwardcwang
Copy link
Contributor

edwardcwang commented Apr 7, 2020

Maybe add a unit test and scaladoc if this method is going to be exposed publicly?

core/src/main/scala/chisel3/Data.scala Outdated Show resolved Hide resolved
core/src/main/scala/chisel3/Data.scala Outdated Show resolved Hide resolved
core/src/main/scala/chisel3/Data.scala Outdated Show resolved Hide resolved
core/src/main/scala/chisel3/Data.scala Outdated Show resolved Hide resolved
sequencer and others added 2 commits April 10, 2020 06:11
Co-Authored-By: edwardcwang <edwardcwang@users.noreply.github.com>
Co-Authored-By: edwardcwang <edwardcwang@users.noreply.github.com>
@edwardcwang edwardcwang added the Squash and merge These commits don't matter, squash and merge label Apr 10, 2020
Copy link
Contributor

@edwardcwang edwardcwang left a comment

Choose a reason for hiding this comment

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

LGTM for me.

@edwardcwang
Copy link
Contributor

@ducky64 If you have no further objections, I'd like to get this merged.

Copy link
Contributor

@ducky64 ducky64 left a comment

Choose a reason for hiding this comment

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

lgtm - this looks good, thanks for writing up the scaladoc!

In the future, we might want to more thoroughly test this and its edge cases (particularly with unknown-width types), but it's experimental, and I'm not completely sure we've thought that much about these semantics (like how we haven't completely resolved connect semantics yet).

@edwardcwang
Copy link
Contributor

🎉🦆

@edwardcwang edwardcwang merged commit 2b71a50 into chipsalliance:master Apr 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Squash and merge These commits don't matter, squash and merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants