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

Revert "Make uselessly public fields in utils private" #1417

Merged
merged 1 commit into from
Apr 17, 2020

Conversation

azidar
Copy link
Contributor

@azidar azidar commented Apr 16, 2020

At the bequest of @jackkoenig, this reverts commit c279860.

Type of change: other enhancement

Impact: API addition (no impact on existing code)

Development Phase: implementation

Release Notes

To properly name private and protected fields in chisel3.util.Arbiter and chisel3.util.Decoupled with the @chiselName macro annotation, these fields were made public, and thus retain a better name.

@azidar azidar requested a review from a team as a code owner April 16, 2020 23:52
Copy link
Contributor

@jackkoenig jackkoenig left a comment

Choose a reason for hiding this comment

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

For historical context, see #407

The private vals were named long ago, but at some point that broke. I think it's worth considering addressing #1410 at some point, but the more important goal is having these fields named in the Verilog.

@jackkoenig
Copy link
Contributor

Should we backport this?

@ucbjrl
Copy link
Contributor

ucbjrl commented Apr 17, 2020

Re: backporting - is there any chance previously private names may now collide with user/client definitions? I would hope not, but I vaguely remember that accessibility (private vs public) did interfere with visibility (inheritance uniqueness), despite the hope that it wouldn't.

@jackkoenig
Copy link
Contributor

Re: backporting - is there any chance previously private names may now collide with user/client definitions?

Good point, yeah this would fail the binary incompatibility check so we shouldn't backport. 3.3 will be out soon so we'll just tell people to upgrade to get the fix.

@jackkoenig jackkoenig merged commit cf03b0d into master Apr 17, 2020
@jackkoenig jackkoenig added this to the 3.3.0 milestone Apr 17, 2020
@jackkoenig jackkoenig deleted the remove-private-vals branch July 7, 2021 03:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants