ailrst
commented
6 months ago due to the complex set of assertions designed to ensure that every element is only in a single IntrusiveList
The fact a list element is only ever in a single list at a time is true by construction, the assertions only exist to make it obvious if the interface is used incorrectly.
In this case the bug is that block's constructor is constructing a new intrusive list when it is passed an intrusive list, rather than just using the list (which I think used to be the case in the past), or moving the elements across.
The simplest solution is probably to by default .copy() the elements on insertion to the intrusive list, and the interface no longer requires inserted elements to not be an element of any other intrusive list.
l-kent
The IntrusiveList is very difficult to work with due to the complex set of assertions designed to ensure that every element is only in a single IntrusiveList. The way this is implemented now (as of the recent staging pull request being merged in) conflicts with the design of the splitOn method: https://github.com/UQ-PAC/bil-to-boogie-translator/blob/b6a2409f60b977985ee0c97959cdcfe04bdb35f0/src/main/scala/util/intrusive_list/IntrusiveList.scala#L241-L257 The splitOn method returns an IntrusiveList that is not part of a Block or Procedure, and was designed to help split one Block into multiple by removing statements past a certain point from an IntrusiveList and adding them to a new, separate IntrusiveList. The issue now is that this new IntrusiveList produced by splitOn cannot be used in the creation of a new Block, as the statements are already in the IntrusiveList produced by splitOn and so cannot be added to the new Block's IntrusiveList. Making this work at present would require some messy workaround like moving all of the result of splitOn's contents to a new List (that is not an IntrusiveList) and then adding that to a new Block, which seems less than ideal.
This should be changed in some way so that the design of IntrusiveList does not contradict itself as it currently does.
This sort of issue is also why having an earlier intermediate GTIRB/ASLp data structure would be helpful - it means we can avoid having to deal with the intricacies of IntrusiveList while creating the broader program data structure, as I suggested in #183.