David Hildenbrand <da...@redhat.com> writes:
> I found some more ugliness, looking at the tests. I am not sure the test
> is correct here.
>
> test_visitor_in_intList():
>
> v = visitor_input_test_init(data, "1,2,0,2-4,20,5-9,1-8");
>
> -> we expect { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 20 }, implying that the
> visitor actually does sorting + duplicate elimination. Now I consider
> this is wrong. We are parsing a list of integers, not an ordered set.
>
> What's your take on this?
I think you're right. Visitors are tied to QAPI, and QAPI does *lists*,
not sets. Lists are more general than sets.
I figure what drove development of this code was a need for sets, so
sets got implemented. Review fail.
If the visitor does lists, whatever needs sets can convert the lists to
sets.
I'd advise against evolving the current code towards sanity. Instead,
rewrite, and rely on tests to avoid unwanted changes in behavior.
