Eric Blake <ebl...@redhat.com> writes: > On 09/24/2015 10:29 AM, Markus Armbruster wrote: > >>>>> + >>>>> + /* FIXME: Order of alternate should not affect semantics */ >>>> >>>> Inhowfar does it affect semantics? Or asked differently: what exactly >>>> is wrong with this test now? >>>> >>>>> + v = visitor_input_test_init(data, "42"); >>>>> + visit_type_AltThree(v, &three, NULL, &error_abort); >>>>> + g_assert_cmpint(three->kind, ==, ALT_THREE_KIND_N); >>>>> + g_assert_cmpfloat(three->n, ==, 42); >>>>> + qapi_free_AltThree(three); >>>>> + one = NULL; >>> >>> >>> AltTwo and AltThree are ostensibly the same struct (two branches, one >>> for 'str' and one for 'number', just in a different order), but they >>> parsed differently (AltTwo failed, AltThree succeeded). The bug is >>> fixed later when the order of the branch declaration no longer impacts >>> the result of the parse. >> >> Then nothing is wrong with this test case, and the FIXME doesn't belong >> here. > > Actually, the test for AltThree succeeds only by accident. There are two > bugs at play; when I fix the first bug (order shouldn't matter: AltTwo > and AltThree should parse identically), then the second bug is finally > exposed (integers aren't being parsed as numbers, in either AltTwo or > AltThree). But I can certainly rework the FIXMEs both here and on the > first fix (19/46) to make it more obvious what the second fix (20/46) is > good for.
Yes, please. I find the FIXME quoted above confusing, because it makes me look for problems exposed by this test when there are none.
