On Sun, Apr 27, 2014 at 5:31 PM, Trevor Saunders <tsaund...@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.
>
>> 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 ?
>
>> 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
>
