Eric Blake <ebl...@redhat.com> writes: > On 02/18/2016 05:05 AM, Markus Armbruster wrote: >> Eric Blake <ebl...@redhat.com> writes: >> >>> The whole point of an alternate is to allow some type-safety while >>> still accepting more than one JSON type. Meanwhile, the 'any' >>> type exists to bypass type-safety altogether. The two are >>> incompatible: you can't accept every type, and still tell which >>> branch of the alternate to use for the parse; fix this to give a >>> sane error instead of a Python stack trace. >>> >>> Signed-off-by: Eric Blake <ebl...@redhat.com> >>> > >> >> Interestingly, find_alternate_member_qtype(T) can return None in two >> ways: when builtin_types[T] is None (only for T == 'any'), and when T is >> neither built-in, struct, enum or union (it must be alternate then). >> >> This leads to the question whether this patch catches exactly 'any', as >> the commit message claims, or alternate as well. >> > >>> >>> @@ -629,7 +629,10 @@ def check_alternate(expr, expr_info): >>> value, >>> allow_metas=['built-in', 'union', 'struct', 'enum']) >>> qtype = find_alternate_member_qtype(value) >> > >> >> Could use a test for alternate member of alternate type. > > One step ahead of you: commit 3d0c4829 added the test > alternate-nested.json, and commits 44bd1276 and dd883c6f fixed the > parser to reject it (first by a hard-coded check, then via allow_metas[] > excluding alternates). 'any' is the only value that could sneak > through, because it is a subset of 'built-in' which allow_metas[] > whitelisted.
Then find_alternate_member_qtype()'s final return None is unreachable, correct? > If you want to squash any of this information into the commit message, > though, I don't mind. I'll consider it when I apply.
