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

WireGroup's get_names for different scopes #64

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

yihuajack
Copy link
Collaborator

Previously get_name will regard all wires with the same name as the same wires, which is an oversimplification. Now, wire names are associated with their wire groups.

Copy link
Owner

@Ben1152000 Ben1152000 left a comment

Choose a reason for hiding this comment

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

Can you check that the modified implementation of get_names works when __main__.py:169 is run? It seems to return a dict tree when it should return a flat list of names.

yihuajack added a commit to yihuajack/sootty that referenced this pull request Sep 11, 2022
@yihuajack
Copy link
Collaborator Author

Can you check that the modified implementation of get_names works when __main__.py:169 is run? It seems to return a dict tree when it should return a flat list of names.

It is designed to return a dict because we want to construct a dict whose keys are scopes and values are sub-scopes or wire names.

I modified this function because a dict is needed for print_breakpoints, so get_wires returns a dict. To be consistent, get_names also returns a dict. The only reference of get_names currently is at Line 169 of __main__.py to raise an exception.

Therefore, currently the behavior of get_names does not make a difference. However, as mentioned In my report, if a better formatting of print_breakpoints is expected, get_names have to return a dict to distinguish different wires with the same names in different scopes.

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.

None yet

2 participants