Re: [PATCH 4/8] builtin/config: collect "value_regexp" data in a struct

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 22, 2019, 06:30 UTC
Message-ID: <xmqqpnhk4e3x.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <CAN0heSo3E0g_ZhHb64cisZQu9hfF31sQEyyZB4X_eRyiFtv4mQ@mail.gmail.com>

Martin Ågren <martin.agren@gmail.com> writes:

Show 14 quoted lines

>> > +static struct {
>> > +     enum { none, regexp } mode;
>>
>> We often use the same identifier for a struct and an instance of the
>> struct, taking advantage of the fact that they live in separate
>> namespaces, but lowercase enumerated values like 'regexp' that
>> collides with the field name (and possibly a variable name used
>> elsewhere) smells a bit too much.
>
> Ok, thanks for sanity-checking.
>
>> > +     regex_t *regexp;
>> > +     int do_not_match; /* used with `regexp` */
>> > +} cmd_line_value;

I _might_ want to take this back. A pattern that uses the "mode" to switch among the possibilities in a union, i.e.

	struct {
		enum {
			<something>_none,
			<something>_regexp,
			<something>_bool,
			<something>_int,
		} mode;
                union {
			<type-used-when-it-is-regexp> regexp;
			<type-used-when-it-is-bool> bool;
			<type-used-when-it-is-int> int;
		} u;
	};

may not be too bad.  So I do not strongly mind the lowercase.

But I still do mind an overly bland names for identifiers in an enum, as enum is not quite a type on its own ('regexp' in one enum may collide with the same identifier in another enum)..

Thanks.