circt
circt copied to clipboard
[HW][NFCI] Prevent null attributes in per_port_attrs array
I bumped into a problem using the new setters for HWModuleOp port attributes. Using the setPortAttrs(size_t idx, DictionaryAttr attr) method on a previously empty attribute array can insert a (non-empty) DictionaryAttribute into the array, which is elsewhere populated with null attributes. This causes a violation of the OptionalAttr<DictArrayAttr> constraint on per_port_attrs (see the added test). The most straight-forward fix could see was to return empty DictionaryAttributes instead of null attributes in the getAllPortAttrs method.
However, I am having a hard time figuring out intent here. Some of the calls to the getters explicitly check for null, others don't. Is there a reason to keep the null attributes despite the constraint? If not, I guess we could avoid the null checks and let the getters explicitly return SmallVector<DictionaryAttr>.
Some of the calls to the getters explicitly check for null, others don't. Is there a reason to keep the null attributes despite the constraint?
Different attribute arrays evolved from different constraints, unfortunately. We need to make a call and stick to it. There is code simplification in always having an array of the correct length width (dropping the optional), there is supper slight representational goodness in "empty" being actually null, rather than having to check. If the array always exists, can it be zero length? can the interrior dictionaries be null, or must they be extant and empty?
I have rewritten this to reflect the changes of #6531. Instead of padding the vector in the getters it is now done in the setters.