[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 06/12] qapi/source: Add builtin null-object sentinel
From: |
Markus Armbruster |
Subject: |
Re: [PATCH 06/12] qapi/source: Add builtin null-object sentinel |
Date: |
Thu, 17 Dec 2020 12:56:44 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) |
John Snow <jsnow@redhat.com> writes:
> On 12/16/20 4:22 AM, Markus Armbruster wrote:
>> 2. On error with "no source info", don't crash.
>> I have my doubts on this one.
>> Such an error means the QAPI code generator screwed up, at least
>> in
>> theory. Crashing is only proper. It gets the screwup fixed.
>>
>
> QAPISemError and friends will also halt the generator and don't
> produce output and will fail tests. They aren't less visible or more
> ignorable somehow.
>
>> In practice, errors due to interactions between built-in stuff and
>> user-defined stuff could conceivably escape testing. I can't
>> remember such a case offhand.
>> Will the "no source info" error be more useful than a crash?
>> Possibly. Will it get the screwup fixed? Maybe not.
>
> I don't understand this; if it's an error -- there's no QAPI, there's
> no QEMU. It's definitely getting fixed.
>
> If QAPISourceInfo is primarily used for printing error information, we
> are already in a situation where the generator is hosed and has
> wandered itself into a problem that can't be ignored.
>
> There's no additional value in having python crash twice per every
> crash because we have bad types in our error reporting functions.
Consider the following scenario. The average developer knows just
enough about QAPI to be dangerous. That's okay; if you had to be a QAPI
expert to modify the QAPI schema, we would have failed. Now meet Joe
Average. He's a good guy. Today his job happens to require extending
the QAPI schema. In a hurry, as usual. So Joe brings his well-honed
voodoo programming skills to bear, and writes something that looks
plausible to him. His build fails. He's not surprised; he's voodoo-
programming after all. However, the error message is less clear than
usual. Something about a '[builtin]' file. There is no '[builtin]'
file. What to do? Obvious! If a bit of voodoo doesn't get you over
the finish line, use more: twiddle the schema until it works.
If his build failed with a Python backtrace instead, Joe would
immediately know that he ran into a bug in our tooling he should report.
Again, I don't mean to criticize Joe. I've walked in his shoes myself.
- Re: [PATCH 04/12] qapi/gen: assert that _start_if is not None in _wrap_ifcond, (continued)
[PATCH 07/12] qapi/gen: write _genc/_genh access shims, John Snow, 2020/12/14
[PATCH 09/12] qapi/gen: move write method to QAPIGenC, make fname a str, John Snow, 2020/12/14
[PATCH 10/12] tests/qapi-schema: Add quotes to module name in test output, John Snow, 2020/12/14
[PATCH 11/12] qapi/schema: Name the builtin module "" instead of None, John Snow, 2020/12/14