Eric Blake <ebl...@redhat.com> writes: > On 10/27/2015 01:46 AM, Markus Armbruster wrote: >> Eric Blake <ebl...@redhat.com> 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.
