Re: Discussion about merging Go frontend

[Dave Korn <dave dot korn dot cygwin at gmail dot com>]

On 02/11/2010 15:06, Ian Lance Taylor wrote:
> Dave Korn writes:
> 
>> +# Trying to get this information from gcc's config is tricky.
>> +case $target in
>> +  x86_64*-mingw*)
>> +    SYM_STYLE=-DSYM_STYLE=ss_none
>> +    ;;
>> +  *-cygwin* | i?86*-mingw* )
>> +    SYM_STYLE=-DSYM_STYLE=ss_win32
>> +    ;;
>> +  *)
>> +    SYM_STYLE=-DSYM_STYLE=ss_none
>> +    ;;
>>  esac
>> -
>> -AC_SUBST(LTO_FORMAT)
>> +AC_SUBST(SYM_STYLE)
> 
> It would seem more natural to use AC_DEFINE here.  Any reason not to do
> that?

  It seems a bit much overkill.  There's only a single -D right now, so why
not pass it straight through?  With AC_DEFINE I'd still have to import @DEFS@
into the makefile, just to get HAVE_CONFIG_H available at build time, and then
add a config.h with a single #define in it.  If there were several symbols to
define, or if there was already an AC_CONFIG_HEADER, I'd do it, but there
isn't yet, so why haul all that extra weight?

>> +  else if (!strncmp (option, "-sym-style=", sizeof ("-sym-style=") - 1))
>> +    {
>> +      switch (option[sizeof ("-sym-style=")])
>> +	{
>> +	case 'w':
>> +	  sym_style = ss_win32;
>> +	  break;
>> +	case 'u':
>> +	  sym_style = ss_uscore;
>> +	  break;
>> +	default:
>> +	  sym_style = ss_none;
>> +	  break;
>> +	}
>> +    }
> 
> This looks wrong, because sizeof ("-sym-style=") will be 1 too large in
> the switch clause.

  Oops, yes, thanks.

> And of course this code now has to use simple_object rather than
> objfile.

  Yep, I'm just revising it now.  Will spin those fixes in at the same time,
thanks for reviewing.  Let me know if you really want to insist on the
AC_DEFINE change, but if it's up to me I'd rather just leave it as it is.

    cheers,
      DaveK