Seongbae Park - Re: [PATCH]: Add a new option -Wvariable-length-array (original) (raw)

This is the mail archive of the gcc-patches@gcc.gnu.orgmailing list for the GCC project.

• From: "Seongbae Park"
• To: "Manuel López-Ibáñez"
• Cc: "Joseph S. Myers" , gcc-patches
• Date: Tue, 6 Feb 2007 17:12:07 -0800
• Subject: Re: [PATCH]: Add a new option -Wvariable-length-array
• References: <ab3a61990702061603j22178af4ve67dee9ecc4ff701@mail.gmail.com> <Pine.LNX.4.64.0702070015260.25338@digraph.polyomino.org.uk> <ab3a61990702061619r6c265a5brfa39d7173ce1ef2a@mail.gmail.com> <ab3a61990702061639h7024daddxaba0e86e14c49c22@mail.gmail.com> <6c33472e0702061707i42a3f966u409f7e437adf5671@mail.gmail.com>

On 2/6/07, Manuel López-Ibáñez lopezibanez@gmail.com wrote:

(Take this as my personal opinion, maintainers may actually think different).

+@item -Wvariable-length-array +@opindex Wvariable-length-array +@opindex Wno-variable-length-array +Warn if a variable length array is used, independent of @option{-pedantic}. +

I think this should said ". This option is also enabled by @option{-pedantic}." instead of ", independent of @option{-pedantic}."

Thanks to Joseph's comment, now the logic is different - -pedantic overrides -Wno-variable-length-array, and they have different messages so they are not identical.

Also, I think that tests for "-pedantic-errors -Wvariable-length-array " and "-pedantic -Wno-variable-length-array" would be appropriate. (What happens then?)

-pedantic/-pedantic-errors win (see my second patch). I'll add new tests for -pedantic-errors/-pedantic cases.

Also, have you checked what happens with just "-std=c89" ?

That option has no effect on this flag.

+Wvariable-length-array +C ObjC C++ ObjC++ Var(warn_vla) +Warn if a variable length array is used +

The typical name for the variable associated to warning option -Wx-x-x is warn_x_x_x. I think it would be good to keep this convention and use warn_variable_length_array instead of warn_vla.

Thanks. I'll change the variable name.

•                   if (TREE_CONSTANT (size))

•                      warning (OPT_Wvariable_length_array,

•                               "The size of array %qs can"

•                               "%'t be evaluated", name);

•                    else

•                      warning (OPT_Wvariable_length_array,

•                               "Variable length array %qs is used",

•                               name)

Most (all?) warning messages begin with lowercase (except for those where the first letter is part of an acronym).

Yes. Joseph also pointed this out and I've updated it accordingly. I'll send out another patch after adding new tests and testing it.

#pragma ident "Seongbae Park, compiler, http://seongbae.blogspot.com";

• Follow-Ups:
  • Re: [PATCH]: Add a new option -Wvariable-length-array
    * From: Joseph S. Myers
• References:
  • [PATCH]: Add a new option -Wvariable-length-array
    * From: Seongbae Park
  • Re: [PATCH]: Add a new option -Wvariable-length-array
    * From: Joseph S. Myers
  • Re: [PATCH]: Add a new option -Wvariable-length-array
    * From: Seongbae Park
  • Re: [PATCH]: Add a new option -Wvariable-length-array
    * From: Seongbae Park
  • Re: [PATCH]: Add a new option -Wvariable-length-array
    * From: Manuel López-Ibáñez