Re: [Patch] Add -gdwarf option to make gcc generate DWARF with the default version

[Jakub Jelinek]

On Thu, Apr 11, 2013 at 02:32:51PM +0530, Senthil Kumar Selvaraj wrote:
> gcc/ChangeLog
> 
> 2013-04-11  Senthil Kumar Selvaraj  
> 
>   * gcc/opts.c : Fix formatting and add comment.

The ChangeLog entry should have been
* opts.c (common_handle_option): Fix formatting and add FALLTHRU
comment.
or so (no gcc/ prefix, paths are always relative to the ChangeLog file
where the changes are recorded, and you need to include the name of the
function).  Committed with those changes.

Jakub

Re: [Patch] Add -gdwarf option to make gcc generate DWARF with the default version

[Senthil Kumar Selvaraj]

On Thu, Apr 11, 2013 at 09:08:08AM +0200, Jakub Jelinek wrote:
> On Wed, Apr 10, 2013 at 06:55:10PM +0530, Senthil Kumar Selvaraj wrote:
> > --- gcc/opts.c
> > +++ gcc/opts.c
> > @@ -1699,6 +1699,18 @@ common_handle_option (struct gcc_options *opts,
> >set_debug_level (SDB_DEBUG, false, arg, opts, opts_set, loc);
> >break;
> >  
> > +case OPT_gdwarf:
> > +  if (arg && strlen(arg) != 0)
> 
> The formatting is wrong.  Space before ( above missing.
> 
> > +{
> > +  error_at (loc, "%<-gdwarf%s%> is ambiguous; "
> > +"use %<-gdwarf-%s%> for DWARF version "
> > +"or %<-gdwarf -g%s%> for debug level", arg, arg, arg);
> > +  break;
> > +}
> > +  else
> > +{
> > +  value = opts->x_dwarf_version;
> > +}
> 
> A single line statement shouldn't be surrounded inside { }.
> 
> ALso, if the OPT_gdwarf case is meant to fallthru into OPT_gdwarf_,
> there should be /* FALLTHRU */ or similar comment.
> 
> >  case OPT_gdwarf_:
> >if (value < 2 || value > 4)
> > error_at (loc, "dwarf version %d is not supported", value);
> 
>   Jakub

That patch is already in. This patch fixes the problems you pointed out.

Regards
Senthil

gcc/ChangeLog

2013-04-11  Senthil Kumar Selvaraj  

* gcc/opts.c : Fix formatting and add comment.


diff --git gcc/opts.c gcc/opts.c
index f96ed16..5f0c49f 100644
--- gcc/opts.c
+++ gcc/opts.c
@@ -1700,7 +1700,7 @@ common_handle_option (struct gcc_options *opts,
   break;
 
 case OPT_gdwarf:
-  if (arg && strlen(arg) != 0)
+  if (arg && strlen (arg) != 0)
 {
   error_at (loc, "%<-gdwarf%s%> is ambiguous; "
 "use %<-gdwarf-%s%> for DWARF version "
@@ -1708,9 +1708,9 @@ common_handle_option (struct gcc_options *opts,
   break;
 }
   else
-{
-  value = opts->x_dwarf_version;
-}
+value = opts->x_dwarf_version;
+  
+  /* FALLTHRU */
 case OPT_gdwarf_:
   if (value < 2 || value > 4)
error_at (loc, "dwarf version %d is not supported", value);

Re: [Patch] Add -gdwarf option to make gcc generate DWARF with the default version

[Jakub Jelinek]

On Wed, Apr 10, 2013 at 06:55:10PM +0530, Senthil Kumar Selvaraj wrote:
> --- gcc/opts.c
> +++ gcc/opts.c
> @@ -1699,6 +1699,18 @@ common_handle_option (struct gcc_options *opts,
>set_debug_level (SDB_DEBUG, false, arg, opts, opts_set, loc);
>break;
>  
> +case OPT_gdwarf:
> +  if (arg && strlen(arg) != 0)

The formatting is wrong.  Space before ( above missing.

> +{
> +  error_at (loc, "%<-gdwarf%s%> is ambiguous; "
> +"use %<-gdwarf-%s%> for DWARF version "
> +"or %<-gdwarf -g%s%> for debug level", arg, arg, arg);
> +  break;
> +}
> +  else
> +{
> +  value = opts->x_dwarf_version;
> +}

A single line statement shouldn't be surrounded inside { }.

ALso, if the OPT_gdwarf case is meant to fallthru into OPT_gdwarf_,
there should be /* FALLTHRU */ or similar comment.

>  case OPT_gdwarf_:
>if (value < 2 || value > 4)
>   error_at (loc, "dwarf version %d is not supported", value);

Jakub

Re: [Patch] Add -gdwarf option to make gcc generate DWARF with the default version

[Jason Merrill]

Applied, thanks.

Jason