[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate typ
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate types |
Date: |
Thu, 4 May 2017 17:18:05 -0300 |
User-agent: |
Mutt/1.8.0 (2017-02-23) |
On Thu, May 04, 2017 at 03:03:36PM -0500, Eric Blake wrote:
> On 05/04/2017 02:42 PM, Eduardo Habkost wrote:
>
> >>>> As in: we forbid the combination of a scalar (whether 'int', 'number',
> >>>> 'bool', and perhaps 'null') with a plain 'str' (since there's no way to
> >>>> tell whether '1' should parse as an integer or the string "1"); and
> >>>> combining a scalar with an 'enum' requires that all enum members be
> >>>> distinct from what could otherwise be parsed as a scalar?
> >>>
> >>> Exactly.
> >>>
> >>>> I can live
> >>>> with such a restriction.
> >>>
> >>> Then let's do it.
> >>>
> >>> Eduardo, are you comfortable implementing this, or would you like me to
> >>> do it?
> >>
> >> I will give it a try and include it in the next version. Thanks!
> >
> > So, I made qapi.py detect ambiguous alternates[1]. The bad news
> > is that lots of the alternates in qapi-schema-test.json already
> > break those rules.
>
> I'm not surprised, but if test code gets in the way of real life, I'm in
> favor of simplifying test code (change what is currently positive tests
> of "does this corner case work even though no one uses it" to instead be
> negative tests "do we properly reject this alternate as ambiguous").
In a few cases, I have the impression that the test cases aren't
testing a feature nobody uses, but it's testing a feature
everybody relies on (like QAPI_CLONE), but using an alternate
type definition that nobody uses.
>
> > This will require changing the schema and
> > rewriting tests in test-clone-visitor and qobject-input-visitor.
>
> Yes, and Markus or I could help do some of that work, if you don't feel
> up to it.
I plan to work on it after I send v2 of this series, but if
anybody wants to take over, please be my guest. :)
>
> >
> > I think there's a small risk we will want to support some of
> > those forbidden alternate combinations again in the future.
>
> Maybe, but 'git revert' is powerfully easy, and whoever adds the
> (now-ambiguous) use case should be able to justify their need at the
> time they (re-)add support.
>
> > If
> > that happens, detecting them at runtime in string-input-visitor
> > will keep us safe.
>
> I still much prefer compile-time rejections ("we don't support this, and
> we're telling you up front") over runtime rejections ("it compiled, and
> only if you get lucky will you discover that you had a problem").
Yeah. I don't mean the runtime check would be a good replacement
for the compile-time checks, but that the runtime checks will be
useful in case we have to disable the compile-time checks one
day.
>
> >
> > I plan to submit v2 with code to detect ambiguous alternates at
> > runtime, only, because it seems simpler than rewriting the test
> > code.
>
> Simpler, maybe. But harder to maintain, so it may STILL be worth going
> the more complex way and updating the testsuite to comply with the new
> rules. Perhaps it can be done as followups (again, where Markus or I
> may step in and do the rest on top of your initial work).
>
> > After that, we can still make QAPI reject them at compile
> > time too, if we really want to.
> >
> > [1]
> > https://github.com/ehabkost/qemu-hacks/commit/cda70a2e1c30c8dadb36fd46095a4f6ee3d89737
> >
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc. +1-919-301-3266
> Virtualization: qemu.org | libvirt.org
>
--
Eduardo
- Re: [Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate types, (continued)
- Re: [Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate types, Eric Blake, 2017/05/02
- Re: [Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate types, Markus Armbruster, 2017/05/03
- Re: [Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate types, Eduardo Habkost, 2017/05/03
- Re: [Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate types, Markus Armbruster, 2017/05/04
- Re: [Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate types, Eric Blake, 2017/05/04
- Re: [Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate types, Markus Armbruster, 2017/05/04
- Re: [Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate types, Eduardo Habkost, 2017/05/04
- Re: [Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate types, Eduardo Habkost, 2017/05/04
- Re: [Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate types, Eric Blake, 2017/05/04
- Re: [Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate types,
Eduardo Habkost <=
- Re: [Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate types, Markus Armbruster, 2017/05/05
[Qemu-devel] [PATCH 3/4] tests: Add [+-]feature and feature=on|off test cases, Eduardo Habkost, 2017/05/02
[Qemu-devel] [PATCH 4/4] x86: Support feature=force on the command-line, Eduardo Habkost, 2017/05/02