Re: [RFC PATCH v2] list-objects-filter: merge filter data structs

From: Matthew DeVore <matvore@comcast.net>
Date: May 30, 2019, 18:29 UTC
Message-ID: <20190530182920.GA4641@comcast.net>
In-Reply-To: <xmqqef4fyl9l.fsf@gitster-ct.c.googlers.com>

On Thu, May 30, 2019 at 09:12:06AM -0700, Junio C Hamano wrote:

Show 8 quoted lines

> > +	union {
> > +		struct {
> 
> Name this, and the ohter two union members, and the union itself as
> one member inside the outer struct; some compilers would be unhappy
> with the GCC extension that allows you to refer to the member in
> this struct as ((struct filter_data *)p)->seen_at_depth, no?
>

Anonymous unions and structs apparently require C11, which I guess is not something we want to require.

If I have to name the union and struct, a lot of the conciseness of this refactor is lost. All accesses of filter data either have to be qualified with something like filter_data->type_specific.tree.seen_at_depth. If we want to avoid that messy construct, we are back to assigning an alias pointer of the correct type as we are doing before this patch anyway.

It's possible there is a clean and concise way to do C-style OO that is better than what is already here, but this seems to be not worth our time at this point. For the time being, I'll just change this patch to simplify the API (so everything is emcompassed in an opaque `struct filter *`) and keep the implementation more or less as-is.

Thank you for taking a look.