[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v11 09/24] qapi: Prefer typesafe upcasts to qapi
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v11 09/24] qapi: Prefer typesafe upcasts to qapi base classes |
Date: |
Tue, 27 Oct 2015 16:06:52 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> On 10/27/2015 01:46 AM, Markus Armbruster wrote:
>> Eric Blake <address@hidden> writes:
>>
>>> A previous patch (commit 1e6c1616) made it possible to
>>> directly cast from a qapi flat union type to its base type.
>>> However, it requires the use of a C cast, which turns off
>>> compiler type-safety checks. Add inline type-safe wrappers
>>> named qapi_FOO_base() for any union type FOO that has a base,
>>> which can be used for a safer upcast, and enhance the
>>> testsuite to cover the new functionality. A future patch
>>> will extend the upcast support to structs.
>>>
>>> Note that C makes const-correct upcasts annoying because
>>> it lacks overloads; these functions cast away const so that
>>> they can accept user pointers whether const or not, and the
>>> result in turn can be assigned to normal or const pointers.
>>> Alternatively, this could have been done with macros, but
>>> those tend to not have quite as much type safety.
>>
>> Well, the macros can be made just as type-safe, but the result is either
>> somewhat ugly and using gcc-isms, or very ugly and unhygienic.
>>
>> I'd write something like "type-safe macros are hairy, and not worthwhile
>> here."
>
> Sure, that works for me.
Sold.
>>> This patch just adds upcasts. None of our code needed to
>>> downcast from a base qapi class to a child.
>>
>> Actually, none of our code needs to upcast unions, either. Only the new
>> tests do. Code that updasts structs exist, but it doesn't use this
>> patch's upcasts until later.
>>
>> Suggest to amend the first paragraph:
>>
>> A previous patch (commit 1e6c1616) made it possible to directly cast
>> from a qapi flat union type to its base type. However, it requires
>> the use of a C cast, which turns off compiler type-safety checks.
>> Fortunately, no such casts exist just, yet.
>
> s/ just,/, just/
Yes, thanks.
>>
>> Regardless, add inline type-safe wrappers named qapi_FOO_base() for
>> any union type FOO that has a base, which can be used for a safer
>> upcast, and enhance the testsuite to cover the new functionality.
>>
>> A future patch will extend the upcast support to structs, where such
>> casts do exist already.
>>
>
> Maybe s/casts/conversions/ - because as of this patch, it is still a
> conversion via foo->base rather than (Base *)foo (it's the next patch
> that gets rid of base, and therefore needs either the cast or the wrapper).
Sold.
>> Patch looks good. I can touch up the commit message in my tree.
>
> Sure, your proposed wording + touchups is fine.
- [Qemu-devel] [PATCH v11 12/24] qapi: Start converting to new qapi union layout, (continued)
- [Qemu-devel] [PATCH v11 12/24] qapi: Start converting to new qapi union layout, Eric Blake, 2015/10/26
- [Qemu-devel] [PATCH v11 13/24] qapi-visit: Convert to new qapi union layout, Eric Blake, 2015/10/26
- [Qemu-devel] [PATCH v11 10/24] qapi: Unbox base members, Eric Blake, 2015/10/26
- [Qemu-devel] [PATCH v11 14/24] tests: Convert to new qapi union layout, Eric Blake, 2015/10/26
- [Qemu-devel] [PATCH v11 08/24] qapi-types: Refactor base fields output, Eric Blake, 2015/10/26
- [Qemu-devel] [PATCH v11 09/24] qapi: Prefer typesafe upcasts to qapi base classes, Eric Blake, 2015/10/26
- [Qemu-devel] [PATCH v11 24/24] qapi: Simplify gen_struct_field(), Eric Blake, 2015/10/26
- [Qemu-devel] [PATCH v11 21/24] tpm: Convert to new qapi union layout, Eric Blake, 2015/10/26
- [Qemu-devel] [PATCH v11 22/24] qapi: Finish converting to new qapi union layout, Eric Blake, 2015/10/26
[Qemu-devel] [PATCH v11 15/24] block: Convert to new qapi union layout, Eric Blake, 2015/10/26
[Qemu-devel] [PATCH v11 03/24] qapi: More robust conditions for when labels are needed, Eric Blake, 2015/10/26
[Qemu-devel] [PATCH v11 23/24] qapi: Reserve 'u' member name, Eric Blake, 2015/10/26