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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [C PATCH] proposal to add new warning -Wsizeof-array-argument


On Sun, Apr 27, 2014 at 06:21:20PM +0530, Prathamesh Kulkarni wrote:
> On Sun, Apr 27, 2014 at 5:31 PM, Trevor Saunders <tsaunders@mozilla.com> wrote:
> > On Sun, Apr 27, 2014 at 02:31:46AM +0530, Prathamesh Kulkarni wrote:
> >> Hi,
> >>     Shall it a good idea to add new warning -Wsizeof-array-argument that
> >> warns when sizeof is applied on parameter declared as an array ?
> >
> > Seems reasonable enough.
> >
> >> Similar to clang's -Wsizeof-array-argument:
> >> http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20110613/042812.html
> >> This was also reported as PR6940:
> >> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=6940
> >>
> >> I have attached a patch that adds the warning to C front-end.
> >
> > if we're doing this for C, we should probably do it for C++ too.
> >
> >> I implemented it by adding a new member BOOL_BITFIELD is_array_parm to
> >> tree_parm_decl. Not sure if that's a good approach.
> >
> > I'm about the last one who should comment on this, but it seems pretty
> > crazy you can't use data that's already stored.
> AFAIU, the information about declarator is stored in c_declarator.
> c_declarator->kind == cdk_array holds true
> if the declarator is an array. However in push_parm_decl, call to
> grokdeclarator returns decl of pointer_type, corresponding to array
> declarator if the array is parameter (TREE_TYPE (decl) is
> pointer_type). So I guess we lose that information there.

I guess that sort of makes sense, so I'll shut up ;)

> >> Index: gcc/tree-core.h
> >> ===================================================================
> >> --- gcc/tree-core.h   (revision 209800)
> >> +++ gcc/tree-core.h   (working copy)
> >> @@ -1411,6 +1411,7 @@ struct GTY(()) tree_const_decl {
> >>  struct GTY(()) tree_parm_decl {
> >>    struct tree_decl_with_rtl common;
> >>    rtx incoming_rtl;
> >> +  BOOL_BITFIELD is_array_parm;
> >
> > BOOL_BITFIELD only makes sense if you declare it as an actually bitfield
> > with size less than that of unisgned int, otherwise you might as well
> > use that directly.  On the other hand I wonder if we can't just nuke
> > BOOL_BITFIELD, it seems to be legacy from a time of C and bool not being
> > a builtin type?
> Thanks, I wasn't aware of that. Could we now use C++ bool type instead ?

you can certainly do |bool x;| as struct fields, that's already all
over.  However its not entirely clear to me if |bool x : 1;| will work
everywhere and take the single bit you'd expect, istr there being
compilers that do stupid things if you use multiple types next to each
other in bitfields, but I'm not sure if we need to care about any of
those.

Trev

> >
> >> Index: gcc/tree.h
> >> ===================================================================
> >> --- gcc/tree.h        (revision 209800)
> >> +++ gcc/tree.h        (working copy)
> >> @@ -1742,6 +1742,7 @@ extern void protected_set_expr_location
> >>  #define TYPE_LANG_SPECIFIC(NODE) \
> >>    (TYPE_CHECK (NODE)->type_with_lang_specific.lang_specific)
> >>
> >> +
> >>  #define TYPE_VALUES(NODE) (ENUMERAL_TYPE_CHECK (NODE)->type_non_common.values)
> >>  #define TYPE_DOMAIN(NODE) (ARRAY_TYPE_CHECK (NODE)->type_non_common.values)
> >>  #define TYPE_FIELDS(NODE) \
> >> @@ -2258,6 +2259,12 @@ extern void decl_value_expr_insert (tree
> >>  #define DECL_INCOMING_RTL(NODE) \
> >>    (PARM_DECL_CHECK (NODE)->parm_decl.incoming_rtl)
> >>
> >> +#define SET_PARM_DECL_IS_ARRAY(NODE, val) \
> >> +  (PARM_DECL_CHECK (NODE)->parm_decl.is_array_parm = (val))
> >
> > if we're adding more stuff here is there a reason it needs to be a macro
> > not a inline function?
> No, shall change that to inline function.
> >
> > Trev
> >

Attachment: signature.asc
Description: Digital signature


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]