`with` statements should have weak references to combinational groups

[rachitnigam]

trafficstars

I ran into a bug where even though a combinational group had been detached from a component (using drain on component.comb_groups), it was retained in memory by an invoke-with statement. Probably the right thing here would be for with statements to carry weak references to combinational groups and have the ownership of the combinational group associated with only the component's field.

[sampsyo]

Sounds good. I am contractually obligated to also mention that flattening (#1183 et al.) would address this problem too by tying all AST nodes' lifetimes to a single lifetime for the whole program.

[rachitnigam]

Good point! However, in this case, I want to turn things into weak references to explicitly model the fact that they are invalid. I don't think that even in a flat representation, you truly want everything to have the lifetime of a program or even a component.

Building from your blog post, one trick to play here is using the upper bit of the index for a flat representation to denote whether something is valid or not and using that to perform this kind of invalidation

[sampsyo]

Ah yes, you are very wise—integers-as-pointers wouldn't itself solve the problem of "notifying" the invoke AST node that the comb group it refers to is gone. Good idea to play tricks with the reference integer itself to denote validity.

[rachitnigam]

Hm, this change would also imply that we should carry WRCs for Group and StaticGroup in Enable and StaticEnable which would be a pretty dramatic change. However, I think the overall idea of clarifying the ownership policy for these structs---that only the component owns then and once removed from it, they are no longer valid---seems sane to me.