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

Add autoinst_multidim_rename test and fix for issue #1848 #1850

Closed
wants to merge 2 commits into from

Conversation

techdude
Copy link

@techdude techdude commented Dec 8, 2023

I had some more time to look into this, and I have both a test case and a fix for the issue. The summary of the changes are as follows:

  • Set vl-bits without the logic that prevents displaying it in certain circumstances. This ensures it has the correct value when used in the multi-dim hint comment, as well as ensuring it will have the correct value if the user uses it directly in an @"(...)" expression.
  • Added variable auto-inst-vector for the auto-inst-vector component that can be masked if verilog-auto-inst-vector isn't t or 'unsigned and the net is signed, or if the port name matches one of the moddecls names (typically wires or outputs on the parent module) and the dimensions match. This preserves the original behavior expected by verilog-auto-inst-vector.
  • In addition, if a port is connected to a different net (via an AUTO_TEMPLATE), then the name after renaming is used for the check (result is in auto-inst-vector-tpl). This ensures that AUTOWIRE/AUTOINPUT/AUTOOUTPUT works correctly for connections within a block, but it matches the expected behavior if the output port/wire already exists.

verilog-mode.el Show resolved Hide resolved
wsnyder pushed a commit that referenced this pull request Dec 14, 2023
* verilog-mode.el (verilog-auto-inst-port): Fix AUTOINST
multi-dimensional array [] substitution.  Reported by Caleb Begly.
@wsnyder
Copy link
Member

wsnyder commented Dec 14, 2023

Thanks - merged manually to fix some spacing issues.

@wsnyder wsnyder closed this Dec 14, 2023
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.

2 participants