Re: [PATCH 2/3] convert "enum date_mode" into a struct
- From
Jeff King <peff@peff.net>
- Date
- Jun 25, 2015, 17:22 UTC
- Message-ID
- <20150625172246.GA24744@peff.net>
- In-Reply-To
- <20150625170328.GV18226@serenity.lan>
On Thu, Jun 25, 2015 at 06:03:28PM +0100, John Keeping wrote:
Show 7 quoted lines
> > Ideally we could say:
> >
> > show_date(t, tz, &{ DATE_NORMAL });
> >
> > but of course C does not allow that.
>
> Yes it does, e.g. in 6.5.2.5 of C11, example 3 shows:Well, yes. But we generally restrict ourselves to C89 here, so we are not even close.
Show 6 quoted lines
> Of course, whether all of the compilers we target support it is a
> different question. If they do, perhaps something like:
>
> #define SIMPLE_DATE(f) &(struct date_mode) { DATE_NORMAL }
>
> would allow the callers to remain reasonably sane.My patch already introduces DATE_MODE, so you could conditionally hide it there, and fall back to date_mode_from_type when the compiler is too old for this. But then, what is the advantage over the existing solution? It's reentrant, but I don't think that is a problem here.
And in patch 3, you'll see that I add an extra assertion to date_mode_from_type that this cannot support (to make sure that we do not create a DATE_STRFTIME mode with no matching format string). The syntax above would at least give us NULL for the string, which is better than random garbage, but I think the assert is better still.
-Peff