[C/C++] Add a new warning flag -Wself-assign

Sorry for my delay in responding. I finally got time to work on this
patch after coming back from holidays.

On Tue, Jun 29, 2010 at 12:51 PM, Manuel López-Ibáñez
<lopezibanez@gmail.com> wrote:
>
> --- gcc/c-family/c-opts.c       (revision 161236)
> +++ gcc/c-family/c-opts.c       (working copy)
> @@ -486,6 +486,7 @@ c_common_handle_option (size_t scode, co
>       warn_unknown_pragmas = value;
>
>       warn_uninitialized = value;
> +      warn_self_assign = value;
>
>       if (!c_dialect_cxx ())
>        {
>
> Please, use the new handle_option as it is done for Wimplicit just a
> few lines above. Otherwise, Werror= and pragma diagnostic won't work
> correctly. I know that the majority of options are not enabled this
> way. They should, but no one has bothered to finish this transition.

Done as suggested.

>
> >        * dbxout.c (DEBUGGER_ARG_OFFSET):
> >        * omega.c (omega_eliminate_redundant)
> >        * tree-ssa-ccp.c (ccp_lattice_meet)
> >        * cgraphunit.c (cgraph_copy_node_for_versioning)
> >        * config/i386/i386.c (ix86_vectorize_builtin_vec_perm)
> >
>
> Please, sent these fixes separately. They are independent from your
> patch and make your patch more difficult to review. The smaller a
> patch the quickest to review. You may think that CCing more
> maintainers will get your patch reviewed quicker, but the opposite is
> often true.

Most certainly. I have removed them from this patch and will send out
separate patches for these changes.

On Tue, Jun 29, 2010 at 9:29 PM, Jason Merrill <jason@redhat.com> wrote:
>
> On 06/24/2010 06:54 PM, Le-Chun Wu wrote:
>>
>> +   expr_folded_flag:
>> +
>> +       EXPR_FOLDED in
>> +           all expressions
>> +           all decls
>> +           all constants
>
> This seems problematic given that DECLs and some constants are shared. If at some point some expression is folded into a DECL, any subsequent appearance of that DECL will be marked EXPR_FOLDED; so if we have
>
> x = x+0;
> x = x;
>
> the second assignment won't warn.  Right?

Yes, that's right.  This is a great catch. Wrapping uses of decls will
surely help in this case but, as you also said, it will require a
significant amount of work to make such a change to the IR. For now,
to fix this issue, I would clear the expr_folded flag after the
self-assign check (for each statement) is done. I also added comments
in tree.h to warn people not to use this flag in other places, and
added a new test for this case. What do you think?

Attached is the revised patch that incorporates Manuel's and Jason's
comments. Bootstrapped (with additional patches that address the
self-assign code in GCC source) and tested on x86_64-gnu-linux. OK for
trunk? (If it's OK, I will make sure the patches that fix the
self-assign warnings in GCC source get submitted before this one.)
Thanks.

Le-chun

gcc/ChangeLog:

2010-07-21  Le-Chun Wu  <lcwu@google.com>

        * doc/invoke.texi (-Wall): Include -Wself-assign.
        (-Wself-assign): Documentation for the new flag.
        (-Winit-self): Remove.
        (-Wuninitialized): Remove mention of -Winit-self.
        (-Wunused-variable): Mention of new workaround.
        * tree.h (tree_base): Add a new tree_base flag indicating if an
        expression is the result of constant-folding.
        (operand_equal_flag): Add two new flags for operand_equal_p routine.
        * fold-const.c (operand_equal_p): Support comparison of NULL
        operands and operands without types.
        (set_expr_folded_flag): New helper function.
        (fold_unary_loc_1): Renamed from fold_unary_loc.
        (fold_unary_loc): A wrapper around fold_unary_loc_1 function that sets
        the EXPR_FOLDED flag of the folded expression if folding is
        successful.
        (fold_binary_loc_1): Renamed from fold_binary_loc.
        (fold_binary_loc): A wrapper around fold_binary_loc_1 function that
        sets the EXPR_FOLDED flag of the folded expression if folding is
        successful.
        (fold_ternary_loc_1): Renamed from fold_ternary_loc.
        (fold_ternary_loc): A wrapper around fold_ternary_loc_1 function that
        sets the EXPR_FOLDED flag of the folded expression if folding is
        successful.
        * c-parser.c (c_parser_declaration_or_fndef): Check for
        self-assign after parsing variable initialization.
        (c_parser_expr_no_commas): Check for self-assign after parsing an
        assignment.

gcc/c-family/ChangeLog

2010-07-21  Le-Chun Wu  <lcwu@google.com>

        * gcc/c-family/c-gimplify.c (c_gimplify_expr): Remove support for
        -Winit-self.
        * gcc/c-family/c.opt: Make -Winit-self a no-op and add -Wself-assign.
        * gcc/c-family/c-opts.c : Enable -Wself-assign by -Wall.
        * gcc/c-family/c-common.c (check_for_self_assign): New function.
        * gcc/c-family/c-common.h: Add prototype for check_for_self_assign.

gcc/cp/ChangeLog

2010-07-21  Le-Chun Wu  <lcwu@google.com>

        * init.c (perform_member_init): Check for self-assign after parsing
        class member initialization.
        * parser.c (cp_parser_assignment_expression): Check for self-assign
        after parsing an assignment.
        (cp_parser_init_declarator): Check for self-assign after parsing
        variable initialization.

gcc/testsuite/ChangeLog

2010-07-21  Le-Chun Wu  <lcwu@google.com>

        * gcc.dg/plugin/selfassign.c (check_self_assign): Renamed from
        warn_self_assign.
        * gcc.dg/wself-assign-1.c: New test.
        * gcc.dg/wself-assign-2.c: New test.
        * gcc.dg/wself-assign-3.c: New test.
        * gcc.dg/uninit-D-O0.c: Add new warning.
        * gcc.dg/uninit-D.c: Add new warning.
        * gcc.dg/uninit-E-O0.c: Modify comment.
        * gcc.dg/uninit-E.c: Modify comment.
        * g++.dg/plugin/selfassign.c (check_self_assign): Renamed from
        warn_self_assign.
        * g++.dg/warn/Wself-assign-1.C: New test.
        * g++.dg/warn/Wself-assign-2.C: New test.
        * g++.dg/warn/Wself-assign-3.C: New test.
        * g++.dg/warn/Wself-assign-4.C: New test.
        * g++.dg/warn/Wself-assign-5.C: New test.
        * g++.dg/warn/Wself-assign-6.C: New test.

[C/C++] Add a new warning flag -Wself-assign

Commit Message

Comments

Patch