EDIFCellInst.equals() returns True for instances of different type in different parents

[cfib]

This might not be a bug, just a documentation issue but certainly was a pitfall for me:

The minimal netlist netlist.edf in the attached CellInsts.zip consists of a toplevel module that instantiates two modules lut_a_container and lut_b_container. These, in turn, instantiate a LUT1 and a LUT2, respectively. Each LUT is instantiated under the name lut_inst.

After running

lut_in_container_a = netlist.getCellInstFromHierName('lut_a_container_inst/lut_inst')
lut_in_container_b = netlist.getCellInstFromHierName('lut_b_container_inst/lut_inst')

I receive the following results (see rapidwright.py in the attached ZIP):

lut_in_container_a.equals(lut_in_container_b)
    True
lut_in_container_a == lut_in_container_b
    True
lut_in_container_a is lut_in_container_b
    False

To avoid this potential pitfall (these two instances may have the same EDIFName, but are different instances), IMO either the EDIFCellInst.equals() operator should take the parent cell into account or the documentation should reflect this behavior.

[clavin-xlnx]

This looks to be a bug, thank you for pointing this out. I agree, the cell type should most certainly be included in checking for equality among two instances of EDIFCellInst. I just surveyed the EDIF objects and how they implement equality as this may have not been fully examined previously:

EDIF Object	Equality check
EDIFCell	EDIFName.equals()
EDIFCellInst	EDIFName.equals()
EDIFDesign	EDIFName.equals()
EDIFHierCellInst	EDIFName.equals()
EDIFHierNet	EDIFNet.equals() && hierarchicalInstName.equals()
EDIFHierPortInst	EDIFPortInst.equals() && hierarchicalInstName.equals()
EDIFLibrary	EDIFName.equals()
EDIFNet	EDIFName.equals()
EDIFPort	name.equals() && EDIFCellInst.equals()
EDIFPortInst	EDIFName.equals()
EDIFPropertyObject	EDIFName.equals()
EDIFPropertyValue	Object.equals()

I recommend the following changes take place:

1. EDIFCellInst.equals() be implemented to include equality of its cell type as well as the name.
2. EDIFHierCellInst.equals() should be implemented to match behavior of EDIFHierNet and EDIFHierPortInst.
3. EDIFPropertyValue should have equality checked by both the type and value rather than defaulting to the Object.equals() behavior.
4. EDIFPropertyObject should also check the property sets match as well. This would impact most all other objects that can be decorated with properties.

One other idea that might be relevant is that based on circumstances, one might want a deepEquals() method that recursively checks all children objects of EDIFCell objects. These would be more expensive checks but could save a lot of code if it was used for doing netlist 'diff' type operations.

Could you share any insights into why you would like to check if cell instances are equal? Most things in a netlist are considered unique objects.

[clavin-xlnx]

My apologies, I just noticed you suggested the parent type of the EDIFCellInst should be checked. That information is more contextual of the EDIFCellInst and does not necessarily have bearing on what the EDIFCellInst is. But I do agree that the documentation should be made more clear on the subject in any case.