amaranth icon indicating copy to clipboard operation
amaranth copied to clipboard

Make AST nodes immutable

Open whitequark opened this issue 1 year ago • 2 comments

Right now it's actually possible to do things like "set sig.width on a Signal" or "modify c.parts on a Cat". This has never been explicitly advertised as something that is supported and is arguably a misuse of the API. Amaranth internally treats AST nodes as immutable; all fragment transformers create new ones, and it doesn't mutate them elsewhere. Any mutation would happen only in downstream code (there's one instance deep in the FSM code generation, #1066).

Now that I'm working on reference documentation I'd like to either state that Values are immutable and make them so, or else document them as mutable. This could potentially break some downstream code though, so I'm nominating this for discussion.

If we decide that the nodes should be mutable, then setters must be introduced that typecheck the assignments.

whitequark avatar Jan 30 '24 23:01 whitequark

We have discussed this change on the 2024-02-12 weekly meeting. The decision was to make all Value hierarchy have immutable attributes.

This will only happen after we find a solution to #1066 though.

whitequark avatar Feb 12 '24 17:02 whitequark

This is now unblocked since #1066 is solved.

whitequark avatar Feb 27 '24 18:02 whitequark

This is completed by #1165, with the caveats that Signal.name and Value.src_loc/Statement.src_loc continue to be mutable.

whitequark avatar Feb 29 '24 18:02 whitequark