support sequence containing sequence?
Opened this issue · 3 comments
(for run and vals)
problems:
unambiguously distinguishing between sequence names and command names- preventing circular reference
It's preferable to detect and prevent circular reference at seq set/edit time, rather than just at seq run/vals time.
Currently we release the seq inventory writelock (while still holding the seq item writelock) during the interactive part of the edit operation, so that we don't prevent other sequences from being created/deleted while someone is dithering around on the edit line. That does mean we can't re-acquire the seq inventory lock later for circular-ref-checking, because of restrictions on order of lock acquisition...
But I think all that's necessary is to readlock any seq items mentioned in the line -- and readlock any seqs mentioned in those, etc. However, that doesn't honor the necessary ordering for multi-item locking. In fact any locking of other seqs is likely to fall afoul of multi-item ordering issues since we're already currently holding one seq item lock.
Hmm.
In any case it looks like it will be way more natural to do the cycle check in sequence.cli_set or cli_edit, rather than in sequence_impl_op.define. That's probably OK if I declare that the split for the responsibility in validation is that cli_set/edit handles problems that involve other objects, while define just checks this object itself. (That means popping the existing undefined-commands check all the way back out into cli_set/edit.)
One more observation: if we allow sequence-containing-sequence, then any "seq run" operation is going to need to readlock all sequences at least briefly (and then hold the relevant ones for the duration of the run). Similarly for "seq print", and similarly with writelocks for "seq vals". So we can certainly do the cycle checks there, and we have to pay the locking cost/complexity in any case.
If we can do the cycle checks there, what should we do at set/edit time?
For "set" we really could do a cycle check without any lock-ordering problems I think, although I'd need to introduce some new lock functions (for getting 1 seq item writelock and a bunch of seq item readlocks, together in the correct order). But that would be a little odd if we couldn't give the same guarantee on "edit". Maybe for both "set" and "edit", after the write is done we can release all locks then get the necessary readlocks to do the cycle check just to generate a warning if it fails... basically say "you're allowed to do this set/edit, but as things stand you won't be able to do print/run/vals ops with the result".
This is feeling like a bit of a rabbit-hole, and I'm not sure how useful a feature this would be, so going to sit on it for a while.
If we're going to be allowing item specs in sequence content (as per issue #28) then it is probably fine to not do the cycle check at all during seq creation or edit... just do it when the seq is used.