Re: [PATCH] builtin/config: work around an unsized array forward declaration

[Jeff King <peff@peff.net>]

On Thu, Jul 05, 2018 at 08:34:45PM +0200, Beat Bolli wrote:

> As reported here[0], Microsoft Visual Studio 2017.2 and "gcc -pedantic"
> don't understand the forward declaration of an unsized static array.
> They insist on an array size:
> 
>     d:\git\src\builtin\config.c(70,46): error C2133: 'builtin_config_options': unknown size
> 
> The thread [1] explains that this is due to the single-pass nature of
> old compilers.

Right, that makes sense.

> To work around this error, introduce the forward-declared function
> usage_builtin_config() instead that uses the array
> builtin_config_options only after it has been defined.
> 
> Also use this function in all other places where usage_with_options() is
> called with the same arguments.

Your patch is obviously correct, but I think here there might be an even
simpler solution: just bump option_parse_type() below the declaration,
since it's the only one that needs it. That hunk is bigger, but the
overall diff is simpler, and we don't need to carry that extra wrapper
function.

As a general rule for this case (because reordering isn't always an
option), I also wonder if we should prefer just introducing a pointer
alias:

  /* forward declaration is a pointer */
  static struct option *builtin_config_options;

  /* later, declare the actual storage and its alias */
  static struct option builtin_config_options_storage[] = {
	...
  };
  static struct option *builtin_config_options = builtin_config_options_storage;

There are occasionally cases where the caller really wants an array and
not a pointer, but in practice those are pretty rare.

I have a slight preference for the reordering solution in this case, but
any of them would be OK with me.

-Peff