search

PyHDI / Pyverilog

Python-based Hardware Design Processing Toolkit for Verilog HDL
Apache License 2.0
619 stars 173 forks source link

ast.IntConst creates collisions on dicts #84

Open Lucaz97 opened 3 years ago

Lucaz97 commented 3 years ago

The parent class ast,Node defines the eq and hash methods in a way that with two ast.IntConst objects with the same value a collision occurs when using such objects as keys in a dict.

Such methods should guarantee that such thing does not occur. I fixed the problem locally by changing the following line https://github.com/PyHDI/Pyverilog/blob/5847539a9d4178a521afe66dbe2b1a1cf36304f3/pyverilog/vparser/ast.py#L246 with attr_names = ('value','lineno')

If you'd like I can do a pull request.

shtaxxx commented 3 years ago

Thanks for the nice suggestion. Could you show us what kind of adverse effects are occurred by the current implementation? Furthermore, I wonder just adding "lineno" is sufficient.

Lucaz97 commented 3 years ago

@shtaxxx Thank you for your reply.

Let's suppose I am parsing this FIR filter implementation: https://github.com/mit-ll/CEP/blob/master/hdl_cores/dsp/FIR_filter.v I have a dictionary that uses as a key some ast nodes, some of which are ast.Constant. In the design above there are some constants of the same values (for example line 74 and 85 both have a constant of value 3). With the current implementation I get collisions on the dict and I only detect the last constant for each value present in the design (I lose the 3 at line 74).
Adding lineno to the attr allows for differentiating those cases.

It is true that if I have a line like:

something = c1 + c2 +c3 +c4 ....

with c1 ... cN constants of the same values, they would still collide. Apparently I did not have any since I only thought about it now and my tests are passing.