[patch] Add a new warning flag -Wself-assign

[patch] Add a new warning flag -Wself-assign

[Le-Chun Wu]

[-- Attachment #1: Type: text/plain, Size: 5271 bytes --]

Hi,

This patch adds a new warnings flag "-Wself-assign" that warns about
self-assignment (including self-initialization). This warning is
intended for detecting accidental self-assignment due to typos, and
therefore does not warn on a statement that is semantically a
self-assignment after constant folding. Here is an example of what
will trigger a self-assign warning and what will not:

          void func()
          {
             int i = 2;
             int x = x;   /* warn */
             float f = 5.0;
             double a[3];

             i = i + 0;   /* not warn */
             f = f / 1;   /* not warn */
             a[1] = a[1]; /* warn */
             i += 0;      /* not warn */
          }

As mentioned above, this flag warns about self-initialization as well.
Unlike the existing  -Winit-self flag, this new flag does not require
the use of -Wuninitialized. And also it warns about
self-initialization of global variables, function-scoped static
variables, and class members which -Winit-self doesn't. Here is
another example:

        class Foo {
         private:
          int a_;

         public:
          Foo() : a_(a_) {} // warns with -Wself-assign, but not with
-Winit-self
        };

        int g = g; // warns with -Wself-assign, but not with -Winit-self
        Foo foo = foo; // warns with -Wself-assign, but not with -Winit-self

        int func()
        {
          int x = x; // warns with both -Wself-assign and -Winit-self
          static int y = y; // warns with -Wself-assign, but not with
-Winit-self
        }

In our testing of this new flag on our internal C/C++ code base, we
saw cases where self-assignment are intentional. For example, a C++
programmers might write code to test whether an overloaded `operator='
works when the same object is assigned to itself.  One way to work
around the self-assign warning in such case is using the functional
form `object.operator=(object)' instead of the assignment form `object
= object', as shown in the following example.

          void test_func()
          {
             MyType t;

             t.operator=(t);  // not warn
             t = t;           // warn
          }

Another common case of intentional self-assignment is using `x = x' to
avoid `-Wunused-variable' warning if x is not used. To work around
the self-assign warning in such cases, people can use `(void) x' or
`static_cast<void>(x)' instead (or use the 'unused' attribute).

(Note that most of what I mentioned above is included in the
documentation for -Wself-assign in invoke.texi.)

This patch is bootstrapped and tested on x86_64-unknown-linux-gnu.

What do people think about this new warning flag? Is it OK for the trunk?

Thanks,

Le-chun


2010-05-28  Le-Chun Wu  <lcwu@google.com>

        * gcc/doc/invoke.texi: Documentation for -Wself-assign.
        * gcc/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.
        * gcc/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.
        * gcc/testsuite/gcc.dg/plugin/selfassign.c (check_self_assign):
        Renamed from warn_self_assign.
        * gcc/testsuite/gcc.dg/wself-assign-1.c: New test.
        * gcc/testsuite/gcc.dg/wself-assign-2.c: New test.
        * gcc/testsuite/g++.dg/plugin/selfassign.c (check_self_assign):
        Renamed from warn_self_assign.
        * gcc/testsuite/g++.dg/warn/Wself-assign-1.C: New test.
        * gcc/testsuite/g++.dg/warn/Wself-assign-2.C: New test.
        * gcc/testsuite/g++.dg/warn/Wself-assign-3.C: New test.
        * gcc/testsuite/g++.dg/warn/Wself-assign-4.C: New test.
        * gcc/cp/init.c (perform_member_init): Check for self-assign after
        parsing class member initialization.
        * gcc/cp/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/common.opt: Add -Wself-assign.
        * gcc/c-common.c (check_for_self_assign): New function that checks for
        self-assignment.
        * gcc/c-common.h: Add prototype for check_for_self_assign.
        * gcc/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.

[-- Attachment #2: selfassign.patch --]
[-- Type: text/x-patch, Size: 27339 bytes --]

Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 159995)
+++ gcc/doc/invoke.texi	(working copy)
@@ -253,7 +253,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wparentheses  -Wpedantic-ms-format -Wno-pedantic-ms-format @gol
 -Wpointer-arith  -Wno-pointer-to-int-cast @gol
 -Wredundant-decls @gol
--Wreturn-type  -Wsequence-point  -Wshadow @gol
+-Wreturn-type -Wself-assign -Wsequence-point  -Wshadow @gol
 -Wsign-compare  -Wsign-conversion  -Wstack-protector @gol
 -Wstrict-aliasing -Wstrict-aliasing=n @gol
 -Wstrict-overflow -Wstrict-overflow=@var{n} @gol
@@ -3306,6 +3306,51 @@ definitions, may be found on the GCC rea
 
 This warning is enabled by @option{-Wall} for C and C++.
 
+@item -Wself-assign
+@opindex Wself-assign
+@opindex Wno-self-assign
+Warn about self-assignment and self-initialization. This warning is intended
+for detecting accidental self-assignment due to typos, and therefore does
+not warn on a statement that is semantically a self-assignment after
+constant folding. Here is an example of what will trigger a self-assign
+warning and what will not:
+
+@smallexample
+@group
+void func()
+@{
+   int i = 2;
+   int x = x;   /* warn */
+   float f = 5.0;
+   double a[3];
+
+   i = i + 0;   /* not warn */
+   f = f / 1;   /* not warn */
+   a[1] = a[1]; /* warn */
+   i += 0;      /* not warn */
+@}
+@end group
+@end smallexample
+
+There are cases where self-assignment might be intentional. For example,
+a C++ programmers might write code to test whether an overloaded
+@code{operator=} works when the same object is assigned to itself.
+One way to work around the self-assign warning in such case is using
+the functional form @code{object.operator=(object)} instead of the
+assignment form @code{object = object}, as shown in the following example.
+
+@smallexample
+@group
+void test_func()
+@{
+   MyType t;
+
+   t.operator=(t);  // not warn
+   t = t;           // warn
+@}
+@end group
+@end smallexample
+
 @item -Wreturn-type
 @opindex Wreturn-type
 @opindex Wno-return-type
@@ -3429,6 +3474,10 @@ This warning is enabled by @option{-Wall
 To suppress this warning use the @samp{unused} attribute
 (@pxref{Variable Attributes}).
 
+Note that a classic way to avoid @option{-Wunused-variable} warning is
+using @code{x = x}, but that does not work with @option{-Wself-assign}.
+Use @code{(void) x} or @code{static_cast<void>(x)} instead.
+
 @item -Wunused-value
 @opindex Wunused-value
 @opindex Wno-unused-value
Index: gcc/tree.h
===================================================================
--- gcc/tree.h	(revision 159995)
+++ gcc/tree.h	(working copy)
@@ -388,8 +388,9 @@ struct GTY(()) tree_base {
   unsigned visited : 1;
   unsigned packed_flag : 1;
   unsigned user_align : 1;
+  unsigned expr_folded_flag : 1;
 
-  unsigned spare : 13;
+  unsigned spare : 12;
 
   /* This field is only used with type nodes; the only reason it is present
      in tree_base instead of tree_type is to save space.  The size of the
@@ -627,6 +628,13 @@ struct GTY(()) tree_common {
 
        SSA_NAME_IS_DEFAULT_DEF in
            SSA_NAME
+
+   expr_folded_flag:
+
+       EXPR_FOLDED in
+           all expressions
+           all decls
+           all constants
 */
 
 #undef DEFTREESTRUCT
@@ -1362,6 +1370,10 @@ extern void omp_clause_range_check_faile
 /* In fixed-point types, means a saturating type.  */
 #define TYPE_SATURATING(NODE) ((NODE)->base.saturating_flag)
 
+/* Nonzero in an expression, a decl, or a constant node if the node is
+   the result of a successful constant-folding.  */
+#define EXPR_FOLDED(NODE) ((NODE)->base.expr_folded_flag)
+
 /* These flags are available for each language front end to use internally.  */
 #define TREE_LANG_FLAG_0(NODE) ((NODE)->base.lang_flag_0)
 #define TREE_LANG_FLAG_1(NODE) ((NODE)->base.lang_flag_1)
@@ -4920,7 +4932,9 @@ extern bool fold_deferring_overflow_warn
 enum operand_equal_flag
 {
   OEP_ONLY_CONST = 1,
-  OEP_PURE_SAME = 2
+  OEP_PURE_SAME = 2,
+  OEP_ALLOW_NULL = 4,      /* Allow comparison of NULL operands.  */
+  OEP_ALLOW_NULL_TYPE = 8  /* Allow comparison of operands without types.  */
 };
 
 extern int operand_equal_p (const_tree, const_tree, unsigned int);
Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c	(revision 159995)
+++ gcc/fold-const.c	(working copy)
@@ -2409,22 +2409,45 @@ combine_comparisons (location_t loc,
 
    If OEP_PURE_SAME is set, then pure functions with identical arguments
    are considered the same.  It is used when the caller has other ways
-   to ensure that global memory is unchanged in between.  */
+   to ensure that global memory is unchanged in between.
+
+   If OEP_ALLOW_NULL is set, this routine will not crash on NULL operands,
+   and two NULL operands are considered equal. This flag is usually set
+   in the context of frontend when ARG0 and/or ARG1 may be NULL mostly due
+   to recursion on partially built expressions (e.g. a CAST_EXPR on a NULL
+   tree.) In this case, we certainly don't want the compiler to crash and
+   it's OK to consider two NULL operands equal. On the other hand, when
+   called in the context of code generation and optimization, if NULL
+   operands are not expected, silently ignoring them could be dangerous
+   and might cause problems downstream that are hard to find/debug. In that
+   case, the flag should probably not be set.  */
 
 int
 operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
 {
+  /* If either is NULL, they must be both NULL to be equal. We only do this
+     check when OEP_ALLOW_NULL is set.  */
+  if ((flags & OEP_ALLOW_NULL) && (!arg0 || !arg1))
+    return arg0 == arg1;
+
+  /* Similar, if either does not have a type (like a released SSA name, or
+     an operand whose type depends on a template parameter), they aren't
+     equal, unless OEP_ALLOW_NULL_TYPE is set, in which case we will continue
+     to compare the operands only when both don't have a type.  */
+  if (!TREE_TYPE (arg0) || !TREE_TYPE (arg1))
+    {
+      if (((~flags) & OEP_ALLOW_NULL_TYPE)
+          || ((TREE_TYPE (arg0) && !TREE_TYPE (arg1))
+              || (!TREE_TYPE (arg0) && TREE_TYPE (arg1))))
+        return 0;
+    }
+
   /* If either is ERROR_MARK, they aren't equal.  */
   if (TREE_CODE (arg0) == ERROR_MARK || TREE_CODE (arg1) == ERROR_MARK
       || TREE_TYPE (arg0) == error_mark_node
       || TREE_TYPE (arg1) == error_mark_node)
     return 0;
 
-  /* Similar, if either does not have a type (like a released SSA name), 
-     they aren't equal.  */
-  if (!TREE_TYPE (arg0) || !TREE_TYPE (arg1))
-    return 0;
-
   /* Check equality of integer constants before bailing out due to
      precision differences.  */
   if (TREE_CODE (arg0) == INTEGER_CST && TREE_CODE (arg1) == INTEGER_CST)
@@ -2435,19 +2458,25 @@ operand_equal_p (const_tree arg0, const_
      because they may change the signedness of the arguments.  As pointers
      strictly don't have a signedness, require either two pointers or
      two non-pointers as well.  */
-  if (TYPE_UNSIGNED (TREE_TYPE (arg0)) != TYPE_UNSIGNED (TREE_TYPE (arg1))
-      || POINTER_TYPE_P (TREE_TYPE (arg0)) != POINTER_TYPE_P (TREE_TYPE (arg1)))
+  if (TREE_TYPE (arg0)
+      && (TYPE_UNSIGNED (TREE_TYPE (arg0)) != TYPE_UNSIGNED (TREE_TYPE (arg1))
+          || (POINTER_TYPE_P (TREE_TYPE (arg0))
+              != POINTER_TYPE_P (TREE_TYPE (arg1)))))
     return 0;
 
   /* We cannot consider pointers to different address space equal.  */
-  if (POINTER_TYPE_P (TREE_TYPE (arg0)) && POINTER_TYPE_P (TREE_TYPE (arg1))
-      && (TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg0)))
-	  != TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg1)))))
+  if (TREE_TYPE (arg0)
+      && (POINTER_TYPE_P (TREE_TYPE (arg0))
+          && POINTER_TYPE_P (TREE_TYPE (arg1))
+          && (TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg0)))
+              != TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg1))))))
     return 0;
 
   /* If both types don't have the same precision, then it is not safe
      to strip NOPs.  */
-  if (TYPE_PRECISION (TREE_TYPE (arg0)) != TYPE_PRECISION (TREE_TYPE (arg1)))
+  if (TREE_TYPE (arg0)
+      && (TYPE_PRECISION (TREE_TYPE (arg0))
+          != TYPE_PRECISION (TREE_TYPE (arg1))))
     return 0;
 
   STRIP_NOPS (arg0);
@@ -2472,9 +2501,10 @@ operand_equal_p (const_tree arg0, const_
   if (TREE_CODE (arg0) != TREE_CODE (arg1)
       /* This is needed for conversions and for COMPONENT_REF.
 	 Might as well play it safe and always test this.  */
-      || TREE_CODE (TREE_TYPE (arg0)) == ERROR_MARK
-      || TREE_CODE (TREE_TYPE (arg1)) == ERROR_MARK
-      || TYPE_MODE (TREE_TYPE (arg0)) != TYPE_MODE (TREE_TYPE (arg1)))
+      || (TREE_TYPE (arg0)
+          && (TREE_CODE (TREE_TYPE (arg0)) == ERROR_MARK
+              || TREE_CODE (TREE_TYPE (arg1)) == ERROR_MARK
+              || TYPE_MODE (TREE_TYPE (arg0)) != TYPE_MODE (TREE_TYPE (arg1)))))
     return 0;
 
   /* If ARG0 and ARG1 are the same SAVE_EXPR, they are necessarily equal.
@@ -2507,7 +2537,8 @@ operand_equal_p (const_tree arg0, const_
 	  return 1;
 
 
-	if (!HONOR_SIGNED_ZEROS (TYPE_MODE (TREE_TYPE (arg0))))
+	if (TREE_TYPE (arg0)
+            && !HONOR_SIGNED_ZEROS (TYPE_MODE (TREE_TYPE (arg0))))
 	  {
 	    /* If we do not distinguish between signed and unsigned zero,
 	       consider them equal.  */
@@ -2575,8 +2606,9 @@ operand_equal_p (const_tree arg0, const_
         {
 	CASE_CONVERT:
         case FIX_TRUNC_EXPR:
-	  if (TYPE_UNSIGNED (TREE_TYPE (arg0))
-	      != TYPE_UNSIGNED (TREE_TYPE (arg1)))
+	  if (TREE_TYPE (arg0)
+              && (TYPE_UNSIGNED (TREE_TYPE (arg0))
+                  != TYPE_UNSIGNED (TREE_TYPE (arg1))))
 	    return 0;
 	  break;
 	default:
@@ -7653,8 +7685,8 @@ build_fold_addr_expr_loc (location_t loc
    OP0.  Return the folded expression if folding is successful.
    Otherwise, return NULL_TREE.  */
 
-tree
-fold_unary_loc (location_t loc, enum tree_code code, tree type, tree op0)
+static tree
+fold_unary_loc_1 (location_t loc, enum tree_code code, tree type, tree op0)
 {
   tree tem;
   tree arg0;
@@ -8302,6 +8334,41 @@ fold_unary_loc (location_t loc, enum tre
     } /* switch (code) */
 }
 
+/* Given an expression tree EXP, set the EXPR_FOLDED flag, and if it is
+   a nop, recursively set the EXPR_FOLDED flag of its operand.  */
+
+static void
+set_expr_folded_flag (tree exp)
+{
+  EXPR_FOLDED (exp) = 1;
+
+  /* If EXP is a nop (i.e. NON_LVALUE_EXPRs and NOP_EXPRs), we need to
+     recursively set the EXPR_FOLDED flag of its operand because the 
+     expression will be stripped later.  */
+  while ((CONVERT_EXPR_P (exp)
+          || TREE_CODE (exp) == NON_LVALUE_EXPR)
+	 && TREE_OPERAND (exp, 0) != error_mark_node)
+    {
+      exp = TREE_OPERAND (exp, 0);
+      EXPR_FOLDED (exp) = 1;
+    }
+}
+
+/* Fold a unary expression of code CODE and type TYPE with operand
+   OP0.  Return the folded expression if folding is successful.
+   Otherwise, return NULL_TREE.
+   This is a wrapper around fold_unary_loc_1 function (which does the
+   actual folding). Set the EXPR_FOLDED flag of the folded expression
+   if folding is successful.  */
+
+tree
+fold_unary_loc (location_t loc, enum tree_code code, tree type, tree op0)
+{
+  tree tem = fold_unary_loc_1 (loc, code, type, op0);
+  if (tem)
+    set_expr_folded_flag (tem);
+  return tem;
+}
 
 /* If the operation was a conversion do _not_ mark a resulting constant
    with TREE_OVERFLOW if the original constant was not.  These conversions
@@ -9394,8 +9461,8 @@ get_pointer_modulus_and_residue (tree ex
    Return the folded expression if folding is successful.  Otherwise,
    return NULL_TREE.  */
 
-tree
-fold_binary_loc (location_t loc,
+static tree
+fold_binary_loc_1 (location_t loc,
 	     enum tree_code code, tree type, tree op0, tree op1)
 {
   enum tree_code_class kind = TREE_CODE_CLASS (code);
@@ -13066,6 +13133,24 @@ fold_binary_loc (location_t loc,
   return tem;
 }
 
+/* Fold a binary expression of code CODE and type TYPE with operands
+   OP0 and OP1.  LOC is the location of the resulting expression.
+   Return the folded expression if folding is successful.  Otherwise,
+   return NULL_TREE.
+   This is a wrapper around fold_binary_loc_1 function (which does the
+   actual folding). Set the EXPR_FOLDED flag of the folded expression
+   if folding is successful.  */
+
+tree 
+fold_binary_loc (location_t loc,
+                 enum tree_code code, tree type, tree op0, tree op1)
+{
+  tree tem = fold_binary_loc_1 (loc, code, type, op0, op1);
+  if (tem)
+    set_expr_folded_flag (tem);
+  return tem;
+}
+
 /* Callback for walk_tree, looking for LABEL_EXPR.  Return *TP if it is
    a LABEL_EXPR; otherwise return NULL_TREE.  Do not check the subtrees
    of GOTO_EXPR.  */
@@ -13102,8 +13187,8 @@ contains_label_p (tree st)
    OP0, OP1, and OP2.  Return the folded expression if folding is
    successful.  Otherwise, return NULL_TREE.  */
 
-tree
-fold_ternary_loc (location_t loc, enum tree_code code, tree type,
+static tree
+fold_ternary_loc_1 (location_t loc, enum tree_code code, tree type,
 	      tree op0, tree op1, tree op2)
 {
   tree tem;
@@ -13438,6 +13523,23 @@ fold_ternary_loc (location_t loc, enum t
     } /* switch (code) */
 }
 
+/* Fold a ternary expression of code CODE and type TYPE with operands
+   OP0, OP1, and OP2.  Return the folded expression if folding is
+   successful.  Otherwise, return NULL_TREE.
+   This is a wrapper around fold_ternary_loc_1 function (which does the
+   actual folding). Set the EXPR_FOLDED flag of the folded expression
+   if folding is successful.  */
+
+tree
+fold_ternary_loc (location_t loc, enum tree_code code, tree type,
+                  tree op0, tree op1, tree op2)
+{
+  tree tem = fold_ternary_loc_1 (loc, code, type, op0, op1, op2);
+  if (tem)
+    set_expr_folded_flag (tem);
+  return tem;
+}
+
 /* Perform constant folding and related simplification of EXPR.
    The related simplifications include x*1 => x, x*0 => 0, etc.,
    and application of the associative law.
Index: gcc/testsuite/gcc.dg/plugin/selfassign.c
===================================================================
--- gcc/testsuite/gcc.dg/plugin/selfassign.c	(revision 159995)
+++ gcc/testsuite/gcc.dg/plugin/selfassign.c	(working copy)
@@ -196,7 +196,7 @@ compare_and_warn (gimple stmt, tree lhs,
 /* Check and warn if STMT is a self-assign statement.  */
 
 static void
-warn_self_assign (gimple stmt)
+check_self_assign (gimple stmt)
 {
   tree rhs, lhs;
 
@@ -251,7 +251,7 @@ execute_warn_self_assign (void)
   FOR_EACH_BB (bb)
     {
       for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
-        warn_self_assign (gsi_stmt (gsi));
+        check_self_assign (gsi_stmt (gsi));
     }
 
   return 0;
Index: gcc/testsuite/gcc.dg/wself-assign-1.c
===================================================================
--- gcc/testsuite/gcc.dg/wself-assign-1.c	(revision 0)
+++ gcc/testsuite/gcc.dg/wself-assign-1.c	(revision 0)
@@ -0,0 +1,27 @@
+/* Test the self-assignemnt detection and warning.  */
+/* { dg-do compile } */
+/* { dg-options "-Wself-assign" } */
+
+struct Bar {
+  int b_;
+  int c_;
+};
+
+int g;
+
+int main()
+{
+  struct Bar *bar;
+  int x = x; /* { dg-warning "assigned to itself" } */
+  static int y;
+  struct Bar b_array[5];
+
+  b_array[x+g].b_ = b_array[x+g].b_; /* { dg-warning "assigned to itself" } */
+  g = g; /* { dg-warning "assigned to itself" } */
+  y = y; /* { dg-warning "assigned to itself" } */
+  bar->b_ = bar->b_; /* { dg-warning "assigned to itself" } */
+  x += 0; /* should not warn */
+  y -= 0; /* should not warn */
+  x /= x; /* should not warn */
+  y *= y; /* should not warn */
+}
Index: gcc/testsuite/gcc.dg/wself-assign-2.c
===================================================================
--- gcc/testsuite/gcc.dg/wself-assign-2.c	(revision 0)
+++ gcc/testsuite/gcc.dg/wself-assign-2.c	(revision 0)
@@ -0,0 +1,24 @@
+/* Test how self-assignment detection handles constant-folding happening */
+/*   when parsing the RHS or the initializer.  */
+/* { dg-do compile } */
+/* { dg-options "-Wself-assign" } */
+
+struct Bar {
+  int b_;
+  float c_;
+};
+
+int g;
+
+int main()
+{
+  struct Bar *bar;
+  int x = x - 0; /* should not warn */
+  static int y;
+  struct Bar b_array[5];
+
+  b_array[x+g].b_ = b_array[x+g].b_ * 1; /* should no warn */
+  g = g + 0; /* should not warn */
+  y = y / 1; /* should not warn */
+  bar->b_ = bar->b_ - 0; /* should not warn  */
+}
Index: gcc/testsuite/g++.dg/plugin/selfassign.c
===================================================================
--- gcc/testsuite/g++.dg/plugin/selfassign.c	(revision 159995)
+++ gcc/testsuite/g++.dg/plugin/selfassign.c	(working copy)
@@ -196,7 +196,7 @@ compare_and_warn (gimple stmt, tree lhs,
 /* Check and warn if STMT is a self-assign statement.  */
 
 static void
-warn_self_assign (gimple stmt)
+check_self_assign (gimple stmt)
 {
   tree rhs, lhs;
 
@@ -251,7 +251,7 @@ execute_warn_self_assign (void)
   FOR_EACH_BB (bb)
     {
       for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
-        warn_self_assign (gsi_stmt (gsi));
+        check_self_assign (gsi_stmt (gsi));
     }
 
   return 0;
Index: gcc/testsuite/g++.dg/warn/Wself-assign-1.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wself-assign-1.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/Wself-assign-1.C	(revision 0)
@@ -0,0 +1,54 @@
+// Test the self-assignemnt detection and warning.
+// { dg-do compile }
+// { dg-options "-Wself-assign" }
+
+class Foo {
+ private:
+  int a_;
+
+ public:
+  Foo() : a_(a_) {} // { dg-warning "assigned to itself" }
+
+  void setA(int a) {
+    a_ = a_; // { dg-warning "assigned to itself" }
+  }
+
+  void operator=(Foo& rhs) {
+    this->a_ = rhs.a_;
+  }
+};
+
+struct Bar {
+  int b_;
+  int c_;
+};
+
+int g = g; // { dg-warning "assigned to itself" }
+Foo foo = foo; // { dg-warning "assigned to itself" }
+
+int func()
+{
+  Bar *bar1, bar2;
+  Foo local_foo;
+  int x = x; // { dg-warning "assigned to itself" }
+  static int y = y; // { dg-warning "assigned to itself" }
+  float *f;
+  Bar bar_array[5];
+  char n;
+  int overflow;
+
+  *f = *f; // { dg-warning "assigned to itself" }
+  bar1->b_ = bar1->b_; // { dg-warning "assigned to itself" }
+  bar2.c_ = bar2.c_; // { dg-warning "assigned to itself" }
+  local_foo = local_foo; // { dg-warning "assigned to itself" }
+  foo = foo; // { dg-warning "assigned to itself" }
+  foo.setA(5);
+  bar_array[3].c_ = bar_array[3].c_; // { dg-warning "assigned to itself" }
+  bar_array[x+g].b_ = bar_array[x+g].b_; // { dg-warning "assigned to itself" }
+  y = x;
+  x = y;
+  x += 0; // should not warn
+  y -= 0; // should not warn
+  x /= x; // should not warn
+  y *= y; // should not warn
+}
Index: gcc/testsuite/g++.dg/warn/Wself-assign-2.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wself-assign-2.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/Wself-assign-2.C	(revision 0)
@@ -0,0 +1,31 @@
+// Test the handling of expressions that depend on template parameters in
+// self-assignemnt detection.
+// { dg-do compile }
+// { dg-options "-Wself-assign" }
+
+template<typename T>
+struct Bar {
+  T x;
+  Bar operator++(int) {
+    Bar tmp = *this;
+    ++x;
+    tmp = tmp; // { dg-warning "assigned to itself" }
+    return tmp;
+  }
+};
+
+template<typename T>
+T DoSomething(T y) {
+  T a[5], *p;
+  Bar<T> b;
+  b.x = b.x; // { dg-warning "assigned to itself" }
+  *p = *p; // { dg-warning "assigned to itself" }
+  a[2] = a[2]; // { dg-warning "assigned to itself" }
+  return *p;
+}
+
+main() {
+  Bar<int> bar;
+  bar++;
+  DoSomething(5);
+}
Index: gcc/testsuite/g++.dg/warn/Wself-assign-3.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wself-assign-3.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/Wself-assign-3.C	(revision 0)
@@ -0,0 +1,35 @@
+// Test how operands_equal_p handles a NULL operand.
+// { dg-do compile }
+// { dg-options "-Wself-assign" }
+
+#include <cstdio>
+
+namespace testing {
+
+class Foo {
+  int f;
+ public:
+  Foo() { printf("Construct Foo\n"); }
+};
+
+class Bar {
+  int b;
+ public:
+  Bar(int x) { printf("Construct Bar\n"); }
+
+  void operator=(const Foo& foo) {
+    printf("Assign Foo to Bar\n");
+  }
+};
+
+}
+
+template <class T>
+void func(T t) {
+  ::testing::Bar(1) = ::testing::Foo(); // used to trigger a segfault
+  ::testing::Foo() = ::testing::Foo(); // { dg-warning "assigned to itself" }
+}
+
+main() {
+  func(2);
+}
Index: gcc/testsuite/g++.dg/warn/Wself-assign-4.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wself-assign-4.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/Wself-assign-4.C	(revision 0)
@@ -0,0 +1,48 @@
+// Test how self-assignment detection handles constant-folding happening
+// when parsing the RHS or the initializer.
+// { dg-do compile }
+// { dg-options "-Wself-assign" }
+
+class Foo {
+ private:
+  int a_;
+
+ public:
+  Foo() : a_(a_+0) {} // should not warn
+
+  void setA(int a) {
+    a_ = a_ + 0; // should not warn
+  }
+
+  void operator=(Foo& rhs) {
+    this->a_ = rhs.a_;
+  }
+};
+
+struct Bar {
+  int b_;
+  float c_;
+};
+
+int g = g * 1; // should not warn
+
+int func()
+{
+  Bar *bar1, bar2;
+  Foo foo;
+  int x = x - 0;        // should not warn
+  static int y = y / 1; // should not warn
+  float *f;
+  Bar bar_array[5];
+
+  *f = *f / 1;             // should not warn
+  bar1->b_ = bar1->b_ * 1; // should not warn
+  bar2.c_ = bar2.c_ - 0;   // should not warn
+  foo.setA(5);
+  bar_array[3].c_ = bar_array[3].c_ * 1;     // should not warn
+  bar_array[x+g].b_ = bar_array[x+g].b_ / 1; // should not warn
+  x += 0;
+  y -= 0;
+  foo = foo;           // { dg-warning "assigned to itself" }
+  foo.operator=(foo);  // should not warn
+}
Index: gcc/cp/init.c
===================================================================
--- gcc/cp/init.c	(revision 159995)
+++ gcc/cp/init.c	(working copy)
@@ -529,8 +529,14 @@ perform_member_init (tree member, tree i
 	init = build_x_compound_expr_from_list (init, "member initializer");
 
       if (init)
-	finish_expr_stmt (cp_build_modify_expr (decl, INIT_EXPR, init,
-						tf_warning_or_error));
+        {
+          finish_expr_stmt (cp_build_modify_expr (decl, INIT_EXPR, init,
+                                                  tf_warning_or_error));
+          /* Check for and warn about self-initialization if -Wself-assign is
+             enabled.  */
+          if (warn_self_assign)
+            check_for_self_assign (input_location, decl, init);
+        }
     }
 
   if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (type))
Index: gcc/cp/parser.c
===================================================================
--- gcc/cp/parser.c	(revision 159995)
+++ gcc/cp/parser.c	(working copy)
@@ -6858,6 +6858,12 @@ cp_parser_assignment_expression (cp_pars
 	      if (cp_parser_non_integral_constant_expression (parser,
 							      NIC_ASSIGNMENT))
 		return error_mark_node;
+
+              /* Check for and warn about self-assignment if -Wself-assign is
+                 enabled and the assignment operator is "=".  */
+              if (warn_self_assign && assignment_operator == NOP_EXPR)
+                check_for_self_assign (input_location, expr, rhs);
+
 	      /* Build the assignment expression.  */
 	      expr = build_x_modify_expr (expr,
 					  assignment_operator,
@@ -14030,6 +14036,10 @@ cp_parser_init_declarator (cp_parser* pa
 			 `explicit' constructor cannot be used.  */
 		      ((is_direct_init || !is_initialized)
 		       ? 0 : LOOKUP_ONLYCONVERTING));
+      /* Check for and warn about self-initialization if -Wself-assign is
+         enabled.  */
+      if (warn_self_assign && initializer)
+        check_for_self_assign (input_location, decl, initializer);
     }
   else if ((cxx_dialect != cxx98) && friend_p
 	   && decl && TREE_CODE (decl) == FUNCTION_DECL)
Index: gcc/common.opt
===================================================================
--- gcc/common.opt	(revision 159995)
+++ gcc/common.opt	(working copy)
@@ -160,6 +160,10 @@ Wpadded
 Common Var(warn_padded) Warning
 Warn when padding is required to align structure members
 
+Wself-assign
+Common Var(warn_self_assign) Init(0) Warning
+Warn when a variable is assigned to itself
+
 Wshadow
 Common Var(warn_shadow) Warning
 Warn when one local variable shadows another
Index: gcc/c-common.c
===================================================================
--- gcc/c-common.c	(revision 159995)
+++ gcc/c-common.c	(working copy)
@@ -9486,4 +9486,21 @@ make_tree_vector_copy (const VEC(tree,gc
   return ret;
 }
 
+/* Check for and warn about self-assignment or self-initialization.
+   LHS and RHS are the tree nodes for the left-hand side and right-hand side
+   of the assignment or initialization we are checking.
+   LOCATION is the source location for RHS.  */
+
+void
+check_for_self_assign (location_t location, tree lhs, tree rhs)
+{
+  /* Only emit a warning if RHS is not a folded expression so that we don't
+     warn on something like x = x / 1.  */
+  if (!EXPR_FOLDED (rhs)
+      && operand_equal_p (lhs, rhs,
+                          OEP_PURE_SAME | OEP_ALLOW_NULL | OEP_ALLOW_NULL_TYPE))
+    warning_at (location, OPT_Wself_assign, G_("%qE is assigned to itself"),
+                lhs);
+}
+
 #include "gt-c-common.h"
Index: gcc/c-common.h
===================================================================
--- gcc/c-common.h	(revision 159995)
+++ gcc/c-common.h	(working copy)
@@ -1060,6 +1060,7 @@ extern VEC(tree,gc) *make_tree_vector (v
 extern void release_tree_vector (VEC(tree,gc) *);
 extern VEC(tree,gc) *make_tree_vector_single (tree);
 extern VEC(tree,gc) *make_tree_vector_copy (const VEC(tree,gc) *);
+extern void check_for_self_assign (location_t, tree, tree);
 
 /* In c-gimplify.c  */
 extern void c_genericize (tree);
Index: gcc/c-parser.c
===================================================================
--- gcc/c-parser.c	(revision 159995)
+++ gcc/c-parser.c	(working copy)
@@ -1297,6 +1297,11 @@ c_parser_declaration_or_fndef (c_parser 
 		  maybe_warn_string_init (TREE_TYPE (d), init);
 		  finish_decl (d, init_loc, init.value,
 		      	       init.original_type, asm_name);
+                  /* Check for and warn about self-initialization if
+                     -Wself-assign is enabled. Don't warn if there is
+                     already an error for the initializer.  */
+                  if (warn_self_assign && DECL_INITIAL (d) != error_mark_node)
+                    check_for_self_assign (here, d, init.value);
 		}
 	    }
 	  else
@@ -4767,7 +4772,13 @@ c_parser_expr_no_commas (c_parser *parse
 				 code, exp_location, rhs.value,
 				 rhs.original_type);
   if (code == NOP_EXPR)
-    ret.original_code = MODIFY_EXPR;
+    {
+      ret.original_code = MODIFY_EXPR;
+      /* Check for and warn about self-assignment if -Wself-assign is
+         enabled.  */
+      if (warn_self_assign)
+        check_for_self_assign (op_location, lhs.value, rhs.value);
+    }
   else
     {
       TREE_NO_WARNING (ret.value) = 1;

Re: [patch] Add a new warning flag -Wself-assign

[Andrew Pinski]

On Fri, May 28, 2010 at 4:47 PM, Le-Chun Wu <lcwu@google.com> wrote:
> As mentioned above, this flag warns about self-initialization as well.
> Unlike the existing  -Winit-self flag, this new flag does not require
> the use of -Wuninitialized.

Well I think we should not warn about local variables with this option
because that was designed to get rid of the uninitialized warning.
The reason why I mention this is because -Wuninitialized is now
supported at -O0.


>        class Foo {
>         private:
>          int a_;
>
>         public:
>          Foo() : a_(a_) {} // warns with -Wself-assign, but not with
> -Winit-self
>        };

I think the above should warn under a different flag and it is
recorded as http://gcc.gnu.org/PR18016 .

I think this warning (except for auto variables) should be turned on with -Wall.

Thanks,
Andrew Pinski

Re: [patch] Add a new warning flag -Wself-assign

[Andrew Pinski]

On Fri, May 28, 2010 at 4:47 PM, Le-Chun Wu <lcwu@google.com> wrote:
>        (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.

This seems wrong and really bad idea.  There seems like you should
have a better way of implementing this without this much extra code to
fold-const.c which is called all through out the middle-end.  I think
if you do add this code, I think you should do some compile time
comparison to make sure it does not slow down the compiler this much.

Thanks,
Andrew Pinski

Re: [patch] Add a new warning flag -Wself-assign

[Dave Korn]

On 29/05/2010 00:47, Le-Chun Wu wrote:

  Hello,

> This patch adds a new warnings flag "-Wself-assign" that warns about
> self-assignment (including self-initialization). This warning is
> intended for detecting accidental self-assignment due to typos, and
> therefore does not warn on a statement that is semantically a
> self-assignment after constant folding. Here is an example of what
> will trigger a self-assign warning and what will not:
> 
>           void func()
>           {
>              int i = 2;
>              int x = x;   /* warn */
>              float f = 5.0;
>              double a[3];
> 
>              i = i + 0;   /* not warn */
>              f = f / 1;   /* not warn */
>              a[1] = a[1]; /* warn */
>              i += 0;      /* not warn */
>           }

  It is a very common idiom to write:

> <returntype> f (<type> x)
> {
>   x = x;
>   [ ... do other stuff ... ]
>   return [some value or none]
> }

... in order try and avoid parameter-unused warnings in a portable fashion;
how does your patch react to this technique?

    cheers,
      DaveK

Re: [patch] Add a new warning flag -Wself-assign

[Eric Botcazou]

> 2010-05-28  Le-Chun Wu  <lcwu@google.com>
>
>         * gcc/doc/invoke.texi: Documentation for -Wself-assign.
>         * gcc/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. * gcc/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.
>         * gcc/testsuite/gcc.dg/plugin/selfassign.c (check_self_assign):
>         Renamed from warn_self_assign.
>         * gcc/testsuite/gcc.dg/wself-assign-1.c: New test.
>         * gcc/testsuite/gcc.dg/wself-assign-2.c: New test.
>         * gcc/testsuite/g++.dg/plugin/selfassign.c (check_self_assign):
>         Renamed from warn_self_assign.
>         * gcc/testsuite/g++.dg/warn/Wself-assign-1.C: New test.
>         * gcc/testsuite/g++.dg/warn/Wself-assign-2.C: New test.
>         * gcc/testsuite/g++.dg/warn/Wself-assign-3.C: New test.
>         * gcc/testsuite/g++.dg/warn/Wself-assign-4.C: New test.
>         * gcc/cp/init.c (perform_member_init): Check for self-assign after
>         parsing class member initialization.
>         * gcc/cp/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/common.opt: Add -Wself-assign.
>         * gcc/c-common.c (check_for_self_assign): New function that checks
> for self-assignment.
>         * gcc/c-common.h: Add prototype for check_for_self_assign.
>         * gcc/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.

Most of this ChangeLog will go in gcc/ChangeLog so no gcc/ prefix.

gcc/cp has its own ChangeLog so:

cp/
        * 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 has its own ChangeLog so:

testsuite/
        * 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.
        * 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.


-- 
Eric Botcazou

Re: [patch] Add a new warning flag -Wself-assign

[Le-Chun Wu]

Andrew,

On Fri, May 28, 2010 at 4:52 PM, Andrew Pinski <pinskia@gmail.com> wrote:
> On Fri, May 28, 2010 at 4:47 PM, Le-Chun Wu <lcwu@google.com> wrote:
>> As mentioned above, this flag warns about self-initialization as well.
>> Unlike the existing  -Winit-self flag, this new flag does not require
>> the use of -Wuninitialized.
>
> Well I think we should not warn about local variables with this option
> because that was designed to get rid of the uninitialized warning.
> The reason why I mention this is because -Wuninitialized is now
> supported at -O0.

It should be pretty easy to modify this patch to not warn about
self-initialization of local variables with this flag. However, what I
don't understand is why a user would want to work around (or suppress)
an uninitialized warning by doing self-initialization. They could have
just initialized a local variable to some other more meaningful value.
Initializing a variable with itself doesn't seem like a good way to
fix an uninitialized warning.

>
>
>>        class Foo {
>>         private:
>>          int a_;
>>
>>         public:
>>          Foo() : a_(a_) {} // warns with -Wself-assign, but not with
>> -Winit-self
>>        };
>
> I think the above should warn under a different flag and it is
> recorded as http://gcc.gnu.org/PR18016 .

I am not sure whether the main purpose of -Winit-self is to work
around uninitialized warnings, but if we are going to warn this case
under a different flag, it seems to me that -Winit-self is the most
natural choice. We could easily modify the part of the code in my
patch that warns about self-initialization (of global variables, local
variables, and class members) to be controlled under -Winit-self. And
in that case I think that -Winit-self should not be gated by
-Wuninitialized.

>
> I think this warning (except for auto variables) should be turned on with -Wall.

I am all for that. In fact, if we are going to make -Winit-self
independent of -Wuninitialized, I would think that -Winit-self should
also be enabled with -Wall.

>>        (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.
>
> This seems wrong and really bad idea.  There seems like you should
> have a better way of implementing this without this much extra code to
> fold-const.c which is called all through out the middle-end.  I think
> if you do add this code, I think you should do some compile time
> comparison to make sure it does not slow down the compiler this much.

The reason why I had to add a flag in expr and set it after folding an
expression is because the C frontend is too eager to constant-fold an
expression at parse time. For example, when parsing an assignment x =
x + 0 (which we don't want to warn under -Wself-assign), what we get
after paring the statement is already x = x (while, on the other hand,
C++ parser would return x = x + 0 first, and then call the folding
routines). While I would welcome any other better ideas, I did some
compile time tests with my patch and the results show that the code
does not slow down the compiler. Diego pointed me to the GCC
PerformanceTesting wiki page that contains some compile-time test
instructions, but there are too many packages to download and set up,
so I picked some of the largest GCC source files to measure the
compile time with and without this patch. Here are the results:

The source trees were sync'ed to revision 160300. The experiments were
conducted on an Intel Core2 Duo (2.4 GHz) system with 8GB of memory
running Ubuntu (Linux kernel 2.6.24). The data shown below are
user+system time in seconds. Each number is the average of 3 runs. The
first column (New) shows the time with the patch, the second column
(Old)  without, and the third column (Diff) the diff percentage. (Note
the source files were all preprocessed before measuring the compile
time.)

                New     Old     Diff
-O1
combine.c       4.29    4.30    -0.06%
expr.c          4.77    4.81    -0.72%
fold-const.c   16.13   16.19    -0.37%
parser.c        5.90    5.91    -0.18%
pt.c           12.70   12.63     0.52%

-O2
combine.c       6.01    6.01    -0.02%
expr.c          6.51    6.43     1.14%
fold-const.c   22.35   22.45    -0.46%
parser.c        8.09    8.11    -0.25%
pt.c           16.65   16.74    -0.53%

-O3
combine.c       7.14    7.22    -1.07%
expr.c          7.71    7.73    -0.28%
fold-const.c   26.22   26.32    -0.40%
parser.c       10.02   10.02    -0.05%
pt.c           20.23   20.25    -0.11%


The compile time for both cases are practically the same (although
somehow with my patch the compile time appears to be slightly better,
which I don't know why. :-))). If there are any other publicly
available source files that you think I should measure their compile
time, I'd be happy to do it.

Thanks,

Le-chun

Re: [patch] Add a new warning flag -Wself-assign

[Le-Chun Wu]

Dave,

On Fri, May 28, 2010 at 5:24 PM, Dave Korn <dave.korn.cygwin@gmail.com> wrote:
> On 29/05/2010 00:47, Le-Chun Wu wrote:
>
>  Hello,
>
>> This patch adds a new warnings flag "-Wself-assign" that warns about
>> self-assignment (including self-initialization). This warning is
>> intended for detecting accidental self-assignment due to typos, and
>> therefore does not warn on a statement that is semantically a
>> self-assignment after constant folding. Here is an example of what
>> will trigger a self-assign warning and what will not:
>>
>>           void func()
>>           {
>>              int i = 2;
>>              int x = x;   /* warn */
>>              float f = 5.0;
>>              double a[3];
>>
>>              i = i + 0;   /* not warn */
>>              f = f / 1;   /* not warn */
>>              a[1] = a[1]; /* warn */
>>              i += 0;      /* not warn */
>>           }
>
>  It is a very common idiom to write:
>
>> <returntype> f (<type> x)
>> {
>>   x = x;
>>   [ ... do other stuff ... ]
>>   return [some value or none]
>> }
>
> ... in order try and avoid parameter-unused warnings in a portable fashion;
> how does your patch react to this technique?
>

Our recommendation is to use `(void) x' or `static_cast<void>(x)'
instead, which should also be portable. (I actually added this
recommendation in the documentation of -Wunused flag in my patch.)

Thanks,

Le-chun

Re: [patch] Add a new warning flag -Wself-assign

[Dave Korn]

On 09/06/2010 19:31, Le-Chun Wu wrote:

> 
> Our recommendation is to use `(void) x' or `static_cast<void>(x)'
> instead, which should also be portable. (I actually added this
> recommendation in the documentation of -Wunused flag in my patch.)

  So you did!  Sorry for overlooking that.

    cheers,
      DaveK

Re: [patch] Add a new warning flag -Wself-assign

[Andrew Pinski]

On Wed, Jun 9, 2010 at 11:25 AM, Le-Chun Wu <lcwu@google.com> wrote:
> It should be pretty easy to modify this patch to not warn about
> self-initialization of local variables with this flag. However, what I
> don't understand is why a user would want to work around (or suppress)
> an uninitialized warning by doing self-initialization. They could have
> just initialized a local variable to some other more meaningful value.
> Initializing a variable with itself doesn't seem like a good way to
> fix an uninitialized warning.

Well this is the documented way of disabling the warning.   If we want
to change this, we should get rid of -Winit-self at the same time.
And remove the documentation for this special case.  I don't remember
the full history behind my change of the documentation to mention this
special case but from what I remember the source at one point did not
warn about "int t = t +1;".  And that I was fixing that case with
keeping the special case of "int i = i;".

Thanks,
Andrew Pinski

Re: [patch] Add a new warning flag -Wself-assign

[Andrew Pinski]

On Wed, Jun 9, 2010 at 11:25 AM, Le-Chun Wu <lcwu@google.com> wrote:
> The reason why I had to add a flag in expr and set it after folding an
> expression is because the C frontend is too eager to constant-fold an
> expression at parse time. For example, when parsing an assignment x =
> x + 0 (which we don't want to warn under -Wself-assign), what we get
> after paring the statement is already x = x (while, on the other hand,
> C++ parser would return x = x + 0 first, and then call the folding
> routines). While I would welcome any other better ideas, I did some
> compile time tests with my patch and the results show that the code
> does not slow down the compiler.

I think we should first decide what interactions we want for this
option vs -Winit-self.  Because adding a new option seems over kill
and wrong.  It would cause more confusion to our users.  In fact maybe
we should just get rid of the special case of -Winit-self and improve
the C++ front-end where -Wself-asign would warn more (the bug report I
mentioned before for an example).

And then we remove the issue I have with fold.  The special case is
implemented one place in c_gimplify_expr.  In fact "int x = x+0;" will
cause right now the unitialized warning for x to be disabled (without
-Winit-self) because of the folding.

Thanks,
Andrew Pinski

Re: [patch] Add a new warning flag -Wself-assign

[Le-Chun Wu]

Eric,

Thanks for the comments. And yes, I know the change logs would go in
different subdirectories and the prefix is not needed. What I normally
did in the past (and found some of the other people also do the same)
is to send out an aggregated ChangeLog for review. And after the patch
is approved, I will split it up, remove the unnecessary prefix, and
submit them to their respective locations. That being said, when I
send out a revised patch later, I will split up the ChangeLog.

Thanks,

Le-chun

On Sat, May 29, 2010 at 12:31 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> 2010-05-28  Le-Chun Wu  <lcwu@google.com>
>>
>>         * gcc/doc/invoke.texi: Documentation for -Wself-assign.
>>         * gcc/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. * gcc/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.
>>         * gcc/testsuite/gcc.dg/plugin/selfassign.c (check_self_assign):
>>         Renamed from warn_self_assign.
>>         * gcc/testsuite/gcc.dg/wself-assign-1.c: New test.
>>         * gcc/testsuite/gcc.dg/wself-assign-2.c: New test.
>>         * gcc/testsuite/g++.dg/plugin/selfassign.c (check_self_assign):
>>         Renamed from warn_self_assign.
>>         * gcc/testsuite/g++.dg/warn/Wself-assign-1.C: New test.
>>         * gcc/testsuite/g++.dg/warn/Wself-assign-2.C: New test.
>>         * gcc/testsuite/g++.dg/warn/Wself-assign-3.C: New test.
>>         * gcc/testsuite/g++.dg/warn/Wself-assign-4.C: New test.
>>         * gcc/cp/init.c (perform_member_init): Check for self-assign after
>>         parsing class member initialization.
>>         * gcc/cp/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/common.opt: Add -Wself-assign.
>>         * gcc/c-common.c (check_for_self_assign): New function that checks
>> for self-assignment.
>>         * gcc/c-common.h: Add prototype for check_for_self_assign.
>>         * gcc/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.
>
> Most of this ChangeLog will go in gcc/ChangeLog so no gcc/ prefix.
>
> gcc/cp has its own ChangeLog so:
>
> cp/
>        * 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 has its own ChangeLog so:
>
> testsuite/
>        * 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.
>        * 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.
>
>
> --
> Eric Botcazou
>

Re: [patch] Add a new warning flag -Wself-assign

[Manuel López-Ibáñez]

On 9 June 2010 20:32, Andrew Pinski <pinskia@gmail.com> wrote:
> On Wed, Jun 9, 2010 at 11:25 AM, Le-Chun Wu <lcwu@google.com> wrote:
>> It should be pretty easy to modify this patch to not warn about
>> self-initialization of local variables with this flag. However, what I
>> don't understand is why a user would want to work around (or suppress)
>> an uninitialized warning by doing self-initialization. They could have
>> just initialized a local variable to some other more meaningful value.
>> Initializing a variable with itself doesn't seem like a good way to
>> fix an uninitialized warning.
>
> Well this is the documented way of disabling the warning.   If we want
> to change this, we should get rid of -Winit-self at the same time.
> And remove the documentation for this special case.  I don't remember
> the full history behind my change of the documentation to mention this
> special case but from what I remember the source at one point did not
> warn about "int t = t +1;".  And that I was fixing that case with
> keeping the special case of "int i = i;".

That is broken in C++ anyway http://gcc.gnu.org/PR34772, so who cares?
Let's get rid of this special case for Wuninitialized, get rid of
-Winit-self and let -Wself-assign warn for everything.

I would recommend that you add [C/C++] to the subject and/or CC
relevant maintainers.

The idea of the patch seems ok to me but the implementation seems
expensive. Adding yet another bit to one of the most used parts of the
whole compiler and the whole folding wrap-up are a bit scary. But I am
sure maintainers can give a more concrete answer and suggest
alternative implementations.

One thing:

+    warning_at (location, OPT_Wself_assign, G_("%qE is assigned to itself"),
+                lhs);

You do not need G_() here.


+Wself-assign
+Common Var(warn_self_assign) Init(0) Warning
+Warn when a variable is assigned to itself
This is not common but only C/C++, it should go in c.opt


Cheers,

Manuel.

Re: [patch] Add a new warning flag -Wself-assign

[Nathan Froyd]

On Wed, Jun 09, 2010 at 11:43:01AM -0700, Le-Chun Wu wrote:
> Thanks for the comments. And yes, I know the change logs would go in
> different subdirectories and the prefix is not needed. What I normally
> did in the past (and found some of the other people also do the same)
> is to send out an aggregated ChangeLog for review. And after the patch
> is approved, I will split it up, remove the unnecessary prefix, and
> submit them to their respective locations. That being said, when I
> send out a revised patch later, I will split up the ChangeLog.

It might be easier to make sure all the ChangeLog entries are in the
correct place first and then aggregate them all for patch submission.
That way you won't make any errors with cut-and-paste etc.  I use the
following script for nicely-formatted multi-directory patches:

#!/bin/bash

for dir in "$@"
do
    echo $dir/
    log=$(svn status $dir/ChangeLog* | egrep '^M'|tail -n 1 | awk '{print $2}')
    cl-head $log
    echo
done

Of course, you need cl-head as well, which is a little rough and doesn't
quite work for multi-author patches, but it's pretty close:

#!/usr/bin/gawk -f

BEGIN {
  printing_log = 0;             # whether we are printing an entry
  holding_blank_line = 0;       # whether previous line was a blank line
}

# We start printing at the first bullet or after the first blank line.
/^[[:space:]]+./ {
  if (!printing_log)
  {
    printing_log = 1;
    holding_blank_line = 0;
  }

  if (printing_log)
  {
    if (holding_blank_line)
    {
      print "";
      holding_blank_line = 0;
    }
    print;
  }
}

# We wait to output blank lines because there's a blank line at the end
# of every entry and we don't want to stick that into our log message.
/^$/ { holding_blank_line = 1; }

# A line with non-whitespace in the first column is the start of the next
# entry.  So we can stop if we were printing the log.
/^[^[:space:]]/ { if (printing_log) exit; }

With both of these, you can simply invoke:

  multi-dir-cl-head gcc gcc/cp gcc/testsuite

and get the desired output:

gcc/
	* file.c (function): Message.

gcc/cp/
	* file2.c (function2): Likewise.

gcc/testsuite/
	* file3.c (function3): Likewise.

Extending multi-dir-cl-head so that it does the right thing from 'svn
status' or similar is left as an exercise for the reader. :)

HTH,
-Nathan

Re: [patch] Add a new warning flag -Wself-assign

[Le-Chun Wu]

On Wed, Jun 9, 2010 at 11:42 AM, Andrew Pinski <pinskia@gmail.com> wrote:
> On Wed, Jun 9, 2010 at 11:25 AM, Le-Chun Wu <lcwu@google.com> wrote:
>> The reason why I had to add a flag in expr and set it after folding an
>> expression is because the C frontend is too eager to constant-fold an
>> expression at parse time. For example, when parsing an assignment x =
>> x + 0 (which we don't want to warn under -Wself-assign), what we get
>> after paring the statement is already x = x (while, on the other hand,
>> C++ parser would return x = x + 0 first, and then call the folding
>> routines). While I would welcome any other better ideas, I did some
>> compile time tests with my patch and the results show that the code
>> does not slow down the compiler.
>
> I think we should first decide what interactions we want for this
> option vs -Winit-self.  Because adding a new option seems over kill
> and wrong.  It would cause more confusion to our users.  In fact maybe
> we should just get rid of the special case of -Winit-self and improve
> the C++ front-end where -Wself-asign would warn more (the bug report I
> mentioned before for an example).

How about getting rid of -Winit-self and having -Wself-assign warn for
everything as Manuel suggested? BTW, my patch already warns about the
example in PR/PR18016.

$ cat self-assign-PR18016.C
class A
{
  public:
    A() : a(a)  // <-- should generate a warning
    {}
    int getA()
    {
      int b = b;  // <-- generates warning as it should
      return this->a + b;
    }

  private:
    int a;
};

int main()
{
    A a;
    return a.getA();
}

$ g++ -c self-assign-PR18016.C -Wself-assign
self-assign-PR18016.C: In constructor ‘A::A()’:
self-assign-PR18016.C:4:14: warning: ‘((A*)this)->A::a’ is assigned to
itself [-Wself-assign]
self-assign-PR18016.C: In member function ‘int A::getA()’:
self-assign-PR18016.C:8:15: warning: ‘b’ is assigned to itself [-Wself-assign]

>
> And then we remove the issue I have with fold.  The special case is
> implemented one place in c_gimplify_expr.  In fact "int x = x+0;" will
> cause right now the unitialized warning for x to be disabled (without
> -Winit-self) because of the folding.
>

If we agree on removing -Winit-self, I will remove the code in
c_gimplify_expr (and other places), update the documentation, and send
out a revised patch.

Thanks,

Le-chun

Re: [patch] Add a new warning flag -Wself-assign

[Le-Chun Wu]

On Wed, Jun 9, 2010 at 11:56 AM, Manuel López-Ibáñez
<lopezibanez@gmail.com> wrote:
> On 9 June 2010 20:32, Andrew Pinski <pinskia@gmail.com> wrote:
>> On Wed, Jun 9, 2010 at 11:25 AM, Le-Chun Wu <lcwu@google.com> wrote:
>>> It should be pretty easy to modify this patch to not warn about
>>> self-initialization of local variables with this flag. However, what I
>>> don't understand is why a user would want to work around (or suppress)
>>> an uninitialized warning by doing self-initialization. They could have
>>> just initialized a local variable to some other more meaningful value.
>>> Initializing a variable with itself doesn't seem like a good way to
>>> fix an uninitialized warning.
>>
>> Well this is the documented way of disabling the warning.   If we want
>> to change this, we should get rid of -Winit-self at the same time.
>> And remove the documentation for this special case.  I don't remember
>> the full history behind my change of the documentation to mention this
>> special case but from what I remember the source at one point did not
>> warn about "int t = t +1;".  And that I was fixing that case with
>> keeping the special case of "int i = i;".
>
> That is broken in C++ anyway http://gcc.gnu.org/PR34772, so who cares?
> Let's get rid of this special case for Wuninitialized, get rid of
> -Winit-self and let -Wself-assign warn for everything.
>
> I would recommend that you add [C/C++] to the subject and/or CC
> relevant maintainers.

Will do when I send out the revised patch. Looking at MAINTAINERS
files, I couldn't  figure out who the relevant maintainers are for
constant-folding code. Should they be middle-end or C/C++ frontend
maintainers?

>
> The idea of the patch seems ok to me but the implementation seems
> expensive. Adding yet another bit to one of the most used parts of the
> whole compiler and the whole folding wrap-up are a bit scary. But I am
> sure maintainers can give a more concrete answer and suggest
> alternative implementations.

While my experiments appear to suggest there should be no compile-time
implications with this patch, I do welcome alternative ideas. Let's
see what other people/maintainers say about it.

>
> One thing:
>
> +    warning_at (location, OPT_Wself_assign, G_("%qE is assigned to itself"),
> +                lhs);
>
> You do not need G_() here.

Will fix.

>
>
> +Wself-assign
> +Common Var(warn_self_assign) Init(0) Warning
> +Warn when a variable is assigned to itself
> This is not common but only C/C++, it should go in c.opt

Will fix.

Thanks,

Le-chun

Re: [patch] Add a new warning flag -Wself-assign

[Le-Chun Wu]

Nathan,

This looks like a useful tool. Thanks for sharing.

Le-chun

On Wed, Jun 9, 2010 at 12:01 PM, Nathan Froyd <froydnj@codesourcery.com> wrote:
> On Wed, Jun 09, 2010 at 11:43:01AM -0700, Le-Chun Wu wrote:
>> Thanks for the comments. And yes, I know the change logs would go in
>> different subdirectories and the prefix is not needed. What I normally
>> did in the past (and found some of the other people also do the same)
>> is to send out an aggregated ChangeLog for review. And after the patch
>> is approved, I will split it up, remove the unnecessary prefix, and
>> submit them to their respective locations. That being said, when I
>> send out a revised patch later, I will split up the ChangeLog.
>
> It might be easier to make sure all the ChangeLog entries are in the
> correct place first and then aggregate them all for patch submission.
> That way you won't make any errors with cut-and-paste etc.  I use the
> following script for nicely-formatted multi-directory patches:
>
> #!/bin/bash
>
> for dir in "$@"
> do
>    echo $dir/
>    log=$(svn status $dir/ChangeLog* | egrep '^M'|tail -n 1 | awk '{print $2}')
>    cl-head $log
>    echo
> done
>
> Of course, you need cl-head as well, which is a little rough and doesn't
> quite work for multi-author patches, but it's pretty close:
>
> #!/usr/bin/gawk -f
>
> BEGIN {
>  printing_log = 0;             # whether we are printing an entry
>  holding_blank_line = 0;       # whether previous line was a blank line
> }
>
> # We start printing at the first bullet or after the first blank line.
> /^[[:space:]]+./ {
>  if (!printing_log)
>  {
>    printing_log = 1;
>    holding_blank_line = 0;
>  }
>
>  if (printing_log)
>  {
>    if (holding_blank_line)
>    {
>      print "";
>      holding_blank_line = 0;
>    }
>    print;
>  }
> }
>
> # We wait to output blank lines because there's a blank line at the end
> # of every entry and we don't want to stick that into our log message.
> /^$/ { holding_blank_line = 1; }
>
> # A line with non-whitespace in the first column is the start of the next
> # entry.  So we can stop if we were printing the log.
> /^[^[:space:]]/ { if (printing_log) exit; }
>
> With both of these, you can simply invoke:
>
>  multi-dir-cl-head gcc gcc/cp gcc/testsuite
>
> and get the desired output:
>
> gcc/
>        * file.c (function): Message.
>
> gcc/cp/
>        * file2.c (function2): Likewise.
>
> gcc/testsuite/
>        * file3.c (function3): Likewise.
>
> Extending multi-dir-cl-head so that it does the right thing from 'svn
> status' or similar is left as an exercise for the reader. :)
>
> HTH,
> -Nathan
>

Re: [patch] Add a new warning flag -Wself-assign

[Gabriel Dos Reis]

On Wed, Jun 9, 2010 at 1:25 PM, Le-Chun Wu <lcwu@google.com> wrote:
>
> The reason why I had to add a flag in expr and set it after folding an
> expression is because the C frontend is too eager to constant-fold an
> expression at parse time. For example, when parsing an assignment x =
> x + 0 (which we don't want to warn under -Wself-assign), what we get
> after paring the statement is already x = x (while, on the other hand,
> C++ parser would return x = x + 0 first, and then call the folding
> routines). While I would welcome any other better ideas, I did some
> compile time tests with my patch and the results show that the code
> does not slow down the compiler. Diego pointed me to the GCC
> PerformanceTesting wiki page that contains some compile-time test
> instructions, but there are too many packages to download and set up,
> so I picked some of the largest GCC source files to measure the
> compile time with and without this patch. Here are the results:

How do you plan handle

    void* p = &p;

?

Re: [patch] Add a new warning flag -Wself-assign

[Le-Chun Wu]

On Wed, Jun 9, 2010 at 6:55 PM, Gabriel Dos Reis
<gdr@integrable-solutions.net> wrote:
> On Wed, Jun 9, 2010 at 1:25 PM, Le-Chun Wu <lcwu@google.com> wrote:
>>
>> The reason why I had to add a flag in expr and set it after folding an
>> expression is because the C frontend is too eager to constant-fold an
>> expression at parse time. For example, when parsing an assignment x =
>> x + 0 (which we don't want to warn under -Wself-assign), what we get
>> after paring the statement is already x = x (while, on the other hand,
>> C++ parser would return x = x + 0 first, and then call the folding
>> routines). While I would welcome any other better ideas, I did some
>> compile time tests with my patch and the results show that the code
>> does not slow down the compiler. Diego pointed me to the GCC
>> PerformanceTesting wiki page that contains some compile-time test
>> instructions, but there are too many packages to download and set up,
>> so I picked some of the largest GCC source files to measure the
>> compile time with and without this patch. Here are the results:
>
> How do you plan handle
>
>    void* p = &p;
>
> ?
>

The new flag doesn't warn about this case. While this code looks
dubious (and the compiler probably should emit a warning), it is
strictly-speaking not a self-assignment or self-initialization as it
assigns p's address to p. If we are going to warn about it, the
warning should probably not be under -Wself-assign.

Thanks,

Le-chun

Re: [patch] Add a new warning flag -Wself-assign

[Manuel López-Ibáñez]

On 10 June 2010 02:58, Le-Chun Wu <lcwu@google.com> wrote:
>>
>> I would recommend that you add [C/C++] to the subject and/or CC
>> relevant maintainers.
>
> Will do when I send out the revised patch. Looking at MAINTAINERS
> files, I couldn't  figure out who the relevant maintainers are for
> constant-folding code. Should they be middle-end or C/C++ frontend
> maintainers?

Probably any or both. Adding middle-end cannot hurt. Patches are more
likely to get approved if they touch only one maintainers area at a
time, so if you do not succeed, breaking up the patch in C and C++
parts could help.

> While my experiments appear to suggest there should be no compile-time
> implications with this patch, I do welcome alternative ideas. Let's
> see what other people/maintainers say about it.

I think the relevant maintainers probably haven't even noticed this
patch because it seems from the subject a diagnostic issue.

What about the revised patch?

Cheers,

Manuel.

Re: [patch] Add a new warning flag -Wself-assign

[Gabriel Dos Reis]

On Thu, Jun 10, 2010 at 12:32 PM, Le-Chun Wu <lcwu@google.com> wrote:
> On Wed, Jun 9, 2010 at 6:55 PM, Gabriel Dos Reis
> <gdr@integrable-solutions.net> wrote:
>> On Wed, Jun 9, 2010 at 1:25 PM, Le-Chun Wu <lcwu@google.com> wrote:
>>>
>>> The reason why I had to add a flag in expr and set it after folding an
>>> expression is because the C frontend is too eager to constant-fold an
>>> expression at parse time. For example, when parsing an assignment x =
>>> x + 0 (which we don't want to warn under -Wself-assign), what we get
>>> after paring the statement is already x = x (while, on the other hand,
>>> C++ parser would return x = x + 0 first, and then call the folding
>>> routines). While I would welcome any other better ideas, I did some
>>> compile time tests with my patch and the results show that the code
>>> does not slow down the compiler. Diego pointed me to the GCC
>>> PerformanceTesting wiki page that contains some compile-time test
>>> instructions, but there are too many packages to download and set up,
>>> so I picked some of the largest GCC source files to measure the
>>> compile time with and without this patch. Here are the results:
>>
>> How do you plan handle
>>
>>    void* p = &p;
>>
>> ?
>>
>
> The new flag doesn't warn about this case.

Good.

> While this code looks
> dubious (and the compiler probably should emit a warning), it is
> strictly-speaking not a self-assignment or self-initialization as it
> assigns p's address to p. If we are going to warn about it, the
> warning should probably not be under -Wself-assign.

I would not recommend warning about it.

>
> Thanks,
>
> Le-chun
>

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

[Le-Chun Wu]

Hi,

Here is the revised patch for the implementation of -Wself-assign
warning. Besides minor changes that address the review comments, the
major differences of this revised patch from the original one are (1)
-Wself-assign is enabled by -Wall, and (2) -Winit-self flag is
removed.

(For the discussion thread of the original patch, please see
http://gcc.gnu.org/ml/gcc-patches/2010-06/msg00974.html)

Bootstrapped and tested on x86_64-linux-gnu. OK for trunk?

Thanks,

Le-chun

2010-06-23  Le-Chun Wu  <lcwu@google.com>

	* gcc/doc/invoke.texi (-Wall): Include -Wself-assign.
	(-Wself-assign): New documentation for the flag.
	(-Winit-self): Removed.
	(-Wuninitialized): Remove mention of -Winit-self.
	(-Wunused-variable): Mention of new workaround.
	* gcc/c-family/c-gimplify.c (c_gimplify_expr): Remove support for
	-Winit-self.
	* gcc/c-family/c.opt: Remove -Winit-self 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/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.
	* gcc/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.
	* gcc/testsuite/gcc.dg/plugin/selfassign.c (check_self_assign):
	Renamed from warn_self_assign.
	* gcc/testsuite/gcc.dg/wself-assign-1.c: New test.
	* gcc/testsuite/gcc.dg/wself-assign-2.c: New test.
	* gcc/testsuite/gcc.dg/wself-assign-3.c: New test.
	* gcc/testsuite/gcc.dg/uninit-D-O0.c: Removed.
	* gcc/testsuite/gcc.dg/uninit-D.c: Removed.
	* gcc/testsuite/gcc.dg/uninit-E-O0.c: Removed.
	* gcc/testsuite/gcc.dg/uninit-E.c: Removed.
	* gcc/testsuite/g++.dg/plugin/selfassign.c (check_self_assign):
	Renamed from warn_self_assign.
	* gcc/testsuite/g++.dg/warn/Wself-assign-1.C: New test.
	* gcc/testsuite/g++.dg/warn/Wself-assign-2.C: New test.
	* gcc/testsuite/g++.dg/warn/Wself-assign-3.C: New test.
	* gcc/testsuite/g++.dg/warn/Wself-assign-4.C: New test.
	* gcc/testsuite/g++.dg/warn/Wself-assign-5.C: New test.
	* gcc/cp/init.c (perform_member_init): Check for self-assign after
	parsing class member initialization.
	* gcc/cp/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/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.


Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 161236)
+++ gcc/doc/invoke.texi	(working copy)
@@ -242,8 +242,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wformat-security  -Wformat-y2k @gol
 -Wframe-larger-than=@var{len} -Wjump-misses-init -Wignored-qualifiers @gol
 -Wimplicit  -Wimplicit-function-declaration  -Wimplicit-int @gol
--Winit-self  -Winline @gol
--Wno-int-to-pointer-cast -Wno-invalid-offsetof @gol
+-Winline -Wno-int-to-pointer-cast -Wno-invalid-offsetof @gol
 -Winvalid-pch -Wlarger-than=@var{len}  -Wunsafe-loop-optimizations @gol
 -Wlogical-op -Wlong-long @gol
 -Wmain  -Wmissing-braces  -Wmissing-field-initializers @gol
@@ -254,7 +253,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wparentheses  -Wpedantic-ms-format -Wno-pedantic-ms-format @gol
 -Wpointer-arith  -Wno-pointer-to-int-cast @gol
 -Wredundant-decls @gol
--Wreturn-type  -Wsequence-point  -Wshadow @gol
+-Wreturn-type -Wself-assign -Wsequence-point  -Wshadow @gol
 -Wsign-compare  -Wsign-conversion  -Wstack-protector @gol
 -Wstrict-aliasing -Wstrict-aliasing=n @gol
 -Wstrict-overflow -Wstrict-overflow=@var{n} @gol
@@ -2943,6 +2942,7 @@ Options} and @ref{Objective-C and Object
 -Wpointer-sign  @gol
 -Wreorder   @gol
 -Wreturn-type  @gol
+-Wself-assign @r{(only in C/C++)}  @gol
 -Wsequence-point  @gol
 -Wsign-compare @r{(only in C++)}  @gol
 -Wstrict-aliasing  @gol
@@ -3143,24 +3143,6 @@ requiring a non-null value by the @code{
 @option{-Wnonnull} is included in @option{-Wall} and @option{-Wformat}.  It
 can be disabled with the @option{-Wno-nonnull} option.

-@item -Winit-self @r{(C, C++, Objective-C and Objective-C++ only)}
-@opindex Winit-self
-@opindex Wno-init-self
-Warn about uninitialized variables which are initialized with themselves.
-Note this option can only be used with the @option{-Wuninitialized} option.
-
-For example, GCC will warn about @code{i} being uninitialized in the
-following snippet only when @option{-Winit-self} has been specified:
-@smallexample
-@group
-int f()
-@{
-  int i = i;
-  return i;
-@}
-@end group
-@end smallexample
-
 @item -Wimplicit-int @r{(C and Objective-C only)}
 @opindex Wimplicit-int
 @opindex Wno-implicit-int
@@ -3323,6 +3305,53 @@ definitions, may be found on the GCC rea

 This warning is enabled by @option{-Wall} for C and C++.

+@item -Wself-assign
+@opindex Wself-assign
+@opindex Wno-self-assign
+Warn about self-assignment and self-initialization. This warning is intended
+for detecting accidental self-assignment due to typos, and therefore does
+not warn on a statement that is semantically a self-assignment after
+constant folding. Here is an example of what will trigger a self-assign
+warning and what will not:
+
+@smallexample
+@group
+void func()
+@{
+   int i = 2;
+   int x = x;   /* warn */
+   float f = 5.0;
+   double a[3];
+
+   i = i + 0;   /* not warn */
+   f = f / 1;   /* not warn */
+   a[1] = a[1]; /* warn */
+   i += 0;      /* not warn */
+@}
+@end group
+@end smallexample
+
+There are cases where self-assignment might be intentional. For example,
+a C++ programmers might write code to test whether an overloaded
+@code{operator=} works when the same object is assigned to itself.
+One way to work around the self-assign warning in such case is using
+the functional form @code{object.operator=(object)} instead of the
+assignment form @code{object = object}, as shown in the following example.
+
+@smallexample
+@group
+void test_func()
+@{
+   MyType t;
+
+   t.operator=(t);  // not warn
+   t = t;           // warn
+@}
+@end group
+@end smallexample
+
+This warning is enabled by @option{-Wall} for C and C++.
+
 @item -Wreturn-type
 @opindex Wreturn-type
 @opindex Wno-return-type
@@ -3446,6 +3475,10 @@ This warning is enabled by @option{-Wall
 To suppress this warning use the @samp{unused} attribute
 (@pxref{Variable Attributes}).

+Note that a classic way to avoid @option{-Wunused-variable} warning is
+using @code{x = x}, but that does not work with @option{-Wself-assign}.
+Use @code{(void) x} or @code{static_cast<void>(x)} instead.
+
 @item -Wunused-value
 @opindex Wunused-value
 @opindex Wno-unused-value
@@ -3475,9 +3508,6 @@ or if a variable may be clobbered by a @
 warn if a non-static reference or non-static @samp{const} member
 appears in a class without constructors.

-If you want to warn about code which uses the uninitialized value of the
-variable in its own initializer, use the @option{-Winit-self} option.
-
 These warnings occur for individual uninitialized or clobbered
 elements of structure, union or array variables as well as for
 variables which are uninitialized or clobbered as a whole.  They do
Index: gcc/c-family/c-gimplify.c
===================================================================
--- gcc/c-family/c-gimplify.c	(revision 161236)
+++ gcc/c-family/c-gimplify.c	(working copy)
@@ -173,18 +173,5 @@ int
 c_gimplify_expr (tree *expr_p, gimple_seq *pre_p ATTRIBUTE_UNUSED,
 		 gimple_seq *post_p ATTRIBUTE_UNUSED)
 {
-  enum tree_code code = TREE_CODE (*expr_p);
-
-  /* This is handled mostly by gimplify.c, but we have to deal with
-     not warning about int x = x; as it is a GCC extension to turn off
-     this warning but only if warn_init_self is zero.  */
-  if (code == DECL_EXPR
-      && TREE_CODE (DECL_EXPR_DECL (*expr_p)) == VAR_DECL
-      && !DECL_EXTERNAL (DECL_EXPR_DECL (*expr_p))
-      && !TREE_STATIC (DECL_EXPR_DECL (*expr_p))
-      && (DECL_INITIAL (DECL_EXPR_DECL (*expr_p)) == DECL_EXPR_DECL (*expr_p))
-      && !warn_init_self)
-    TREE_NO_WARNING (DECL_EXPR_DECL (*expr_p)) = 1;
-
   return GS_UNHANDLED;
 }
Index: gcc/c-family/c.opt
===================================================================
--- gcc/c-family/c.opt	(revision 161236)
+++ gcc/c-family/c.opt	(working copy)
@@ -258,10 +258,6 @@ Wignored-qualifiers
 C C++ Var(warn_ignored_qualifiers) Init(-1) Warning
 Warn whenever type qualifiers are ignored.

-Winit-self
-C ObjC C++ ObjC++ Var(warn_init_self) Warning
-Warn about variables which are initialized to themselves
-
 Wimplicit
 C ObjC Var(warn_implicit) Init(-1) Warning
 Warn about implicit declarations
@@ -425,6 +421,10 @@ Wreturn-type
 C ObjC C++ ObjC++ Var(warn_return_type) Warning
 Warn whenever a function's return type defaults to \"int\" (C), or
about inconsistent return types (C++)

+Wself-assign
+C C++ Var(warn_self_assign) Init(0) Warning
+Warn when a variable is assigned to itself
+
 Wselector
 ObjC ObjC++ Var(warn_selector) Warning
 Warn if a selector has multiple methods
Index: gcc/c-family/c-opts.c
===================================================================
--- 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 ())
 	{
Index: gcc/c-family/c-common.c
===================================================================
--- gcc/c-family/c-common.c	(revision 161236)
+++ gcc/c-family/c-common.c	(working copy)
@@ -9266,4 +9266,20 @@ make_tree_vector_copy (const VEC(tree,gc
   return ret;
 }

+/* Check for and warn about self-assignment or self-initialization.
+   LHS and RHS are the tree nodes for the left-hand side and right-hand side
+   of the assignment or initialization we are checking.
+   LOCATION is the source location for RHS.  */
+
+void
+check_for_self_assign (location_t location, tree lhs, tree rhs)
+{
+  /* Only emit a warning if RHS is not a folded expression so that we don't
+     warn on something like x = x / 1.  */
+  if (!EXPR_FOLDED (rhs)
+      && operand_equal_p (lhs, rhs,
+                          OEP_PURE_SAME | OEP_ALLOW_NULL |
OEP_ALLOW_NULL_TYPE))
+    warning_at (location, OPT_Wself_assign, "%qE is assigned to itself", lhs);
+}
+
 #include "gt-c-family-c-common.h"
Index: gcc/c-family/c-common.h
===================================================================
--- gcc/c-family/c-common.h	(revision 161236)
+++ gcc/c-family/c-common.h	(working copy)
@@ -893,6 +893,7 @@ extern VEC(tree,gc) *make_tree_vector (v
 extern void release_tree_vector (VEC(tree,gc) *);
 extern VEC(tree,gc) *make_tree_vector_single (tree);
 extern VEC(tree,gc) *make_tree_vector_copy (const VEC(tree,gc) *);
+extern void check_for_self_assign (location_t, tree, tree);

 /* In c-gimplify.c  */
 extern void c_genericize (tree);
Index: gcc/tree.h
===================================================================
--- gcc/tree.h	(revision 161236)
+++ gcc/tree.h	(working copy)
@@ -388,8 +388,9 @@ struct GTY(()) tree_base {
   unsigned visited : 1;
   unsigned packed_flag : 1;
   unsigned user_align : 1;
+  unsigned expr_folded_flag : 1;

-  unsigned spare : 13;
+  unsigned spare : 12;

   /* This field is only used with type nodes; the only reason it is present
      in tree_base instead of tree_type is to save space.  The size of the
@@ -627,6 +628,13 @@ struct GTY(()) tree_common {

        SSA_NAME_IS_DEFAULT_DEF in
            SSA_NAME
+
+   expr_folded_flag:
+
+       EXPR_FOLDED in
+           all expressions
+           all decls
+           all constants
 */

 #undef DEFTREESTRUCT
@@ -1363,6 +1371,10 @@ extern void omp_clause_range_check_faile
 /* In fixed-point types, means a saturating type.  */
 #define TYPE_SATURATING(NODE) ((NODE)->base.saturating_flag)

+/* Nonzero in an expression, a decl, or a constant node if the node is
+   the result of a successful constant-folding.  */
+#define EXPR_FOLDED(NODE) ((NODE)->base.expr_folded_flag)
+
 /* These flags are available for each language front end to use internally.  */
 #define TREE_LANG_FLAG_0(NODE) ((NODE)->base.lang_flag_0)
 #define TREE_LANG_FLAG_1(NODE) ((NODE)->base.lang_flag_1)
@@ -4931,7 +4943,9 @@ extern bool fold_deferring_overflow_warn
 enum operand_equal_flag
 {
   OEP_ONLY_CONST = 1,
-  OEP_PURE_SAME = 2
+  OEP_PURE_SAME = 2,
+  OEP_ALLOW_NULL = 4,      /* Allow comparison of NULL operands.  */
+  OEP_ALLOW_NULL_TYPE = 8  /* Allow comparison of operands without types.  */
 };

 extern int operand_equal_p (const_tree, const_tree, unsigned int);
Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c	(revision 161236)
+++ gcc/fold-const.c	(working copy)
@@ -2404,22 +2404,45 @@ combine_comparisons (location_t loc,

    If OEP_PURE_SAME is set, then pure functions with identical arguments
    are considered the same.  It is used when the caller has other ways
-   to ensure that global memory is unchanged in between.  */
+   to ensure that global memory is unchanged in between.
+
+   If OEP_ALLOW_NULL is set, this routine will not crash on NULL operands,
+   and two NULL operands are considered equal. This flag is usually set
+   in the context of frontend when ARG0 and/or ARG1 may be NULL mostly due
+   to recursion on partially built expressions (e.g. a CAST_EXPR on a NULL
+   tree.) In this case, we certainly don't want the compiler to crash and
+   it's OK to consider two NULL operands equal. On the other hand, when
+   called in the context of code generation and optimization, if NULL
+   operands are not expected, silently ignoring them could be dangerous
+   and might cause problems downstream that are hard to find/debug. In that
+   case, the flag should probably not be set.  */

 int
 operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
 {
+  /* If either is NULL, they must be both NULL to be equal. We only do this
+     check when OEP_ALLOW_NULL is set.  */
+  if ((flags & OEP_ALLOW_NULL) && (!arg0 || !arg1))
+    return arg0 == arg1;
+
+  /* Similar, if either does not have a type (like a released SSA name, or
+     an operand whose type depends on a template parameter), they aren't
+     equal, unless OEP_ALLOW_NULL_TYPE is set, in which case we will continue
+     to compare the operands only when both don't have a type.  */
+  if (!TREE_TYPE (arg0) || !TREE_TYPE (arg1))
+    {
+      if (((~flags) & OEP_ALLOW_NULL_TYPE)
+          || ((TREE_TYPE (arg0) && !TREE_TYPE (arg1))
+              || (!TREE_TYPE (arg0) && TREE_TYPE (arg1))))
+        return 0;
+    }
+
   /* If either is ERROR_MARK, they aren't equal.  */
   if (TREE_CODE (arg0) == ERROR_MARK || TREE_CODE (arg1) == ERROR_MARK
       || TREE_TYPE (arg0) == error_mark_node
       || TREE_TYPE (arg1) == error_mark_node)
     return 0;

-  /* Similar, if either does not have a type (like a released SSA name),
-     they aren't equal.  */
-  if (!TREE_TYPE (arg0) || !TREE_TYPE (arg1))
-    return 0;
-
   /* Check equality of integer constants before bailing out due to
      precision differences.  */
   if (TREE_CODE (arg0) == INTEGER_CST && TREE_CODE (arg1) == INTEGER_CST)
@@ -2430,19 +2453,25 @@ operand_equal_p (const_tree arg0, const_
      because they may change the signedness of the arguments.  As pointers
      strictly don't have a signedness, require either two pointers or
      two non-pointers as well.  */
-  if (TYPE_UNSIGNED (TREE_TYPE (arg0)) != TYPE_UNSIGNED (TREE_TYPE (arg1))
-      || POINTER_TYPE_P (TREE_TYPE (arg0)) != POINTER_TYPE_P
(TREE_TYPE (arg1)))
+  if (TREE_TYPE (arg0)
+      && (TYPE_UNSIGNED (TREE_TYPE (arg0)) != TYPE_UNSIGNED (TREE_TYPE (arg1))
+          || (POINTER_TYPE_P (TREE_TYPE (arg0))
+              != POINTER_TYPE_P (TREE_TYPE (arg1)))))
     return 0;

   /* We cannot consider pointers to different address space equal.  */
-  if (POINTER_TYPE_P (TREE_TYPE (arg0)) && POINTER_TYPE_P (TREE_TYPE (arg1))
-      && (TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg0)))
-	  != TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg1)))))
+  if (TREE_TYPE (arg0)
+      && (POINTER_TYPE_P (TREE_TYPE (arg0))
+          && POINTER_TYPE_P (TREE_TYPE (arg1))
+          && (TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg0)))
+              != TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg1))))))
     return 0;

   /* If both types don't have the same precision, then it is not safe
      to strip NOPs.  */
-  if (TYPE_PRECISION (TREE_TYPE (arg0)) != TYPE_PRECISION (TREE_TYPE (arg1)))
+  if (TREE_TYPE (arg0)
+      && (TYPE_PRECISION (TREE_TYPE (arg0))
+          != TYPE_PRECISION (TREE_TYPE (arg1))))
     return 0;

   STRIP_NOPS (arg0);
@@ -2467,9 +2496,10 @@ operand_equal_p (const_tree arg0, const_
   if (TREE_CODE (arg0) != TREE_CODE (arg1)
       /* This is needed for conversions and for COMPONENT_REF.
 	 Might as well play it safe and always test this.  */
-      || TREE_CODE (TREE_TYPE (arg0)) == ERROR_MARK
-      || TREE_CODE (TREE_TYPE (arg1)) == ERROR_MARK
-      || TYPE_MODE (TREE_TYPE (arg0)) != TYPE_MODE (TREE_TYPE (arg1)))
+      || (TREE_TYPE (arg0)
+          && (TREE_CODE (TREE_TYPE (arg0)) == ERROR_MARK
+              || TREE_CODE (TREE_TYPE (arg1)) == ERROR_MARK
+              || TYPE_MODE (TREE_TYPE (arg0)) != TYPE_MODE (TREE_TYPE
(arg1)))))
     return 0;

   /* If ARG0 and ARG1 are the same SAVE_EXPR, they are necessarily equal.
@@ -2502,7 +2532,8 @@ operand_equal_p (const_tree arg0, const_
 	  return 1;


-	if (!HONOR_SIGNED_ZEROS (TYPE_MODE (TREE_TYPE (arg0))))
+	if (TREE_TYPE (arg0)
+            && !HONOR_SIGNED_ZEROS (TYPE_MODE (TREE_TYPE (arg0))))
 	  {
 	    /* If we do not distinguish between signed and unsigned zero,
 	       consider them equal.  */
@@ -2570,8 +2601,9 @@ operand_equal_p (const_tree arg0, const_
         {
 	CASE_CONVERT:
         case FIX_TRUNC_EXPR:
-	  if (TYPE_UNSIGNED (TREE_TYPE (arg0))
-	      != TYPE_UNSIGNED (TREE_TYPE (arg1)))
+	  if (TREE_TYPE (arg0)
+              && (TYPE_UNSIGNED (TREE_TYPE (arg0))
+                  != TYPE_UNSIGNED (TREE_TYPE (arg1))))
 	    return 0;
 	  break;
 	default:
@@ -7653,8 +7685,8 @@ build_fold_addr_expr_loc (location_t loc
    OP0.  Return the folded expression if folding is successful.
    Otherwise, return NULL_TREE.  */

-tree
-fold_unary_loc (location_t loc, enum tree_code code, tree type, tree op0)
+static tree
+fold_unary_loc_1 (location_t loc, enum tree_code code, tree type, tree op0)
 {
   tree tem;
   tree arg0;
@@ -8302,6 +8334,41 @@ fold_unary_loc (location_t loc, enum tre
     } /* switch (code) */
 }

+/* Given an expression tree EXP, set the EXPR_FOLDED flag, and if it is
+   a nop, recursively set the EXPR_FOLDED flag of its operand.  */
+
+static void
+set_expr_folded_flag (tree exp)
+{
+  EXPR_FOLDED (exp) = 1;
+
+  /* If EXP is a nop (i.e. NON_LVALUE_EXPRs and NOP_EXPRs), we need to
+     recursively set the EXPR_FOLDED flag of its operand because the
+     expression will be stripped later.  */
+  while ((CONVERT_EXPR_P (exp)
+          || TREE_CODE (exp) == NON_LVALUE_EXPR)
+	 && TREE_OPERAND (exp, 0) != error_mark_node)
+    {
+      exp = TREE_OPERAND (exp, 0);
+      EXPR_FOLDED (exp) = 1;
+    }
+}
+
+/* Fold a unary expression of code CODE and type TYPE with operand
+   OP0.  Return the folded expression if folding is successful.
+   Otherwise, return NULL_TREE.
+   This is a wrapper around fold_unary_loc_1 function (which does the
+   actual folding). Set the EXPR_FOLDED flag of the folded expression
+   if folding is successful.  */
+
+tree
+fold_unary_loc (location_t loc, enum tree_code code, tree type, tree op0)
+{
+  tree tem = fold_unary_loc_1 (loc, code, type, op0);
+  if (tem)
+    set_expr_folded_flag (tem);
+  return tem;
+}

 /* If the operation was a conversion do _not_ mark a resulting constant
    with TREE_OVERFLOW if the original constant was not.  These conversions
@@ -9394,8 +9461,8 @@ get_pointer_modulus_and_residue (tree ex
    Return the folded expression if folding is successful.  Otherwise,
    return NULL_TREE.  */

-tree
-fold_binary_loc (location_t loc,
+static tree
+fold_binary_loc_1 (location_t loc,
 	     enum tree_code code, tree type, tree op0, tree op1)
 {
   enum tree_code_class kind = TREE_CODE_CLASS (code);
@@ -13066,6 +13133,24 @@ fold_binary_loc (location_t loc,
   return tem;
 }

+/* Fold a binary expression of code CODE and type TYPE with operands
+   OP0 and OP1.  LOC is the location of the resulting expression.
+   Return the folded expression if folding is successful.  Otherwise,
+   return NULL_TREE.
+   This is a wrapper around fold_binary_loc_1 function (which does the
+   actual folding). Set the EXPR_FOLDED flag of the folded expression
+   if folding is successful.  */
+
+tree
+fold_binary_loc (location_t loc,
+                 enum tree_code code, tree type, tree op0, tree op1)
+{
+  tree tem = fold_binary_loc_1 (loc, code, type, op0, op1);
+  if (tem)
+    set_expr_folded_flag (tem);
+  return tem;
+}
+
 /* Callback for walk_tree, looking for LABEL_EXPR.  Return *TP if it is
    a LABEL_EXPR; otherwise return NULL_TREE.  Do not check the subtrees
    of GOTO_EXPR.  */
@@ -13102,8 +13187,8 @@ contains_label_p (tree st)
    OP0, OP1, and OP2.  Return the folded expression if folding is
    successful.  Otherwise, return NULL_TREE.  */

-tree
-fold_ternary_loc (location_t loc, enum tree_code code, tree type,
+static tree
+fold_ternary_loc_1 (location_t loc, enum tree_code code, tree type,
 	      tree op0, tree op1, tree op2)
 {
   tree tem;
@@ -13438,6 +13523,23 @@ fold_ternary_loc (location_t loc, enum t
     } /* switch (code) */
 }

+/* Fold a ternary expression of code CODE and type TYPE with operands
+   OP0, OP1, and OP2.  Return the folded expression if folding is
+   successful.  Otherwise, return NULL_TREE.
+   This is a wrapper around fold_ternary_loc_1 function (which does the
+   actual folding). Set the EXPR_FOLDED flag of the folded expression
+   if folding is successful.  */
+
+tree
+fold_ternary_loc (location_t loc, enum tree_code code, tree type,
+                  tree op0, tree op1, tree op2)
+{
+  tree tem = fold_ternary_loc_1 (loc, code, type, op0, op1, op2);
+  if (tem)
+    set_expr_folded_flag (tem);
+  return tem;
+}
+
 /* Perform constant folding and related simplification of EXPR.
    The related simplifications include x*1 => x, x*0 => 0, etc.,
    and application of the associative law.
Index: gcc/testsuite/gcc.dg/plugin/selfassign.c
===================================================================
--- gcc/testsuite/gcc.dg/plugin/selfassign.c	(revision 161236)
+++ gcc/testsuite/gcc.dg/plugin/selfassign.c	(working copy)
@@ -196,7 +196,7 @@ compare_and_warn (gimple stmt, tree lhs,
 /* Check and warn if STMT is a self-assign statement.  */

 static void
-warn_self_assign (gimple stmt)
+check_self_assign (gimple stmt)
 {
   tree rhs, lhs;

@@ -251,7 +251,7 @@ execute_warn_self_assign (void)
   FOR_EACH_BB (bb)
     {
       for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
-        warn_self_assign (gsi_stmt (gsi));
+        check_self_assign (gsi_stmt (gsi));
     }

   return 0;
Index: gcc/testsuite/gcc.dg/wself-assign-1.c
===================================================================
--- gcc/testsuite/gcc.dg/wself-assign-1.c	(revision 0)
+++ gcc/testsuite/gcc.dg/wself-assign-1.c	(revision 0)
@@ -0,0 +1,27 @@
+/* Test the self-assignemnt detection and warning.  */
+/* { dg-do compile } */
+/* { dg-options "-Wself-assign" } */
+
+struct Bar {
+  int b_;
+  int c_;
+};
+
+int g;
+
+int main()
+{
+  struct Bar *bar;
+  int x = x; /* { dg-warning "assigned to itself" } */
+  static int y;
+  struct Bar b_array[5];
+
+  b_array[x+g].b_ = b_array[x+g].b_; /* { dg-warning "assigned to itself" } */
+  g = g; /* { dg-warning "assigned to itself" } */
+  y = y; /* { dg-warning "assigned to itself" } */
+  bar->b_ = bar->b_; /* { dg-warning "assigned to itself" } */
+  x += 0; /* should not warn */
+  y -= 0; /* should not warn */
+  x /= x; /* should not warn */
+  y *= y; /* should not warn */
+}
Index: gcc/testsuite/gcc.dg/wself-assign-2.c
===================================================================
--- gcc/testsuite/gcc.dg/wself-assign-2.c	(revision 0)
+++ gcc/testsuite/gcc.dg/wself-assign-2.c	(revision 0)
@@ -0,0 +1,24 @@
+/* Test how self-assignment detection handles constant-folding happening */
+/*   when parsing the RHS or the initializer.  */
+/* { dg-do compile } */
+/* { dg-options "-Wself-assign" } */
+
+struct Bar {
+  int b_;
+  float c_;
+};
+
+int g;
+
+int main()
+{
+  struct Bar *bar;
+  int x = x - 0; /* should not warn */
+  static int y;
+  struct Bar b_array[5];
+
+  b_array[x+g].b_ = b_array[x+g].b_ * 1; /* should no warn */
+  g = g + 0; /* should not warn */
+  y = y / 1; /* should not warn */
+  bar->b_ = bar->b_ - 0; /* should not warn  */
+}
Index: gcc/testsuite/gcc.dg/wself-assign-3.c
===================================================================
--- gcc/testsuite/gcc.dg/wself-assign-3.c	(revision 0)
+++ gcc/testsuite/gcc.dg/wself-assign-3.c	(revision 0)
@@ -0,0 +1,9 @@
+/* Test whether -Wself-assign is enabled by -Wall. */
+/* { dg-do compile } */
+/* { dg-options "-Wall" } */
+
+int bar() {
+  int i = 0;
+  i = i;             /* { dg-warning "assigned to itself" } */
+  return i;
+}
Index: gcc/testsuite/gcc.dg/uninit-D-O0.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-D-O0.c	(revision 161236)
+++ gcc/testsuite/gcc.dg/uninit-D-O0.c	(working copy)
@@ -1,9 +0,0 @@
-/* Test we do not warn about initializing variable with self. */
-/* { dg-do compile } */
-/* { dg-options "-Wuninitialized" } */
-
-int f()
-{
-  int i = i;
-  return i;
-}
Index: gcc/testsuite/gcc.dg/uninit-D.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-D.c	(revision 161236)
+++ gcc/testsuite/gcc.dg/uninit-D.c	(working copy)
@@ -1,9 +0,0 @@
-/* Test we do not warn about initializing variable with self. */
-/* { dg-do compile } */
-/* { dg-options "-O -Wuninitialized" } */
-
-int f()
-{
-  int i = i;
-  return i;
-}
Index: gcc/testsuite/gcc.dg/uninit-E-O0.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-E-O0.c	(revision 161236)
+++ gcc/testsuite/gcc.dg/uninit-E-O0.c	(working copy)
@@ -1,9 +0,0 @@
-/* Test we do warn about initializing variable with self when
-Winit-self is supplied. */
-/* { dg-do compile } */
-/* { dg-options "-Wuninitialized -Winit-self" } */
-
-int f()
-{
-  int i = i; /* { dg-warning "i" "uninitialized variable warning" }  */
-  return i;
-}
Index: gcc/testsuite/gcc.dg/uninit-E.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-E.c	(revision 161236)
+++ gcc/testsuite/gcc.dg/uninit-E.c	(working copy)
@@ -1,9 +0,0 @@
-/* Test we do warn about initializing variable with self when
-Winit-self is supplied. */
-/* { dg-do compile } */
-/* { dg-options "-O -Wuninitialized -Winit-self" } */
-
-int f()
-{
-  int i = i; /* { dg-warning "i" "uninitialized variable warning" }  */
-  return i;
-}
Index: gcc/testsuite/g++.dg/plugin/selfassign.c
===================================================================
--- gcc/testsuite/g++.dg/plugin/selfassign.c	(revision 161236)
+++ gcc/testsuite/g++.dg/plugin/selfassign.c	(working copy)
@@ -196,7 +196,7 @@ compare_and_warn (gimple stmt, tree lhs,
 /* Check and warn if STMT is a self-assign statement.  */

 static void
-warn_self_assign (gimple stmt)
+check_self_assign (gimple stmt)
 {
   tree rhs, lhs;

@@ -251,7 +251,7 @@ execute_warn_self_assign (void)
   FOR_EACH_BB (bb)
     {
       for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
-        warn_self_assign (gsi_stmt (gsi));
+        check_self_assign (gsi_stmt (gsi));
     }

   return 0;
Index: gcc/testsuite/g++.dg/warn/Wself-assign-1.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wself-assign-1.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/Wself-assign-1.C	(revision 0)
@@ -0,0 +1,54 @@
+// Test the self-assignemnt detection and warning.
+// { dg-do compile }
+// { dg-options "-Wself-assign" }
+
+class Foo {
+ private:
+  int a_;
+
+ public:
+  Foo() : a_(a_) {} // { dg-warning "assigned to itself" }
+
+  void setA(int a) {
+    a_ = a_; // { dg-warning "assigned to itself" }
+  }
+
+  void operator=(Foo& rhs) {
+    this->a_ = rhs.a_;
+  }
+};
+
+struct Bar {
+  int b_;
+  int c_;
+};
+
+int g = g; // { dg-warning "assigned to itself" }
+Foo foo = foo; // { dg-warning "assigned to itself" }
+
+int func()
+{
+  Bar *bar1, bar2;
+  Foo local_foo;
+  int x = x; // { dg-warning "assigned to itself" }
+  static int y = y; // { dg-warning "assigned to itself" }
+  float *f;
+  Bar bar_array[5];
+  char n;
+  int overflow;
+
+  *f = *f; // { dg-warning "assigned to itself" }
+  bar1->b_ = bar1->b_; // { dg-warning "assigned to itself" }
+  bar2.c_ = bar2.c_; // { dg-warning "assigned to itself" }
+  local_foo = local_foo; // { dg-warning "assigned to itself" }
+  foo = foo; // { dg-warning "assigned to itself" }
+  foo.setA(5);
+  bar_array[3].c_ = bar_array[3].c_; // { dg-warning "assigned to itself" }
+  bar_array[x+g].b_ = bar_array[x+g].b_; // { dg-warning "assigned to itself" }
+  y = x;
+  x = y;
+  x += 0; // should not warn
+  y -= 0; // should not warn
+  x /= x; // should not warn
+  y *= y; // should not warn
+}
Index: gcc/testsuite/g++.dg/warn/Wself-assign-2.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wself-assign-2.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/Wself-assign-2.C	(revision 0)
@@ -0,0 +1,31 @@
+// Test the handling of expressions that depend on template parameters in
+// self-assignemnt detection.
+// { dg-do compile }
+// { dg-options "-Wself-assign" }
+
+template<typename T>
+struct Bar {
+  T x;
+  Bar operator++(int) {
+    Bar tmp = *this;
+    ++x;
+    tmp = tmp; // { dg-warning "assigned to itself" }
+    return tmp;
+  }
+};
+
+template<typename T>
+T DoSomething(T y) {
+  T a[5], *p;
+  Bar<T> b;
+  b.x = b.x; // { dg-warning "assigned to itself" }
+  *p = *p; // { dg-warning "assigned to itself" }
+  a[2] = a[2]; // { dg-warning "assigned to itself" }
+  return *p;
+}
+
+main() {
+  Bar<int> bar;
+  bar++;
+  DoSomething(5);
+}
Index: gcc/testsuite/g++.dg/warn/Wself-assign-3.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wself-assign-3.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/Wself-assign-3.C	(revision 0)
@@ -0,0 +1,35 @@
+// Test how operands_equal_p handles a NULL operand.
+// { dg-do compile }
+// { dg-options "-Wself-assign" }
+
+#include <cstdio>
+
+namespace testing {
+
+class Foo {
+  int f;
+ public:
+  Foo() { printf("Construct Foo\n"); }
+};
+
+class Bar {
+  int b;
+ public:
+  Bar(int x) { printf("Construct Bar\n"); }
+
+  void operator=(const Foo& foo) {
+    printf("Assign Foo to Bar\n");
+  }
+};
+
+}
+
+template <class T>
+void func(T t) {
+  ::testing::Bar(1) = ::testing::Foo(); // used to trigger a segfault
+  ::testing::Foo() = ::testing::Foo(); // { dg-warning "assigned to itself" }
+}
+
+main() {
+  func(2);
+}
Index: gcc/testsuite/g++.dg/warn/Wself-assign-4.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wself-assign-4.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/Wself-assign-4.C	(revision 0)
@@ -0,0 +1,48 @@
+// Test how self-assignment detection handles constant-folding happening
+// when parsing the RHS or the initializer.
+// { dg-do compile }
+// { dg-options "-Wself-assign" }
+
+class Foo {
+ private:
+  int a_;
+
+ public:
+  Foo() : a_(a_+0) {} // should not warn
+
+  void setA(int a) {
+    a_ = a_ + 0; // should not warn
+  }
+
+  void operator=(Foo& rhs) {
+    this->a_ = rhs.a_;
+  }
+};
+
+struct Bar {
+  int b_;
+  float c_;
+};
+
+int g = g * 1; // should not warn
+
+int func()
+{
+  Bar *bar1, bar2;
+  Foo foo;
+  int x = x - 0;        // should not warn
+  static int y = y / 1; // should not warn
+  float *f;
+  Bar bar_array[5];
+
+  *f = *f / 1;             // should not warn
+  bar1->b_ = bar1->b_ * 1; // should not warn
+  bar2.c_ = bar2.c_ - 0;   // should not warn
+  foo.setA(5);
+  bar_array[3].c_ = bar_array[3].c_ * 1;     // should not warn
+  bar_array[x+g].b_ = bar_array[x+g].b_ / 1; // should not warn
+  x += 0;
+  y -= 0;
+  foo = foo;           // { dg-warning "assigned to itself" }
+  foo.operator=(foo);  // should not warn
+}
Index: gcc/testsuite/g++.dg/warn/Wself-assign-5.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wself-assign-5.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/Wself-assign-5.C	(revision 0)
@@ -0,0 +1,21 @@
+// Test whether -Wself-assign is enabled by -Wall.
+// { dg-do compile }
+// { dg-options "-Wall" }
+
+class Foo {
+  int a, b;
+ public:
+  Foo() : a(a) { }   // { dg-warning "assigned to itself" }
+  int foo() { return b; }
+};
+
+int c = c;           // { dg-warning "assigned to itself" }
+
+Foo foo;
+
+int bar() {
+  static int d = d;  // { dg-warning "assigned to itself" }
+  int i = 0;
+  i = i;             // { dg-warning "assigned to itself" }
+  return d;
+}
Index: gcc/cp/init.c
===================================================================
--- gcc/cp/init.c	(revision 161236)
+++ gcc/cp/init.c	(working copy)
@@ -529,8 +529,14 @@ perform_member_init (tree member, tree i
 	init = build_x_compound_expr_from_list (init, ELK_MEM_INIT);

       if (init)
-	finish_expr_stmt (cp_build_modify_expr (decl, INIT_EXPR, init,
-						tf_warning_or_error));
+        {
+          finish_expr_stmt (cp_build_modify_expr (decl, INIT_EXPR, init,
+                                                  tf_warning_or_error));
+          /* Check for and warn about self-initialization if -Wself-assign is
+             enabled.  */
+          if (warn_self_assign)
+            check_for_self_assign (input_location, decl, init);
+        }
     }

   if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (type))
Index: gcc/cp/parser.c
===================================================================
--- gcc/cp/parser.c	(revision 161236)
+++ gcc/cp/parser.c	(working copy)
@@ -6903,6 +6903,12 @@ cp_parser_assignment_expression (cp_pars
 	      if (cp_parser_non_integral_constant_expression (parser,
 							      NIC_ASSIGNMENT))
 		return error_mark_node;
+
+              /* Check for and warn about self-assignment if -Wself-assign is
+                 enabled and the assignment operator is "=".  */
+              if (warn_self_assign && assignment_operator == NOP_EXPR)
+                check_for_self_assign (input_location, expr, rhs);
+
 	      /* Build the assignment expression.  */
 	      expr = build_x_modify_expr (expr,
 					  assignment_operator,
@@ -14075,6 +14081,10 @@ cp_parser_init_declarator (cp_parser* pa
 			 `explicit' constructor cannot be used.  */
 		      ((is_direct_init || !is_initialized)
 		       ? 0 : LOOKUP_ONLYCONVERTING));
+      /* Check for and warn about self-initialization if -Wself-assign is
+         enabled.  */
+      if (warn_self_assign && initializer)
+        check_for_self_assign (input_location, decl, initializer);
     }
   else if ((cxx_dialect != cxx98) && friend_p
 	   && decl && TREE_CODE (decl) == FUNCTION_DECL)
Index: gcc/c-parser.c
===================================================================
--- gcc/c-parser.c	(revision 161236)
+++ gcc/c-parser.c	(working copy)
@@ -1297,6 +1297,11 @@ c_parser_declaration_or_fndef (c_parser
 		  maybe_warn_string_init (TREE_TYPE (d), init);
 		  finish_decl (d, init_loc, init.value,
 		      	       init.original_type, asm_name);
+                  /* Check for and warn about self-initialization if
+                     -Wself-assign is enabled. Don't warn if there is
+                     already an error for the initializer.  */
+                  if (warn_self_assign && DECL_INITIAL (d) != error_mark_node)
+                    check_for_self_assign (here, d, init.value);
 		}
 	    }
 	  else
@@ -4767,7 +4772,13 @@ c_parser_expr_no_commas (c_parser *parse
 				 code, exp_location, rhs.value,
 				 rhs.original_type);
   if (code == NOP_EXPR)
-    ret.original_code = MODIFY_EXPR;
+    {
+      ret.original_code = MODIFY_EXPR;
+      /* Check for and warn about self-assignment if -Wself-assign is
+         enabled.  */
+      if (warn_self_assign)
+        check_for_self_assign (op_location, lhs.value, rhs.value);
+    }
   else
     {
       TREE_NO_WARNING (ret.value) = 1;

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

[Paul Koning]

> ...
> Besides minor changes that address the review comments, the
> major differences of this revised patch from the original one are (1)
> -Wself-assign is enabled by -Wall, and (2) -Winit-self flag is
> removed.

Given that self-assign is the documented way to suppress a warning (for
uninitialized variable) is it appropriate for -Wall to turn this on?  It
would mean that code that was specifically fixed to avoid warnings under
-Wall will now get warnings again on that exact same line.  I don't
object to the feature itself, but I do object to that aspect.

	paul

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

[Le-Chun Wu]

Paul,

On Wed, Jun 23, 2010 at 2:30 PM, Paul Koning <Paul_Koning@dell.com> wrote:
>> ...
>> Besides minor changes that address the review comments, the
>> major differences of this revised patch from the original one are (1)
>> -Wself-assign is enabled by -Wall, and (2) -Winit-self flag is
>> removed.
>
> Given that self-assign is the documented way to suppress a warning (for
> uninitialized variable) is it appropriate for -Wall to turn this on?  It
> would mean that code that was specifically fixed to avoid warnings under
> -Wall will now get warnings again on that exact same line.  I don't
> object to the feature itself, but I do object to that aspect.
>

This patch actually removes the paragraph in invoke.texi that
recommends using self-initialization to suppress uninitialized
variable warnings. Yes, this would be a disruptive change to people
who use self-initialization, but in my opinion, self-initialization
doesn't really make sense and is not a good way to fix or work around
uninitialized variable warnings. (One can initialize a variable with
other more meaningful value as easily.)

Le-chun

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

[Le-Chun Wu]

On Wed, Jun 23, 2010 at 2:56 PM, Le-Chun Wu <lcwu@google.com> wrote:
> Paul,
>
> On Wed, Jun 23, 2010 at 2:30 PM, Paul Koning <Paul_Koning@dell.com> wrote:
>>> ...
>>> Besides minor changes that address the review comments, the
>>> major differences of this revised patch from the original one are (1)
>>> -Wself-assign is enabled by -Wall, and (2) -Winit-self flag is
>>> removed.
>>
>> Given that self-assign is the documented way to suppress a warning (for
>> uninitialized variable) is it appropriate for -Wall to turn this on?  It
>> would mean that code that was specifically fixed to avoid warnings under
>> -Wall will now get warnings again on that exact same line.  I don't
>> object to the feature itself, but I do object to that aspect.
>>
>
> This patch actually removes the paragraph in invoke.texi that
> recommends using self-initialization to suppress uninitialized
> variable warnings. Yes, this would be a disruptive change to people
> who use self-initialization, but in my opinion, self-initialization
> doesn't really make sense and is not a good way to fix or work around
> uninitialized variable warnings. (One can initialize a variable with
> other more meaningful value as easily.)
>

I forgot to mention that when this patch is approved and checked in, I
will also update gcc-4.6/changes.html to mention that
self-initialization is not longer a recommended way to suppress the
uninitialized variable warnings.

Le-chun

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

[Joseph S. Myers]

On Wed, 23 Jun 2010, Le-Chun Wu wrote:

> Hi,
> 
> Here is the revised patch for the implementation of -Wself-assign
> warning. Besides minor changes that address the review comments, the
> major differences of this revised patch from the original one are (1)
> -Wself-assign is enabled by -Wall, and (2) -Winit-self flag is
> removed.
> 
> (For the discussion thread of the original patch, please see
> http://gcc.gnu.org/ml/gcc-patches/2010-06/msg00974.html)

I see nothing in that thread that justifies breaking backwards 
compatibility by removing an option.  The usual practice for non-semantic 
options (those not affecting how source code is interpreted or placing 
specific requirements on generated code) would be to keep as a no-op; this 
was done for -Wunreachable-code, for example.  It might perhaps be better 
to make -Winit-self an alias for the new option, as the nearest current 
equivalent.

> 	* gcc/doc/invoke.texi (-Wall): Include -Wself-assign.

Please give the ChangeLog entries separately for each affected ChangeLog 
file.  This one for example would go in gcc/ChangeLog and the entry would 
start "* doc/invoke.texi".  The logs in gcc/c-family and gcc/testsuite and 
gcc/cp would also have entries, with the paths being relative to the 
relevant ChangeLog directories.

> +@item -Wself-assign
> +@opindex Wself-assign
> +@opindex Wno-self-assign
> +Warn about self-assignment and self-initialization. This warning is intended
> +for detecting accidental self-assignment due to typos, and therefore does
> +not warn on a statement that is semantically a self-assignment after
> +constant folding. Here is an example of what will trigger a self-assign
> +warning and what will not:
> +
> +@smallexample
> +@group
> +void func()
> +@{
> +   int i = 2;
> +   int x = x;   /* warn */
> +   float f = 5.0;
> +   double a[3];
> +
> +   i = i + 0;   /* not warn */

Note that an *initialization*, as opposed to an *assignment*, using i + 0 
as the initializer for i (or i + 1, for that matter), is an example of 
erroneous code (using an uninitialized value) people may well want a 
warning for and can get one for right now with -Winit-self.  Can they 
still get such a warning with the new option?

(The logic above seems fine for which *assignments* get warnings for being 
self-assignments.  Though if i is uninitialized, an assignment i = i + 0 
should still get a warning for the uninitialized use of i, and I hope it 
does; that warning doesn't depend on -Winit-self at present.)

-- 
Joseph S. Myers
joseph@codesourcery.com

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

[Le-Chun Wu]

On Wed, Jun 23, 2010 at 3:37 PM, Joseph S. Myers
<joseph@codesourcery.com> wrote:
> On Wed, 23 Jun 2010, Le-Chun Wu wrote:
>
>> Hi,
>>
>> Here is the revised patch for the implementation of -Wself-assign
>> warning. Besides minor changes that address the review comments, the
>> major differences of this revised patch from the original one are (1)
>> -Wself-assign is enabled by -Wall, and (2) -Winit-self flag is
>> removed.
>>
>> (For the discussion thread of the original patch, please see
>> http://gcc.gnu.org/ml/gcc-patches/2010-06/msg00974.html)
>
> I see nothing in that thread that justifies breaking backwards
> compatibility by removing an option.  The usual practice for non-semantic
> options (those not affecting how source code is interpreted or placing
> specific requirements on generated code) would be to keep as a no-op; this
> was done for -Wunreachable-code, for example.  It might perhaps be better
> to make -Winit-self an alias for the new option, as the nearest current
> equivalent.

Yes, I agree we should not remove -Winit-self and break backwards
compatibility. I will put the option back. However, in this case I
think it's probably better to make it a no-op because -Winit-self used
to only work with -Wuninitialized. (i.e. Without using
-Wuninitialized, -Winit-self is a no-op.) Now that we remove the
functionality of -Winit-self, people who use -Wuninitialized will
always get a warning on the following code with or without
-Winit-self:

void func() {
  int i = i;
}

And also by not making it an alias of -Wself-assign, people who use
-Winit-self won't be surprised by the new warning on self-assignment
statements.

I will send out a revised patch later.

>
>>       * gcc/doc/invoke.texi (-Wall): Include -Wself-assign.
>
> Please give the ChangeLog entries separately for each affected ChangeLog
> file.  This one for example would go in gcc/ChangeLog and the entry would
> start "* doc/invoke.texi".  The logs in gcc/c-family and gcc/testsuite and
> gcc/cp would also have entries, with the paths being relative to the
> relevant ChangeLog directories.

Yes. I actually have separate ChangeLog files in my svn tree (and Eric
mention that to me as well). I was simply lazy and reused the
ChangeLog entry I sent out in my original patch. :-) I will send out
separate ChangeLog entries in my revised patch.

>
>> +@item -Wself-assign
>> +@opindex Wself-assign
>> +@opindex Wno-self-assign
>> +Warn about self-assignment and self-initialization. This warning is intended
>> +for detecting accidental self-assignment due to typos, and therefore does
>> +not warn on a statement that is semantically a self-assignment after
>> +constant folding. Here is an example of what will trigger a self-assign
>> +warning and what will not:
>> +
>> +@smallexample
>> +@group
>> +void func()
>> +@{
>> +   int i = 2;
>> +   int x = x;   /* warn */
>> +   float f = 5.0;
>> +   double a[3];
>> +
>> +   i = i + 0;   /* not warn */
>
> Note that an *initialization*, as opposed to an *assignment*, using i + 0
> as the initializer for i (or i + 1, for that matter), is an example of
> erroneous code (using an uninitialized value) people may well want a
> warning for and can get one for right now with -Winit-self.  Can they
> still get such a warning with the new option?

The new flag doesn't warn on "int i = i + 0", but -Wuninitialized does
(which is a more appropriate option to control the warning on this
case, I think).

$ cat self-init.c
void func()
{
  int a = a + 0;
  int b = b + 1;
  int c;
  c = c + 0;
}

$ gcc -c self-init.c -Wuninitialized
self-init.c: In function ‘func’:
self-init.c:3:7: warning: ‘a’ is used uninitialized in this function
[-Wuninitialized]
self-init.c:4:7: warning: ‘b’ is used uninitialized in this function
[-Wuninitialized]
self-init.c:6:5: warning: ‘c’ is used uninitialized in this function
[-Wuninitialized]

So I think we are covered here.

>
> (The logic above seems fine for which *assignments* get warnings for being
> self-assignments.  Though if i is uninitialized, an assignment i = i + 0
> should still get a warning for the uninitialized use of i, and I hope it
> does; that warning doesn't depend on -Winit-self at present.)

Yes, it does. See the example above.

Thanks,

Le-chun

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

[Joseph S. Myers]

On Wed, 23 Jun 2010, Le-Chun Wu wrote:

> The new flag doesn't warn on "int i = i + 0", but -Wuninitialized does
> (which is a more appropriate option to control the warning on this
> case, I think).

Agreed.  Perhaps instead of removing the -Winit-self testcases you should 
change them to verify that -Wuninitialized alone is now enough to enable 
those warnings.

-- 
Joseph S. Myers
joseph@codesourcery.com

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

[Le-Chun Wu]

On Wed, Jun 23, 2010 at 5:51 PM, Joseph S. Myers
<joseph@codesourcery.com> wrote:
> On Wed, 23 Jun 2010, Le-Chun Wu wrote:
>
>> The new flag doesn't warn on "int i = i + 0", but -Wuninitialized does
>> (which is a more appropriate option to control the warning on this
>> case, I think).
>
> Agreed.  Perhaps instead of removing the -Winit-self testcases you should
> change them to verify that -Wuninitialized alone is now enough to enable
> those warnings.

That's exactly what I am doing now. :-)

Thanks,

Le-chun

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

[Paul Koning]

> > Given that self-assign is the documented way to suppress a warning
> (for
> > uninitialized variable) is it appropriate for -Wall to turn this on?
>  It
> > would mean that code that was specifically fixed to avoid warnings
> under
> > -Wall will now get warnings again on that exact same line.  I don't
> > object to the feature itself, but I do object to that aspect.
> >
> 
> This patch actually removes the paragraph in invoke.texi that
> recommends using self-initialization to suppress uninitialized
> variable warnings. Yes, this would be a disruptive change to people
> who use self-initialization, but in my opinion, self-initialization
> doesn't really make sense and is not a good way to fix or work around
> uninitialized variable warnings. (One can initialize a variable with
> other more meaningful value as easily.)

Yes, and that's usually fine.  But there is a performance difference -- a small one admittedly.  If the initialization is in fact not necessary, then the existing coding convention suppresses the warning with no run-time overhead, while the proposed workaround you describe does have a run-time cost.

	paul

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

[Le-Chun Wu]

On Wed, Jun 23, 2010 at 5:54 PM, Paul Koning <Paul_Koning@dell.com> wrote:
>> > Given that self-assign is the documented way to suppress a warning
>> (for
>> > uninitialized variable) is it appropriate for -Wall to turn this on?
>>  It
>> > would mean that code that was specifically fixed to avoid warnings
>> under
>> > -Wall will now get warnings again on that exact same line.  I don't
>> > object to the feature itself, but I do object to that aspect.
>> >
>>
>> This patch actually removes the paragraph in invoke.texi that
>> recommends using self-initialization to suppress uninitialized
>> variable warnings. Yes, this would be a disruptive change to people
>> who use self-initialization, but in my opinion, self-initialization
>> doesn't really make sense and is not a good way to fix or work around
>> uninitialized variable warnings. (One can initialize a variable with
>> other more meaningful value as easily.)
>
> Yes, and that's usually fine.  But there is a performance difference -- a small one admittedly.  If the initialization is in fact not necessary, then the existing coding convention suppresses the warning with no run-time overhead, while the proposed workaround you describe does have a run-time cost.
>

I would think most of the uninitialized warnings triggered are genuine
bugs and the programmers should just fix them. The self-init
workaround was probably used primarily to suppress the false positives
that were often caused by the analysis not understanding the
predicates that guard the definitions and uses of variables. Now that
David Li has checked in the patch that makes the uninitialized
variable analysis predicate-aware (r158835), the false positive rate
should go down quite a lot. And even for the rare cases where people
still have to suppress the warnings, as you said, the performance
difference of initializing a variable to a real value should be pretty
small (but the code would make more sense). So I still think that we
should include -Wself-assign in -Wall, and that people who use
self-initialization (with -Wall) should evaluate whether the false
positives still exist, and either remove the self-initialization or
change the initialization to a more meaningful value.

Thanks,

Le-chun

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

[Le-Chun Wu]

[-- Attachment #1: Type: text/plain, Size: 4557 bytes --]

Attached please find the revised patch for the new flag -Wself-assign.
Comparing with the previous patch, this one puts -Winit-self back but
keeps it as a no-op, and resurrects the -Winit-self test cases to make
sure the flag is still accepted by GCC. It also fixes the following
self-assign/self-init warnings in GCC source code when bootstrapping.
(The flag actually identified a real bug/typo in cgraphunit.c. Yeah!
:-)) Relevant maintainers of these files are also cc'ed in this email.

        * 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)

Again, bootstrapped and tested on x86_64-linux-gnu. OK for trunk?

Le-chun

gcc/ChangeLog

2010-06-24  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.
        * dbxout.c (DEBUGGER_ARG_OFFSET): Change OFFSET to OFFSET+0 to avoid
        self-assign warning.
        * omega.c (omega_eliminate_redundant): Remove a self-assign statement.
        * tree-ssa-ccp.c (ccp_lattice_meet): Remove a self-assign statement
        and an unnecessary assignment.
        * cgraphunit.c (cgraph_copy_node_for_versioning): Fix a typo detected
        by -Wself-assign.
        * config/i386/i386.c (ix86_vectorize_builtin_vec_perm): Remove
        unnecessary self-init.

gcc/c-family/ChangeLog

2010-06-24  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-06-24  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

        * 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.

[-- Attachment #2: selfassign.patch --]
[-- Type: text/x-patch, Size: 37636 bytes --]

Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 161236)
+++ gcc/doc/invoke.texi	(working copy)
@@ -242,8 +242,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wformat-security  -Wformat-y2k @gol
 -Wframe-larger-than=@var{len} -Wjump-misses-init -Wignored-qualifiers @gol
 -Wimplicit  -Wimplicit-function-declaration  -Wimplicit-int @gol
--Winit-self  -Winline @gol
--Wno-int-to-pointer-cast -Wno-invalid-offsetof @gol
+-Winline -Wno-int-to-pointer-cast -Wno-invalid-offsetof @gol
 -Winvalid-pch -Wlarger-than=@var{len}  -Wunsafe-loop-optimizations @gol
 -Wlogical-op -Wlong-long @gol
 -Wmain  -Wmissing-braces  -Wmissing-field-initializers @gol
@@ -254,7 +253,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wparentheses  -Wpedantic-ms-format -Wno-pedantic-ms-format @gol
 -Wpointer-arith  -Wno-pointer-to-int-cast @gol
 -Wredundant-decls @gol
--Wreturn-type  -Wsequence-point  -Wshadow @gol
+-Wreturn-type -Wself-assign -Wsequence-point  -Wshadow @gol
 -Wsign-compare  -Wsign-conversion  -Wstack-protector @gol
 -Wstrict-aliasing -Wstrict-aliasing=n @gol
 -Wstrict-overflow -Wstrict-overflow=@var{n} @gol
@@ -2943,6 +2942,7 @@ Options} and @ref{Objective-C and Object
 -Wpointer-sign  @gol
 -Wreorder   @gol
 -Wreturn-type  @gol
+-Wself-assign @r{(only in C/C++)}  @gol
 -Wsequence-point  @gol
 -Wsign-compare @r{(only in C++)}  @gol
 -Wstrict-aliasing  @gol
@@ -3143,24 +3143,6 @@ requiring a non-null value by the @code{
 @option{-Wnonnull} is included in @option{-Wall} and @option{-Wformat}.  It
 can be disabled with the @option{-Wno-nonnull} option.
 
-@item -Winit-self @r{(C, C++, Objective-C and Objective-C++ only)}
-@opindex Winit-self
-@opindex Wno-init-self
-Warn about uninitialized variables which are initialized with themselves.
-Note this option can only be used with the @option{-Wuninitialized} option.
-
-For example, GCC will warn about @code{i} being uninitialized in the
-following snippet only when @option{-Winit-self} has been specified:
-@smallexample
-@group
-int f()
-@{
-  int i = i;
-  return i;
-@}
-@end group
-@end smallexample
-
 @item -Wimplicit-int @r{(C and Objective-C only)}
 @opindex Wimplicit-int
 @opindex Wno-implicit-int
@@ -3323,6 +3305,53 @@ definitions, may be found on the GCC rea
 
 This warning is enabled by @option{-Wall} for C and C++.
 
+@item -Wself-assign
+@opindex Wself-assign
+@opindex Wno-self-assign
+Warn about self-assignment and self-initialization. This warning is intended
+for detecting accidental self-assignment due to typos, and therefore does
+not warn on a statement that is semantically a self-assignment after
+constant folding. Here is an example of what will trigger a self-assign
+warning and what will not:
+
+@smallexample
+@group
+void func()
+@{
+   int i = 2;
+   int x = x;   /* warn */
+   float f = 5.0;
+   double a[3];
+
+   i = i + 0;   /* not warn */
+   f = f / 1;   /* not warn */
+   a[1] = a[1]; /* warn */
+   i += 0;      /* not warn */
+@}
+@end group
+@end smallexample
+
+There are cases where self-assignment might be intentional. For example,
+a C++ programmers might write code to test whether an overloaded
+@code{operator=} works when the same object is assigned to itself.
+One way to work around the self-assign warning in such case is using
+the functional form @code{object.operator=(object)} instead of the
+assignment form @code{object = object}, as shown in the following example.
+
+@smallexample
+@group
+void test_func()
+@{
+   MyType t;
+
+   t.operator=(t);  // not warn
+   t = t;           // warn
+@}
+@end group
+@end smallexample
+
+This warning is enabled by @option{-Wall} for C and C++.
+
 @item -Wreturn-type
 @opindex Wreturn-type
 @opindex Wno-return-type
@@ -3446,6 +3475,10 @@ This warning is enabled by @option{-Wall
 To suppress this warning use the @samp{unused} attribute
 (@pxref{Variable Attributes}).
 
+Note that a classic way to avoid @option{-Wunused-variable} warning is
+using @code{x = x}, but that does not work with @option{-Wself-assign}.
+Use @code{(void) x} or @code{static_cast<void>(x)} instead.
+
 @item -Wunused-value
 @opindex Wunused-value
 @opindex Wno-unused-value
@@ -3475,9 +3508,6 @@ or if a variable may be clobbered by a @
 warn if a non-static reference or non-static @samp{const} member
 appears in a class without constructors.
 
-If you want to warn about code which uses the uninitialized value of the
-variable in its own initializer, use the @option{-Winit-self} option.
-
 These warnings occur for individual uninitialized or clobbered
 elements of structure, union or array variables as well as for
 variables which are uninitialized or clobbered as a whole.  They do
Index: gcc/c-family/c-gimplify.c
===================================================================
--- gcc/c-family/c-gimplify.c	(revision 161236)
+++ gcc/c-family/c-gimplify.c	(working copy)
@@ -173,18 +173,5 @@ int
 c_gimplify_expr (tree *expr_p, gimple_seq *pre_p ATTRIBUTE_UNUSED,
 		 gimple_seq *post_p ATTRIBUTE_UNUSED)
 {
-  enum tree_code code = TREE_CODE (*expr_p);
-
-  /* This is handled mostly by gimplify.c, but we have to deal with
-     not warning about int x = x; as it is a GCC extension to turn off
-     this warning but only if warn_init_self is zero.  */
-  if (code == DECL_EXPR
-      && TREE_CODE (DECL_EXPR_DECL (*expr_p)) == VAR_DECL
-      && !DECL_EXTERNAL (DECL_EXPR_DECL (*expr_p))
-      && !TREE_STATIC (DECL_EXPR_DECL (*expr_p))
-      && (DECL_INITIAL (DECL_EXPR_DECL (*expr_p)) == DECL_EXPR_DECL (*expr_p))
-      && !warn_init_self)
-    TREE_NO_WARNING (DECL_EXPR_DECL (*expr_p)) = 1;
-
   return GS_UNHANDLED;
 }
Index: gcc/c-family/c.opt
===================================================================
--- gcc/c-family/c.opt	(revision 161236)
+++ gcc/c-family/c.opt	(working copy)
@@ -259,8 +259,8 @@ C C++ Var(warn_ignored_qualifiers) Init(
 Warn whenever type qualifiers are ignored.
 
 Winit-self
-C ObjC C++ ObjC++ Var(warn_init_self) Warning
-Warn about variables which are initialized to themselves
+C ObjC C++ ObjC++
+Does nothing. Preserved for backward compatibility.
 
 Wimplicit
 C ObjC Var(warn_implicit) Init(-1) Warning
@@ -425,6 +425,10 @@ Wreturn-type
 C ObjC C++ ObjC++ Var(warn_return_type) Warning
 Warn whenever a function's return type defaults to \"int\" (C), or about inconsistent return types (C++)
 
+Wself-assign
+C C++ Var(warn_self_assign) Init(0) Warning
+Warn when a variable is assigned to itself
+
 Wselector
 ObjC ObjC++ Var(warn_selector) Warning
 Warn if a selector has multiple methods
Index: gcc/c-family/c-opts.c
===================================================================
--- 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 ())
 	{
Index: gcc/c-family/c-common.c
===================================================================
--- gcc/c-family/c-common.c	(revision 161236)
+++ gcc/c-family/c-common.c	(working copy)
@@ -9266,4 +9266,20 @@ make_tree_vector_copy (const VEC(tree,gc
   return ret;
 }
 
+/* Check for and warn about self-assignment or self-initialization.
+   LHS and RHS are the tree nodes for the left-hand side and right-hand side
+   of the assignment or initialization we are checking.
+   LOCATION is the source location for RHS.  */
+
+void
+check_for_self_assign (location_t location, tree lhs, tree rhs)
+{
+  /* Only emit a warning if RHS is not a folded expression so that we don't
+     warn on something like x = x / 1.  */
+  if (!EXPR_FOLDED (rhs)
+      && operand_equal_p (lhs, rhs,
+                          OEP_PURE_SAME | OEP_ALLOW_NULL | OEP_ALLOW_NULL_TYPE))
+    warning_at (location, OPT_Wself_assign, "%qE is assigned to itself", lhs);
+}
+
 #include "gt-c-family-c-common.h"
Index: gcc/c-family/c-common.h
===================================================================
--- gcc/c-family/c-common.h	(revision 161236)
+++ gcc/c-family/c-common.h	(working copy)
@@ -893,6 +893,7 @@ extern VEC(tree,gc) *make_tree_vector (v
 extern void release_tree_vector (VEC(tree,gc) *);
 extern VEC(tree,gc) *make_tree_vector_single (tree);
 extern VEC(tree,gc) *make_tree_vector_copy (const VEC(tree,gc) *);
+extern void check_for_self_assign (location_t, tree, tree);
 
 /* In c-gimplify.c  */
 extern void c_genericize (tree);
Index: gcc/tree.h
===================================================================
--- gcc/tree.h	(revision 161236)
+++ gcc/tree.h	(working copy)
@@ -388,8 +388,9 @@ struct GTY(()) tree_base {
   unsigned visited : 1;
   unsigned packed_flag : 1;
   unsigned user_align : 1;
+  unsigned expr_folded_flag : 1;
 
-  unsigned spare : 13;
+  unsigned spare : 12;
 
   /* This field is only used with type nodes; the only reason it is present
      in tree_base instead of tree_type is to save space.  The size of the
@@ -627,6 +628,13 @@ struct GTY(()) tree_common {
 
        SSA_NAME_IS_DEFAULT_DEF in
            SSA_NAME
+
+   expr_folded_flag:
+
+       EXPR_FOLDED in
+           all expressions
+           all decls
+           all constants
 */
 
 #undef DEFTREESTRUCT
@@ -1363,6 +1371,10 @@ extern void omp_clause_range_check_faile
 /* In fixed-point types, means a saturating type.  */
 #define TYPE_SATURATING(NODE) ((NODE)->base.saturating_flag)
 
+/* Nonzero in an expression, a decl, or a constant node if the node is
+   the result of a successful constant-folding.  */
+#define EXPR_FOLDED(NODE) ((NODE)->base.expr_folded_flag)
+
 /* These flags are available for each language front end to use internally.  */
 #define TREE_LANG_FLAG_0(NODE) ((NODE)->base.lang_flag_0)
 #define TREE_LANG_FLAG_1(NODE) ((NODE)->base.lang_flag_1)
@@ -4931,7 +4943,9 @@ extern bool fold_deferring_overflow_warn
 enum operand_equal_flag
 {
   OEP_ONLY_CONST = 1,
-  OEP_PURE_SAME = 2
+  OEP_PURE_SAME = 2,
+  OEP_ALLOW_NULL = 4,      /* Allow comparison of NULL operands.  */
+  OEP_ALLOW_NULL_TYPE = 8  /* Allow comparison of operands without types.  */
 };
 
 extern int operand_equal_p (const_tree, const_tree, unsigned int);
Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c	(revision 161236)
+++ gcc/fold-const.c	(working copy)
@@ -2404,22 +2404,45 @@ combine_comparisons (location_t loc,
 
    If OEP_PURE_SAME is set, then pure functions with identical arguments
    are considered the same.  It is used when the caller has other ways
-   to ensure that global memory is unchanged in between.  */
+   to ensure that global memory is unchanged in between.
+
+   If OEP_ALLOW_NULL is set, this routine will not crash on NULL operands,
+   and two NULL operands are considered equal. This flag is usually set
+   in the context of frontend when ARG0 and/or ARG1 may be NULL mostly due
+   to recursion on partially built expressions (e.g. a CAST_EXPR on a NULL
+   tree.) In this case, we certainly don't want the compiler to crash and
+   it's OK to consider two NULL operands equal. On the other hand, when
+   called in the context of code generation and optimization, if NULL
+   operands are not expected, silently ignoring them could be dangerous
+   and might cause problems downstream that are hard to find/debug. In that
+   case, the flag should probably not be set.  */
 
 int
 operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
 {
+  /* If either is NULL, they must be both NULL to be equal. We only do this
+     check when OEP_ALLOW_NULL is set.  */
+  if ((flags & OEP_ALLOW_NULL) && (!arg0 || !arg1))
+    return arg0 == arg1;
+
+  /* Similar, if either does not have a type (like a released SSA name, or
+     an operand whose type depends on a template parameter), they aren't
+     equal, unless OEP_ALLOW_NULL_TYPE is set, in which case we will continue
+     to compare the operands only when both don't have a type.  */
+  if (!TREE_TYPE (arg0) || !TREE_TYPE (arg1))
+    {
+      if (((~flags) & OEP_ALLOW_NULL_TYPE)
+          || ((TREE_TYPE (arg0) && !TREE_TYPE (arg1))
+              || (!TREE_TYPE (arg0) && TREE_TYPE (arg1))))
+        return 0;
+    }
+
   /* If either is ERROR_MARK, they aren't equal.  */
   if (TREE_CODE (arg0) == ERROR_MARK || TREE_CODE (arg1) == ERROR_MARK
       || TREE_TYPE (arg0) == error_mark_node
       || TREE_TYPE (arg1) == error_mark_node)
     return 0;
 
-  /* Similar, if either does not have a type (like a released SSA name), 
-     they aren't equal.  */
-  if (!TREE_TYPE (arg0) || !TREE_TYPE (arg1))
-    return 0;
-
   /* Check equality of integer constants before bailing out due to
      precision differences.  */
   if (TREE_CODE (arg0) == INTEGER_CST && TREE_CODE (arg1) == INTEGER_CST)
@@ -2430,19 +2453,25 @@ operand_equal_p (const_tree arg0, const_
      because they may change the signedness of the arguments.  As pointers
      strictly don't have a signedness, require either two pointers or
      two non-pointers as well.  */
-  if (TYPE_UNSIGNED (TREE_TYPE (arg0)) != TYPE_UNSIGNED (TREE_TYPE (arg1))
-      || POINTER_TYPE_P (TREE_TYPE (arg0)) != POINTER_TYPE_P (TREE_TYPE (arg1)))
+  if (TREE_TYPE (arg0)
+      && (TYPE_UNSIGNED (TREE_TYPE (arg0)) != TYPE_UNSIGNED (TREE_TYPE (arg1))
+          || (POINTER_TYPE_P (TREE_TYPE (arg0))
+              != POINTER_TYPE_P (TREE_TYPE (arg1)))))
     return 0;
 
   /* We cannot consider pointers to different address space equal.  */
-  if (POINTER_TYPE_P (TREE_TYPE (arg0)) && POINTER_TYPE_P (TREE_TYPE (arg1))
-      && (TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg0)))
-	  != TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg1)))))
+  if (TREE_TYPE (arg0)
+      && (POINTER_TYPE_P (TREE_TYPE (arg0))
+          && POINTER_TYPE_P (TREE_TYPE (arg1))
+          && (TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg0)))
+              != TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg1))))))
     return 0;
 
   /* If both types don't have the same precision, then it is not safe
      to strip NOPs.  */
-  if (TYPE_PRECISION (TREE_TYPE (arg0)) != TYPE_PRECISION (TREE_TYPE (arg1)))
+  if (TREE_TYPE (arg0)
+      && (TYPE_PRECISION (TREE_TYPE (arg0))
+          != TYPE_PRECISION (TREE_TYPE (arg1))))
     return 0;
 
   STRIP_NOPS (arg0);
@@ -2467,9 +2496,10 @@ operand_equal_p (const_tree arg0, const_
   if (TREE_CODE (arg0) != TREE_CODE (arg1)
       /* This is needed for conversions and for COMPONENT_REF.
 	 Might as well play it safe and always test this.  */
-      || TREE_CODE (TREE_TYPE (arg0)) == ERROR_MARK
-      || TREE_CODE (TREE_TYPE (arg1)) == ERROR_MARK
-      || TYPE_MODE (TREE_TYPE (arg0)) != TYPE_MODE (TREE_TYPE (arg1)))
+      || (TREE_TYPE (arg0)
+          && (TREE_CODE (TREE_TYPE (arg0)) == ERROR_MARK
+              || TREE_CODE (TREE_TYPE (arg1)) == ERROR_MARK
+              || TYPE_MODE (TREE_TYPE (arg0)) != TYPE_MODE (TREE_TYPE (arg1)))))
     return 0;
 
   /* If ARG0 and ARG1 are the same SAVE_EXPR, they are necessarily equal.
@@ -2502,7 +2532,8 @@ operand_equal_p (const_tree arg0, const_
 	  return 1;
 
 
-	if (!HONOR_SIGNED_ZEROS (TYPE_MODE (TREE_TYPE (arg0))))
+	if (TREE_TYPE (arg0)
+            && !HONOR_SIGNED_ZEROS (TYPE_MODE (TREE_TYPE (arg0))))
 	  {
 	    /* If we do not distinguish between signed and unsigned zero,
 	       consider them equal.  */
@@ -2570,8 +2601,9 @@ operand_equal_p (const_tree arg0, const_
         {
 	CASE_CONVERT:
         case FIX_TRUNC_EXPR:
-	  if (TYPE_UNSIGNED (TREE_TYPE (arg0))
-	      != TYPE_UNSIGNED (TREE_TYPE (arg1)))
+	  if (TREE_TYPE (arg0)
+              && (TYPE_UNSIGNED (TREE_TYPE (arg0))
+                  != TYPE_UNSIGNED (TREE_TYPE (arg1))))
 	    return 0;
 	  break;
 	default:
@@ -7653,8 +7685,8 @@ build_fold_addr_expr_loc (location_t loc
    OP0.  Return the folded expression if folding is successful.
    Otherwise, return NULL_TREE.  */
 
-tree
-fold_unary_loc (location_t loc, enum tree_code code, tree type, tree op0)
+static tree
+fold_unary_loc_1 (location_t loc, enum tree_code code, tree type, tree op0)
 {
   tree tem;
   tree arg0;
@@ -8302,6 +8334,41 @@ fold_unary_loc (location_t loc, enum tre
     } /* switch (code) */
 }
 
+/* Given an expression tree EXP, set the EXPR_FOLDED flag, and if it is
+   a nop, recursively set the EXPR_FOLDED flag of its operand.  */
+
+static void
+set_expr_folded_flag (tree exp)
+{
+  EXPR_FOLDED (exp) = 1;
+
+  /* If EXP is a nop (i.e. NON_LVALUE_EXPRs and NOP_EXPRs), we need to
+     recursively set the EXPR_FOLDED flag of its operand because the 
+     expression will be stripped later.  */
+  while ((CONVERT_EXPR_P (exp)
+          || TREE_CODE (exp) == NON_LVALUE_EXPR)
+	 && TREE_OPERAND (exp, 0) != error_mark_node)
+    {
+      exp = TREE_OPERAND (exp, 0);
+      EXPR_FOLDED (exp) = 1;
+    }
+}
+
+/* Fold a unary expression of code CODE and type TYPE with operand
+   OP0.  Return the folded expression if folding is successful.
+   Otherwise, return NULL_TREE.
+   This is a wrapper around fold_unary_loc_1 function (which does the
+   actual folding). Set the EXPR_FOLDED flag of the folded expression
+   if folding is successful.  */
+
+tree
+fold_unary_loc (location_t loc, enum tree_code code, tree type, tree op0)
+{
+  tree tem = fold_unary_loc_1 (loc, code, type, op0);
+  if (tem)
+    set_expr_folded_flag (tem);
+  return tem;
+}
 
 /* If the operation was a conversion do _not_ mark a resulting constant
    with TREE_OVERFLOW if the original constant was not.  These conversions
@@ -9394,8 +9461,8 @@ get_pointer_modulus_and_residue (tree ex
    Return the folded expression if folding is successful.  Otherwise,
    return NULL_TREE.  */
 
-tree
-fold_binary_loc (location_t loc,
+static tree
+fold_binary_loc_1 (location_t loc,
 	     enum tree_code code, tree type, tree op0, tree op1)
 {
   enum tree_code_class kind = TREE_CODE_CLASS (code);
@@ -13066,6 +13133,24 @@ fold_binary_loc (location_t loc,
   return tem;
 }
 
+/* Fold a binary expression of code CODE and type TYPE with operands
+   OP0 and OP1.  LOC is the location of the resulting expression.
+   Return the folded expression if folding is successful.  Otherwise,
+   return NULL_TREE.
+   This is a wrapper around fold_binary_loc_1 function (which does the
+   actual folding). Set the EXPR_FOLDED flag of the folded expression
+   if folding is successful.  */
+
+tree 
+fold_binary_loc (location_t loc,
+                 enum tree_code code, tree type, tree op0, tree op1)
+{
+  tree tem = fold_binary_loc_1 (loc, code, type, op0, op1);
+  if (tem)
+    set_expr_folded_flag (tem);
+  return tem;
+}
+
 /* Callback for walk_tree, looking for LABEL_EXPR.  Return *TP if it is
    a LABEL_EXPR; otherwise return NULL_TREE.  Do not check the subtrees
    of GOTO_EXPR.  */
@@ -13102,8 +13187,8 @@ contains_label_p (tree st)
    OP0, OP1, and OP2.  Return the folded expression if folding is
    successful.  Otherwise, return NULL_TREE.  */
 
-tree
-fold_ternary_loc (location_t loc, enum tree_code code, tree type,
+static tree
+fold_ternary_loc_1 (location_t loc, enum tree_code code, tree type,
 	      tree op0, tree op1, tree op2)
 {
   tree tem;
@@ -13438,6 +13523,23 @@ fold_ternary_loc (location_t loc, enum t
     } /* switch (code) */
 }
 
+/* Fold a ternary expression of code CODE and type TYPE with operands
+   OP0, OP1, and OP2.  Return the folded expression if folding is
+   successful.  Otherwise, return NULL_TREE.
+   This is a wrapper around fold_ternary_loc_1 function (which does the
+   actual folding). Set the EXPR_FOLDED flag of the folded expression
+   if folding is successful.  */
+
+tree
+fold_ternary_loc (location_t loc, enum tree_code code, tree type,
+                  tree op0, tree op1, tree op2)
+{
+  tree tem = fold_ternary_loc_1 (loc, code, type, op0, op1, op2);
+  if (tem)
+    set_expr_folded_flag (tem);
+  return tem;
+}
+
 /* Perform constant folding and related simplification of EXPR.
    The related simplifications include x*1 => x, x*0 => 0, etc.,
    and application of the associative law.
Index: gcc/omega.c
===================================================================
--- gcc/omega.c	(revision 161236)
+++ gcc/omega.c	(working copy)
@@ -2213,7 +2213,9 @@ omega_eliminate_redundant (omega_pb pb, 
 			  || pb->geqs[e3].color == omega_red)
 			goto nextE3;
 
-		      alpha3 = alpha3;
+                      /* Dubious self-assignment. */
+		      /* alpha3 = alpha3; */
+
 		      /* verify alpha1*v1+alpha2*v2 = alpha3*v3 */
 		      for (k = pb->num_vars; k >= 1; k--)
 			if (alpha3 * pb->geqs[e3].coef[k]
Index: gcc/cgraphunit.c
===================================================================
--- gcc/cgraphunit.c	(revision 161236)
+++ gcc/cgraphunit.c	(working copy)
@@ -2128,7 +2128,7 @@ cgraph_copy_node_for_versioning (struct 
    new_version->local.local = true;
    new_version->local.vtable_method = false;
    new_version->global = old_version->global;
-   new_version->rtl = new_version->rtl;
+   new_version->rtl = old_version->rtl;
    new_version->reachable = true;
    new_version->count = old_version->count;
 
Index: gcc/testsuite/gcc.dg/wself-assign-3.c
===================================================================
--- gcc/testsuite/gcc.dg/wself-assign-3.c	(revision 0)
+++ gcc/testsuite/gcc.dg/wself-assign-3.c	(revision 0)
@@ -0,0 +1,9 @@
+/* Test whether -Wself-assign is enabled by -Wall. */
+/* { dg-do compile } */
+/* { dg-options "-Wall" } */
+
+int bar() {
+  int i = 0;
+  i = i;             /* { dg-warning "assigned to itself" } */
+  return i;
+}
Index: gcc/testsuite/gcc.dg/uninit-D-O0.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-D-O0.c	(revision 161236)
+++ gcc/testsuite/gcc.dg/uninit-D-O0.c	(working copy)
@@ -1,9 +1,9 @@
-/* Test we do not warn about initializing variable with self. */
+/* Test we do warn about initializing variable with self. */
 /* { dg-do compile } */
 /* { dg-options "-Wuninitialized" } */
 
 int f()
 {
-  int i = i;
+  int i = i; /* { dg-warning "i" "uninitialized variable warning" }  */
   return i;
 }
Index: gcc/testsuite/gcc.dg/uninit-D.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-D.c	(revision 161236)
+++ gcc/testsuite/gcc.dg/uninit-D.c	(working copy)
@@ -1,9 +1,9 @@
-/* Test we do not warn about initializing variable with self. */
+/* Test we do warn about initializing variable with self. */
 /* { dg-do compile } */
 /* { dg-options "-O -Wuninitialized" } */
 
 int f()
 {
-  int i = i;
+  int i = i; /* { dg-warning "i" "uninitialized variable warning" }  */
   return i;
 }
Index: gcc/testsuite/gcc.dg/plugin/selfassign.c
===================================================================
--- gcc/testsuite/gcc.dg/plugin/selfassign.c	(revision 161236)
+++ gcc/testsuite/gcc.dg/plugin/selfassign.c	(working copy)
@@ -196,7 +196,7 @@ compare_and_warn (gimple stmt, tree lhs,
 /* Check and warn if STMT is a self-assign statement.  */
 
 static void
-warn_self_assign (gimple stmt)
+check_self_assign (gimple stmt)
 {
   tree rhs, lhs;
 
@@ -251,7 +251,7 @@ execute_warn_self_assign (void)
   FOR_EACH_BB (bb)
     {
       for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
-        warn_self_assign (gsi_stmt (gsi));
+        check_self_assign (gsi_stmt (gsi));
     }
 
   return 0;
Index: gcc/testsuite/gcc.dg/wself-assign-1.c
===================================================================
--- gcc/testsuite/gcc.dg/wself-assign-1.c	(revision 0)
+++ gcc/testsuite/gcc.dg/wself-assign-1.c	(revision 0)
@@ -0,0 +1,27 @@
+/* Test the self-assignemnt detection and warning.  */
+/* { dg-do compile } */
+/* { dg-options "-Wself-assign" } */
+
+struct Bar {
+  int b_;
+  int c_;
+};
+
+int g;
+
+int main()
+{
+  struct Bar *bar;
+  int x = x; /* { dg-warning "assigned to itself" } */
+  static int y;
+  struct Bar b_array[5];
+
+  b_array[x+g].b_ = b_array[x+g].b_; /* { dg-warning "assigned to itself" } */
+  g = g; /* { dg-warning "assigned to itself" } */
+  y = y; /* { dg-warning "assigned to itself" } */
+  bar->b_ = bar->b_; /* { dg-warning "assigned to itself" } */
+  x += 0; /* should not warn */
+  y -= 0; /* should not warn */
+  x /= x; /* should not warn */
+  y *= y; /* should not warn */
+}
Index: gcc/testsuite/gcc.dg/uninit-E.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-E.c	(revision 161236)
+++ gcc/testsuite/gcc.dg/uninit-E.c	(working copy)
@@ -1,4 +1,5 @@
-/* Test we do warn about initializing variable with self when -Winit-self is supplied. */
+/* Test we do warn about initializing variable with self and that -Winit-self
+   is preserved for backward compatibility. */
 /* { dg-do compile } */
 /* { dg-options "-O -Wuninitialized -Winit-self" } */
 
Index: gcc/testsuite/gcc.dg/wself-assign-2.c
===================================================================
--- gcc/testsuite/gcc.dg/wself-assign-2.c	(revision 0)
+++ gcc/testsuite/gcc.dg/wself-assign-2.c	(revision 0)
@@ -0,0 +1,24 @@
+/* Test how self-assignment detection handles constant-folding happening */
+/*   when parsing the RHS or the initializer.  */
+/* { dg-do compile } */
+/* { dg-options "-Wself-assign" } */
+
+struct Bar {
+  int b_;
+  float c_;
+};
+
+int g;
+
+int main()
+{
+  struct Bar *bar;
+  int x = x - 0; /* should not warn */
+  static int y;
+  struct Bar b_array[5];
+
+  b_array[x+g].b_ = b_array[x+g].b_ * 1; /* should no warn */
+  g = g + 0; /* should not warn */
+  y = y / 1; /* should not warn */
+  bar->b_ = bar->b_ - 0; /* should not warn  */
+}
Index: gcc/testsuite/gcc.dg/uninit-E-O0.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-E-O0.c	(revision 161236)
+++ gcc/testsuite/gcc.dg/uninit-E-O0.c	(working copy)
@@ -1,4 +1,5 @@
-/* Test we do warn about initializing variable with self when -Winit-self is supplied. */
+/* Test we do warn about initializing variable with self and that -Winit-self
+   is preserved for backward compatibility. */
 /* { dg-do compile } */
 /* { dg-options "-Wuninitialized -Winit-self" } */
 
Index: gcc/testsuite/g++.dg/plugin/selfassign.c
===================================================================
--- gcc/testsuite/g++.dg/plugin/selfassign.c	(revision 161236)
+++ gcc/testsuite/g++.dg/plugin/selfassign.c	(working copy)
@@ -196,7 +196,7 @@ compare_and_warn (gimple stmt, tree lhs,
 /* Check and warn if STMT is a self-assign statement.  */
 
 static void
-warn_self_assign (gimple stmt)
+check_self_assign (gimple stmt)
 {
   tree rhs, lhs;
 
@@ -251,7 +251,7 @@ execute_warn_self_assign (void)
   FOR_EACH_BB (bb)
     {
       for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
-        warn_self_assign (gsi_stmt (gsi));
+        check_self_assign (gsi_stmt (gsi));
     }
 
   return 0;
Index: gcc/testsuite/g++.dg/warn/Wself-assign-1.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wself-assign-1.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/Wself-assign-1.C	(revision 0)
@@ -0,0 +1,54 @@
+// Test the self-assignemnt detection and warning.
+// { dg-do compile }
+// { dg-options "-Wself-assign" }
+
+class Foo {
+ private:
+  int a_;
+
+ public:
+  Foo() : a_(a_) {} // { dg-warning "assigned to itself" }
+
+  void setA(int a) {
+    a_ = a_; // { dg-warning "assigned to itself" }
+  }
+
+  void operator=(Foo& rhs) {
+    this->a_ = rhs.a_;
+  }
+};
+
+struct Bar {
+  int b_;
+  int c_;
+};
+
+int g = g; // { dg-warning "assigned to itself" }
+Foo foo = foo; // { dg-warning "assigned to itself" }
+
+int func()
+{
+  Bar *bar1, bar2;
+  Foo local_foo;
+  int x = x; // { dg-warning "assigned to itself" }
+  static int y = y; // { dg-warning "assigned to itself" }
+  float *f;
+  Bar bar_array[5];
+  char n;
+  int overflow;
+
+  *f = *f; // { dg-warning "assigned to itself" }
+  bar1->b_ = bar1->b_; // { dg-warning "assigned to itself" }
+  bar2.c_ = bar2.c_; // { dg-warning "assigned to itself" }
+  local_foo = local_foo; // { dg-warning "assigned to itself" }
+  foo = foo; // { dg-warning "assigned to itself" }
+  foo.setA(5);
+  bar_array[3].c_ = bar_array[3].c_; // { dg-warning "assigned to itself" }
+  bar_array[x+g].b_ = bar_array[x+g].b_; // { dg-warning "assigned to itself" }
+  y = x;
+  x = y;
+  x += 0; // should not warn
+  y -= 0; // should not warn
+  x /= x; // should not warn
+  y *= y; // should not warn
+}
Index: gcc/testsuite/g++.dg/warn/Wself-assign-2.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wself-assign-2.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/Wself-assign-2.C	(revision 0)
@@ -0,0 +1,31 @@
+// Test the handling of expressions that depend on template parameters in
+// self-assignemnt detection.
+// { dg-do compile }
+// { dg-options "-Wself-assign" }
+
+template<typename T>
+struct Bar {
+  T x;
+  Bar operator++(int) {
+    Bar tmp = *this;
+    ++x;
+    tmp = tmp; // { dg-warning "assigned to itself" }
+    return tmp;
+  }
+};
+
+template<typename T>
+T DoSomething(T y) {
+  T a[5], *p;
+  Bar<T> b;
+  b.x = b.x; // { dg-warning "assigned to itself" }
+  *p = *p; // { dg-warning "assigned to itself" }
+  a[2] = a[2]; // { dg-warning "assigned to itself" }
+  return *p;
+}
+
+main() {
+  Bar<int> bar;
+  bar++;
+  DoSomething(5);
+}
Index: gcc/testsuite/g++.dg/warn/Wself-assign-3.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wself-assign-3.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/Wself-assign-3.C	(revision 0)
@@ -0,0 +1,35 @@
+// Test how operands_equal_p handles a NULL operand.
+// { dg-do compile }
+// { dg-options "-Wself-assign" }
+
+#include <cstdio>
+
+namespace testing {
+
+class Foo {
+  int f;
+ public:
+  Foo() { printf("Construct Foo\n"); }
+};
+
+class Bar {
+  int b;
+ public:
+  Bar(int x) { printf("Construct Bar\n"); }
+
+  void operator=(const Foo& foo) {
+    printf("Assign Foo to Bar\n");
+  }
+};
+
+}
+
+template <class T>
+void func(T t) {
+  ::testing::Bar(1) = ::testing::Foo(); // used to trigger a segfault
+  ::testing::Foo() = ::testing::Foo(); // { dg-warning "assigned to itself" }
+}
+
+main() {
+  func(2);
+}
Index: gcc/testsuite/g++.dg/warn/Wself-assign-4.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wself-assign-4.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/Wself-assign-4.C	(revision 0)
@@ -0,0 +1,48 @@
+// Test how self-assignment detection handles constant-folding happening
+// when parsing the RHS or the initializer.
+// { dg-do compile }
+// { dg-options "-Wself-assign" }
+
+class Foo {
+ private:
+  int a_;
+
+ public:
+  Foo() : a_(a_+0) {} // should not warn
+
+  void setA(int a) {
+    a_ = a_ + 0; // should not warn
+  }
+
+  void operator=(Foo& rhs) {
+    this->a_ = rhs.a_;
+  }
+};
+
+struct Bar {
+  int b_;
+  float c_;
+};
+
+int g = g * 1; // should not warn
+
+int func()
+{
+  Bar *bar1, bar2;
+  Foo foo;
+  int x = x - 0;        // should not warn
+  static int y = y / 1; // should not warn
+  float *f;
+  Bar bar_array[5];
+
+  *f = *f / 1;             // should not warn
+  bar1->b_ = bar1->b_ * 1; // should not warn
+  bar2.c_ = bar2.c_ - 0;   // should not warn
+  foo.setA(5);
+  bar_array[3].c_ = bar_array[3].c_ * 1;     // should not warn
+  bar_array[x+g].b_ = bar_array[x+g].b_ / 1; // should not warn
+  x += 0;
+  y -= 0;
+  foo = foo;           // { dg-warning "assigned to itself" }
+  foo.operator=(foo);  // should not warn
+}
Index: gcc/testsuite/g++.dg/warn/Wself-assign-5.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wself-assign-5.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/Wself-assign-5.C	(revision 0)
@@ -0,0 +1,21 @@
+// Test whether -Wself-assign is enabled by -Wall.
+// { dg-do compile }
+// { dg-options "-Wall" }
+
+class Foo {
+  int a, b;
+ public:
+  Foo() : a(a) { }   // { dg-warning "assigned to itself" }
+  int foo() { return b; }
+};
+
+int c = c;           // { dg-warning "assigned to itself" }
+
+Foo foo;
+
+int bar() {
+  static int d = d;  // { dg-warning "assigned to itself" }
+  int i = 0;
+  i = i;             // { dg-warning "assigned to itself" }
+  return d;
+}
Index: gcc/cp/init.c
===================================================================
--- gcc/cp/init.c	(revision 161236)
+++ gcc/cp/init.c	(working copy)
@@ -529,8 +529,14 @@ perform_member_init (tree member, tree i
 	init = build_x_compound_expr_from_list (init, ELK_MEM_INIT);
 
       if (init)
-	finish_expr_stmt (cp_build_modify_expr (decl, INIT_EXPR, init,
-						tf_warning_or_error));
+        {
+          finish_expr_stmt (cp_build_modify_expr (decl, INIT_EXPR, init,
+                                                  tf_warning_or_error));
+          /* Check for and warn about self-initialization if -Wself-assign is
+             enabled.  */
+          if (warn_self_assign)
+            check_for_self_assign (input_location, decl, init);
+        }
     }
 
   if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (type))
Index: gcc/cp/parser.c
===================================================================
--- gcc/cp/parser.c	(revision 161236)
+++ gcc/cp/parser.c	(working copy)
@@ -6903,6 +6903,12 @@ cp_parser_assignment_expression (cp_pars
 	      if (cp_parser_non_integral_constant_expression (parser,
 							      NIC_ASSIGNMENT))
 		return error_mark_node;
+
+              /* Check for and warn about self-assignment if -Wself-assign is
+                 enabled and the assignment operator is "=".  */
+              if (warn_self_assign && assignment_operator == NOP_EXPR)
+                check_for_self_assign (input_location, expr, rhs);
+
 	      /* Build the assignment expression.  */
 	      expr = build_x_modify_expr (expr,
 					  assignment_operator,
@@ -14075,6 +14081,10 @@ cp_parser_init_declarator (cp_parser* pa
 			 `explicit' constructor cannot be used.  */
 		      ((is_direct_init || !is_initialized)
 		       ? 0 : LOOKUP_ONLYCONVERTING));
+      /* Check for and warn about self-initialization if -Wself-assign is
+         enabled.  */
+      if (warn_self_assign && initializer)
+        check_for_self_assign (input_location, decl, initializer);
     }
   else if ((cxx_dialect != cxx98) && friend_p
 	   && decl && TREE_CODE (decl) == FUNCTION_DECL)
Index: gcc/tree-ssa-ccp.c
===================================================================
--- gcc/tree-ssa-ccp.c	(revision 161236)
+++ gcc/tree-ssa-ccp.c	(working copy)
@@ -728,9 +728,8 @@ ccp_lattice_meet (prop_value_t *val1, pr
 	 Ci M Cj = VARYING	if (i != j)
 
          If these two values come from memory stores, make sure that
-	 they come from the same memory reference.  */
-      val1->lattice_val = CONSTANT;
-      val1->value = val1->value;
+	 they come from the same memory reference.
+         Nothing to do.  VAL1 already contains the value we want.  */
     }
   else
     {
Index: gcc/dbxout.c
===================================================================
--- gcc/dbxout.c	(revision 161236)
+++ gcc/dbxout.c	(working copy)
@@ -288,9 +288,12 @@ static const char *base_input_file;
 #endif
 
 /* A C expression for the integer offset value of an argument (N_PSYM)
-   having address X (an RTX).  The nominal offset is OFFSET.  */
+   having address X (an RTX).  The nominal offset is OFFSET.
+   Note that we use OFFSET + 0 here to avoid the self-assign warning
+   when the macro is called in a context like
+   number = DEBUGGER_ARG_OFFSET(number, X)  */
 #ifndef DEBUGGER_ARG_OFFSET
-#define DEBUGGER_ARG_OFFSET(OFFSET, X) (OFFSET)
+#define DEBUGGER_ARG_OFFSET(OFFSET, X) (OFFSET + 0)
 #endif
 
 /* This obstack holds the stab string currently being constructed.  We
Index: gcc/c-parser.c
===================================================================
--- gcc/c-parser.c	(revision 161236)
+++ gcc/c-parser.c	(working copy)
@@ -1297,6 +1297,11 @@ c_parser_declaration_or_fndef (c_parser 
 		  maybe_warn_string_init (TREE_TYPE (d), init);
 		  finish_decl (d, init_loc, init.value,
 		      	       init.original_type, asm_name);
+                  /* Check for and warn about self-initialization if
+                     -Wself-assign is enabled. Don't warn if there is
+                     already an error for the initializer.  */
+                  if (warn_self_assign && DECL_INITIAL (d) != error_mark_node)
+                    check_for_self_assign (here, d, init.value);
 		}
 	    }
 	  else
@@ -4767,7 +4772,13 @@ c_parser_expr_no_commas (c_parser *parse
 				 code, exp_location, rhs.value,
 				 rhs.original_type);
   if (code == NOP_EXPR)
-    ret.original_code = MODIFY_EXPR;
+    {
+      ret.original_code = MODIFY_EXPR;
+      /* Check for and warn about self-assignment if -Wself-assign is
+         enabled.  */
+      if (warn_self_assign)
+        check_for_self_assign (op_location, lhs.value, rhs.value);
+    }
   else
     {
       TREE_NO_WARNING (ret.value) = 1;
Index: gcc/config/i386/i386.c
===================================================================
--- gcc/config/i386/i386.c	(revision 161236)
+++ gcc/config/i386/i386.c	(working copy)
@@ -29379,7 +29379,7 @@ ix86_vectorize_builtin_vec_perm (tree ve
   tree itype = TREE_TYPE (vec_type);
   bool u = TYPE_UNSIGNED (itype);
   enum machine_mode vmode = TYPE_MODE (vec_type);
-  enum ix86_builtins fcode = fcode; /* Silence bogus warning.  */
+  enum ix86_builtins fcode;
   bool ok = TARGET_SSE2;
 
   switch (vmode)

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

[Le-Chun Wu]

Ping?

On Thu, Jun 24, 2010 at 3:54 PM, Le-Chun Wu <lcwu@google.com> wrote:
> Attached please find the revised patch for the new flag -Wself-assign.
> Comparing with the previous patch, this one puts -Winit-self back but
> keeps it as a no-op, and resurrects the -Winit-self test cases to make
> sure the flag is still accepted by GCC. It also fixes the following
> self-assign/self-init warnings in GCC source code when bootstrapping.
> (The flag actually identified a real bug/typo in cgraphunit.c. Yeah!
> :-)) Relevant maintainers of these files are also cc'ed in this email.
>
>        * 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)
>
> Again, bootstrapped and tested on x86_64-linux-gnu. OK for trunk?
>
> Le-chun
>
> gcc/ChangeLog
>
> 2010-06-24  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.
>        * dbxout.c (DEBUGGER_ARG_OFFSET): Change OFFSET to OFFSET+0 to avoid
>        self-assign warning.
>        * omega.c (omega_eliminate_redundant): Remove a self-assign statement.
>        * tree-ssa-ccp.c (ccp_lattice_meet): Remove a self-assign statement
>        and an unnecessary assignment.
>        * cgraphunit.c (cgraph_copy_node_for_versioning): Fix a typo detected
>        by -Wself-assign.
>        * config/i386/i386.c (ix86_vectorize_builtin_vec_perm): Remove
>        unnecessary self-init.
>
> gcc/c-family/ChangeLog
>
> 2010-06-24  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-06-24  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
>
>        * 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.
>

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

[Manuel López-Ibáñez]

--- 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.

>        * 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.

Manuel.

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

[Jason Merrill]

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?

Incidentally, I've been thinking lately that we really ought to wrap 
uses of decls in another tree so that we have location information for 
the use, not just the decl itself; thus the introduction of 
mark_lvalue_use and mark_rvalue_use in the C++ front end (which are 
currently just placeholders).  That might help with this issue as well, 
but of course would be a significant chunk of work.

Jason

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

[Le-Chun Wu]

[-- Attachment #1: Type: text/plain, Size: 6325 bytes --]

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.

[-- Attachment #2: selfassign.patch --]
[-- Type: text/x-patch, Size: 36322 bytes --]

Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 162385)
+++ gcc/doc/invoke.texi	(working copy)
@@ -242,8 +242,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wformat-security  -Wformat-y2k @gol
 -Wframe-larger-than=@var{len} -Wjump-misses-init -Wignored-qualifiers @gol
 -Wimplicit  -Wimplicit-function-declaration  -Wimplicit-int @gol
--Winit-self  -Winline @gol
--Wno-int-to-pointer-cast -Wno-invalid-offsetof @gol
+-Winline -Wno-int-to-pointer-cast -Wno-invalid-offsetof @gol
 -Winvalid-pch -Wlarger-than=@var{len}  -Wunsafe-loop-optimizations @gol
 -Wlogical-op -Wlong-long @gol
 -Wmain  -Wmissing-braces  -Wmissing-field-initializers @gol
@@ -254,7 +253,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wparentheses  -Wpedantic-ms-format -Wno-pedantic-ms-format @gol
 -Wpointer-arith  -Wno-pointer-to-int-cast @gol
 -Wredundant-decls @gol
--Wreturn-type  -Wsequence-point  -Wshadow @gol
+-Wreturn-type -Wself-assign -Wsequence-point  -Wshadow @gol
 -Wsign-compare  -Wsign-conversion  -Wstack-protector @gol
 -Wstrict-aliasing -Wstrict-aliasing=n @gol
 -Wstrict-overflow -Wstrict-overflow=@var{n} @gol
@@ -2944,6 +2943,7 @@ Options} and @ref{Objective-C and Object
 -Wpointer-sign  @gol
 -Wreorder   @gol
 -Wreturn-type  @gol
+-Wself-assign @r{(only in C/C++)}  @gol
 -Wsequence-point  @gol
 -Wsign-compare @r{(only in C++)}  @gol
 -Wstrict-aliasing  @gol
@@ -3144,24 +3144,6 @@ requiring a non-null value by the @code{
 @option{-Wnonnull} is included in @option{-Wall} and @option{-Wformat}.  It
 can be disabled with the @option{-Wno-nonnull} option.
 
-@item -Winit-self @r{(C, C++, Objective-C and Objective-C++ only)}
-@opindex Winit-self
-@opindex Wno-init-self
-Warn about uninitialized variables which are initialized with themselves.
-Note this option can only be used with the @option{-Wuninitialized} option.
-
-For example, GCC will warn about @code{i} being uninitialized in the
-following snippet only when @option{-Winit-self} has been specified:
-@smallexample
-@group
-int f()
-@{
-  int i = i;
-  return i;
-@}
-@end group
-@end smallexample
-
 @item -Wimplicit-int @r{(C and Objective-C only)}
 @opindex Wimplicit-int
 @opindex Wno-implicit-int
@@ -3330,6 +3312,53 @@ definitions, may be found on the GCC rea
 
 This warning is enabled by @option{-Wall} for C and C++.
 
+@item -Wself-assign
+@opindex Wself-assign
+@opindex Wno-self-assign
+Warn about self-assignment and self-initialization. This warning is intended
+for detecting accidental self-assignment due to typos, and therefore does
+not warn on a statement that is semantically a self-assignment after
+constant folding. Here is an example of what will trigger a self-assign
+warning and what will not:
+
+@smallexample
+@group
+void func()
+@{
+   int i = 2;
+   int x = x;   /* warn */
+   float f = 5.0;
+   double a[3];
+
+   i = i + 0;   /* not warn */
+   f = f / 1;   /* not warn */
+   a[1] = a[1]; /* warn */
+   i += 0;      /* not warn */
+@}
+@end group
+@end smallexample
+
+There are cases where self-assignment might be intentional. For example,
+a C++ programmers might write code to test whether an overloaded
+@code{operator=} works when the same object is assigned to itself.
+One way to work around the self-assign warning in such case is using
+the functional form @code{object.operator=(object)} instead of the
+assignment form @code{object = object}, as shown in the following example.
+
+@smallexample
+@group
+void test_func()
+@{
+   MyType t;
+
+   t.operator=(t);  // not warn
+   t = t;           // warn
+@}
+@end group
+@end smallexample
+
+This warning is enabled by @option{-Wall} for C and C++.
+
 @item -Wreturn-type
 @opindex Wreturn-type
 @opindex Wno-return-type
@@ -3453,6 +3482,10 @@ This warning is enabled by @option{-Wall
 To suppress this warning use the @samp{unused} attribute
 (@pxref{Variable Attributes}).
 
+Note that a classic way to avoid @option{-Wunused-variable} warning is
+using @code{x = x}, but that does not work with @option{-Wself-assign}.
+Use @code{(void) x} or @code{static_cast<void>(x)} instead.
+
 @item -Wunused-value
 @opindex Wunused-value
 @opindex Wno-unused-value
@@ -3482,9 +3515,6 @@ or if a variable may be clobbered by a @
 warn if a non-static reference or non-static @samp{const} member
 appears in a class without constructors.
 
-If you want to warn about code which uses the uninitialized value of the
-variable in its own initializer, use the @option{-Winit-self} option.
-
 These warnings occur for individual uninitialized or clobbered
 elements of structure, union or array variables as well as for
 variables which are uninitialized or clobbered as a whole.  They do
Index: gcc/c-family/c-gimplify.c
===================================================================
--- gcc/c-family/c-gimplify.c	(revision 162385)
+++ gcc/c-family/c-gimplify.c	(working copy)
@@ -172,18 +172,5 @@ int
 c_gimplify_expr (tree *expr_p, gimple_seq *pre_p ATTRIBUTE_UNUSED,
 		 gimple_seq *post_p ATTRIBUTE_UNUSED)
 {
-  enum tree_code code = TREE_CODE (*expr_p);
-
-  /* This is handled mostly by gimplify.c, but we have to deal with
-     not warning about int x = x; as it is a GCC extension to turn off
-     this warning but only if warn_init_self is zero.  */
-  if (code == DECL_EXPR
-      && TREE_CODE (DECL_EXPR_DECL (*expr_p)) == VAR_DECL
-      && !DECL_EXTERNAL (DECL_EXPR_DECL (*expr_p))
-      && !TREE_STATIC (DECL_EXPR_DECL (*expr_p))
-      && (DECL_INITIAL (DECL_EXPR_DECL (*expr_p)) == DECL_EXPR_DECL (*expr_p))
-      && !warn_init_self)
-    TREE_NO_WARNING (DECL_EXPR_DECL (*expr_p)) = 1;
-
   return GS_UNHANDLED;
 }
Index: gcc/c-family/c.opt
===================================================================
--- gcc/c-family/c.opt	(revision 162385)
+++ gcc/c-family/c.opt	(working copy)
@@ -259,8 +259,8 @@ C C++ Var(warn_ignored_qualifiers) Init(
 Warn whenever type qualifiers are ignored.
 
 Winit-self
-C ObjC C++ ObjC++ Var(warn_init_self) Warning
-Warn about variables which are initialized to themselves
+C ObjC C++ ObjC++
+Does nothing. Preserved for backward compatibility.
 
 Wimplicit
 C ObjC Var(warn_implicit) Init(-1) Warning
@@ -425,6 +425,10 @@ Wreturn-type
 C ObjC C++ ObjC++ Var(warn_return_type) Warning
 Warn whenever a function's return type defaults to \"int\" (C), or about inconsistent return types (C++)
 
+Wself-assign
+C C++ Var(warn_self_assign) Init(0) Warning
+Warn when a variable is assigned to itself
+
 Wselector
 ObjC ObjC++ Var(warn_selector) Warning
 Warn if a selector has multiple methods
Index: gcc/c-family/c-opts.c
===================================================================
--- gcc/c-family/c-opts.c	(revision 162385)
+++ 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;
+      handle_option (OPT_Wself_assign, value, NULL, c_family_lang_mask, kind);
 
       if (!c_dialect_cxx ())
 	{
Index: gcc/c-family/c-common.c
===================================================================
--- gcc/c-family/c-common.c	(revision 162385)
+++ gcc/c-family/c-common.c	(working copy)
@@ -9305,4 +9305,30 @@ make_tree_vector_copy (const VEC(tree,gc
   return ret;
 }
 
+/* Check for and warn about self-assignment or self-initialization.
+   LHS and RHS are the tree nodes for the left-hand side and right-hand side
+   of the assignment or initialization we are checking.
+   LOCATION is the source location for RHS.  */
+
+void
+check_for_self_assign (location_t location, tree lhs, tree rhs)
+{
+  /* Only emit a warning if RHS is not a folded expression so that we don't
+     warn on something like x = x / 1.  */
+  if (!EXPR_FOLDED (rhs)) {
+    if (operand_equal_p (lhs, rhs,
+                         OEP_PURE_SAME | OEP_ALLOW_NULL | OEP_ALLOW_NULL_TYPE))
+      warning_at (location, OPT_Wself_assign, "%qE is assigned to itself", lhs);
+  }
+  else
+    /* Clear the expr_folded flag as DECLS and CONSTANTS are shared by
+       expressions/statements. Without doing this, the compiler will not
+       warn on the second statement in the following example:
+
+         x = x + 0;
+         x = x;
+    */
+    set_expr_folded_flag (rhs, 0);
+}
+
 #include "gt-c-family-c-common.h"
Index: gcc/c-family/c-common.h
===================================================================
--- gcc/c-family/c-common.h	(revision 162385)
+++ gcc/c-family/c-common.h	(working copy)
@@ -900,6 +900,7 @@ extern VEC(tree,gc) *make_tree_vector (v
 extern void release_tree_vector (VEC(tree,gc) *);
 extern VEC(tree,gc) *make_tree_vector_single (tree);
 extern VEC(tree,gc) *make_tree_vector_copy (const VEC(tree,gc) *);
+extern void check_for_self_assign (location_t, tree, tree);
 
 /* In c-gimplify.c  */
 extern void c_genericize (tree);
Index: gcc/tree.h
===================================================================
--- gcc/tree.h	(revision 162385)
+++ gcc/tree.h	(working copy)
@@ -387,8 +387,9 @@ struct GTY(()) tree_base {
   unsigned visited : 1;
   unsigned packed_flag : 1;
   unsigned user_align : 1;
+  unsigned expr_folded_flag : 1;
 
-  unsigned spare : 13;
+  unsigned spare : 12;
 
   /* This field is only used with type nodes; the only reason it is present
      in tree_base instead of tree_type is to save space.  The size of the
@@ -626,6 +627,16 @@ struct GTY(()) tree_common {
 
        SSA_NAME_IS_DEFAULT_DEF in
            SSA_NAME
+
+   expr_folded_flag:
+
+       EXPR_FOLDED in
+           all expressions
+           all decls
+           all constants
+       (This flag is used only for -Wself-assign warning and cleared after
+       the check (see check_for_self_assign in c-common.c). Don't use it in
+       other places/passes.)
 */
 
 #undef DEFTREESTRUCT
@@ -1350,6 +1361,12 @@ extern void omp_clause_range_check_faile
 /* In fixed-point types, means a saturating type.  */
 #define TYPE_SATURATING(NODE) ((NODE)->base.saturating_flag)
 
+/* Nonzero in an expression, a decl, or a constant node if the node is
+   the result of a successful constant-folding. This flag is used only
+   for -Wself-assign warning and cleared after the check (see
+   check_for_self_assign in c-common.c). Don't use it in other places.  */
+#define EXPR_FOLDED(NODE) ((NODE)->base.expr_folded_flag)
+
 /* These flags are available for each language front end to use internally.  */
 #define TREE_LANG_FLAG_0(NODE) ((NODE)->base.lang_flag_0)
 #define TREE_LANG_FLAG_1(NODE) ((NODE)->base.lang_flag_1)
@@ -4862,6 +4879,9 @@ extern int folding_initializer;
 extern int native_encode_expr (const_tree, unsigned char *, int);
 extern tree native_interpret_expr (tree, const unsigned char *, int);
 
+/* Set/reset the expr_folded flag.  */
+extern void set_expr_folded_flag (tree, int);
+
 /* Fold constants as much as possible in an expression.
    Returns the simplified expression.
    Acts only on the top level of the expression;
@@ -4924,7 +4944,9 @@ extern bool fold_deferring_overflow_warn
 enum operand_equal_flag
 {
   OEP_ONLY_CONST = 1,
-  OEP_PURE_SAME = 2
+  OEP_PURE_SAME = 2,
+  OEP_ALLOW_NULL = 4,      /* Allow comparison of NULL operands.  */
+  OEP_ALLOW_NULL_TYPE = 8  /* Allow comparison of operands without types.  */
 };
 
 extern int operand_equal_p (const_tree, const_tree, unsigned int);
Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c	(revision 162385)
+++ gcc/fold-const.c	(working copy)
@@ -2388,22 +2388,45 @@ combine_comparisons (location_t loc,
 
    If OEP_PURE_SAME is set, then pure functions with identical arguments
    are considered the same.  It is used when the caller has other ways
-   to ensure that global memory is unchanged in between.  */
+   to ensure that global memory is unchanged in between.
+
+   If OEP_ALLOW_NULL is set, this routine will not crash on NULL operands,
+   and two NULL operands are considered equal. This flag is usually set
+   in the context of frontend when ARG0 and/or ARG1 may be NULL mostly due
+   to recursion on partially built expressions (e.g. a CAST_EXPR on a NULL
+   tree.) In this case, we certainly don't want the compiler to crash and
+   it's OK to consider two NULL operands equal. On the other hand, when
+   called in the context of code generation and optimization, if NULL
+   operands are not expected, silently ignoring them could be dangerous
+   and might cause problems downstream that are hard to find/debug. In that
+   case, the flag should probably not be set.  */
 
 int
 operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
 {
+  /* If either is NULL, they must be both NULL to be equal. We only do this
+     check when OEP_ALLOW_NULL is set.  */
+  if ((flags & OEP_ALLOW_NULL) && (!arg0 || !arg1))
+    return arg0 == arg1;
+
+  /* Similar, if either does not have a type (like a released SSA name, or
+     an operand whose type depends on a template parameter), they aren't
+     equal, unless OEP_ALLOW_NULL_TYPE is set, in which case we will continue
+     to compare the operands only when both don't have a type.  */
+  if (!TREE_TYPE (arg0) || !TREE_TYPE (arg1))
+    {
+      if (((~flags) & OEP_ALLOW_NULL_TYPE)
+          || ((TREE_TYPE (arg0) && !TREE_TYPE (arg1))
+              || (!TREE_TYPE (arg0) && TREE_TYPE (arg1))))
+        return 0;
+    }
+
   /* If either is ERROR_MARK, they aren't equal.  */
   if (TREE_CODE (arg0) == ERROR_MARK || TREE_CODE (arg1) == ERROR_MARK
       || TREE_TYPE (arg0) == error_mark_node
       || TREE_TYPE (arg1) == error_mark_node)
     return 0;
 
-  /* Similar, if either does not have a type (like a released SSA name), 
-     they aren't equal.  */
-  if (!TREE_TYPE (arg0) || !TREE_TYPE (arg1))
-    return 0;
-
   /* Check equality of integer constants before bailing out due to
      precision differences.  */
   if (TREE_CODE (arg0) == INTEGER_CST && TREE_CODE (arg1) == INTEGER_CST)
@@ -2414,19 +2437,25 @@ operand_equal_p (const_tree arg0, const_
      because they may change the signedness of the arguments.  As pointers
      strictly don't have a signedness, require either two pointers or
      two non-pointers as well.  */
-  if (TYPE_UNSIGNED (TREE_TYPE (arg0)) != TYPE_UNSIGNED (TREE_TYPE (arg1))
-      || POINTER_TYPE_P (TREE_TYPE (arg0)) != POINTER_TYPE_P (TREE_TYPE (arg1)))
+  if (TREE_TYPE (arg0)
+      && (TYPE_UNSIGNED (TREE_TYPE (arg0)) != TYPE_UNSIGNED (TREE_TYPE (arg1))
+          || (POINTER_TYPE_P (TREE_TYPE (arg0))
+              != POINTER_TYPE_P (TREE_TYPE (arg1)))))
     return 0;
 
   /* We cannot consider pointers to different address space equal.  */
-  if (POINTER_TYPE_P (TREE_TYPE (arg0)) && POINTER_TYPE_P (TREE_TYPE (arg1))
-      && (TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg0)))
-	  != TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg1)))))
+  if (TREE_TYPE (arg0)
+      && (POINTER_TYPE_P (TREE_TYPE (arg0))
+          && POINTER_TYPE_P (TREE_TYPE (arg1))
+          && (TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg0)))
+              != TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (arg1))))))
     return 0;
 
   /* If both types don't have the same precision, then it is not safe
      to strip NOPs.  */
-  if (TYPE_PRECISION (TREE_TYPE (arg0)) != TYPE_PRECISION (TREE_TYPE (arg1)))
+  if (TREE_TYPE (arg0)
+      && (TYPE_PRECISION (TREE_TYPE (arg0))
+          != TYPE_PRECISION (TREE_TYPE (arg1))))
     return 0;
 
   STRIP_NOPS (arg0);
@@ -2451,9 +2480,10 @@ operand_equal_p (const_tree arg0, const_
   if (TREE_CODE (arg0) != TREE_CODE (arg1)
       /* This is needed for conversions and for COMPONENT_REF.
 	 Might as well play it safe and always test this.  */
-      || TREE_CODE (TREE_TYPE (arg0)) == ERROR_MARK
-      || TREE_CODE (TREE_TYPE (arg1)) == ERROR_MARK
-      || TYPE_MODE (TREE_TYPE (arg0)) != TYPE_MODE (TREE_TYPE (arg1)))
+      || (TREE_TYPE (arg0)
+          && (TREE_CODE (TREE_TYPE (arg0)) == ERROR_MARK
+              || TREE_CODE (TREE_TYPE (arg1)) == ERROR_MARK
+              || TYPE_MODE (TREE_TYPE (arg0)) != TYPE_MODE (TREE_TYPE (arg1)))))
     return 0;
 
   /* If ARG0 and ARG1 are the same SAVE_EXPR, they are necessarily equal.
@@ -2486,7 +2516,8 @@ operand_equal_p (const_tree arg0, const_
 	  return 1;
 
 
-	if (!HONOR_SIGNED_ZEROS (TYPE_MODE (TREE_TYPE (arg0))))
+	if (TREE_TYPE (arg0)
+            && !HONOR_SIGNED_ZEROS (TYPE_MODE (TREE_TYPE (arg0))))
 	  {
 	    /* If we do not distinguish between signed and unsigned zero,
 	       consider them equal.  */
@@ -2554,8 +2585,9 @@ operand_equal_p (const_tree arg0, const_
         {
 	CASE_CONVERT:
         case FIX_TRUNC_EXPR:
-	  if (TYPE_UNSIGNED (TREE_TYPE (arg0))
-	      != TYPE_UNSIGNED (TREE_TYPE (arg1)))
+	  if (TREE_TYPE (arg0)
+              && (TYPE_UNSIGNED (TREE_TYPE (arg0))
+                  != TYPE_UNSIGNED (TREE_TYPE (arg1))))
 	    return 0;
 	  break;
 	default:
@@ -7640,8 +7672,8 @@ build_fold_addr_expr_loc (location_t loc
    OP0.  Return the folded expression if folding is successful.
    Otherwise, return NULL_TREE.  */
 
-tree
-fold_unary_loc (location_t loc, enum tree_code code, tree type, tree op0)
+static tree
+fold_unary_loc_1 (location_t loc, enum tree_code code, tree type, tree op0)
 {
   tree tem;
   tree arg0;
@@ -8291,6 +8323,41 @@ fold_unary_loc (location_t loc, enum tre
     } /* switch (code) */
 }
 
+/* Given an expression tree EXP, set the EXPR_FOLDED flag with VALUE,
+   and if it is a nop, recursively set the EXPR_FOLDED flag of its operand.  */
+
+void
+set_expr_folded_flag (tree exp, int value)
+{
+  EXPR_FOLDED (exp) = value;
+
+  /* If EXP is a nop (i.e. NON_LVALUE_EXPRs and NOP_EXPRs), we need to
+     recursively set the EXPR_FOLDED flag of its operand because the 
+     expression will be stripped later.  */
+  while ((CONVERT_EXPR_P (exp)
+          || TREE_CODE (exp) == NON_LVALUE_EXPR)
+	 && TREE_OPERAND (exp, 0) != error_mark_node)
+    {
+      exp = TREE_OPERAND (exp, 0);
+      EXPR_FOLDED (exp) = value;
+    }
+}
+
+/* Fold a unary expression of code CODE and type TYPE with operand
+   OP0.  Return the folded expression if folding is successful.
+   Otherwise, return NULL_TREE.
+   This is a wrapper around fold_unary_loc_1 function (which does the
+   actual folding). Set the EXPR_FOLDED flag of the folded expression
+   if folding is successful.  */
+
+tree
+fold_unary_loc (location_t loc, enum tree_code code, tree type, tree op0)
+{
+  tree tem = fold_unary_loc_1 (loc, code, type, op0);
+  if (tem)
+    set_expr_folded_flag (tem, 1);
+  return tem;
+}
 
 /* If the operation was a conversion do _not_ mark a resulting constant
    with TREE_OVERFLOW if the original constant was not.  These conversions
@@ -9393,8 +9460,8 @@ get_pointer_modulus_and_residue (tree ex
    Return the folded expression if folding is successful.  Otherwise,
    return NULL_TREE.  */
 
-tree
-fold_binary_loc (location_t loc,
+static tree
+fold_binary_loc_1 (location_t loc,
 	     enum tree_code code, tree type, tree op0, tree op1)
 {
   enum tree_code_class kind = TREE_CODE_CLASS (code);
@@ -13095,6 +13162,24 @@ fold_binary_loc (location_t loc,
   return tem;
 }
 
+/* Fold a binary expression of code CODE and type TYPE with operands
+   OP0 and OP1.  LOC is the location of the resulting expression.
+   Return the folded expression if folding is successful.  Otherwise,
+   return NULL_TREE.
+   This is a wrapper around fold_binary_loc_1 function (which does the
+   actual folding). Set the EXPR_FOLDED flag of the folded expression
+   if folding is successful.  */
+
+tree 
+fold_binary_loc (location_t loc,
+                 enum tree_code code, tree type, tree op0, tree op1)
+{
+  tree tem = fold_binary_loc_1 (loc, code, type, op0, op1);
+  if (tem)
+    set_expr_folded_flag (tem, 1);
+  return tem;
+}
+
 /* Callback for walk_tree, looking for LABEL_EXPR.  Return *TP if it is
    a LABEL_EXPR; otherwise return NULL_TREE.  Do not check the subtrees
    of GOTO_EXPR.  */
@@ -13131,8 +13216,8 @@ contains_label_p (tree st)
    OP0, OP1, and OP2.  Return the folded expression if folding is
    successful.  Otherwise, return NULL_TREE.  */
 
-tree
-fold_ternary_loc (location_t loc, enum tree_code code, tree type,
+static tree
+fold_ternary_loc_1 (location_t loc, enum tree_code code, tree type,
 	      tree op0, tree op1, tree op2)
 {
   tree tem;
@@ -13467,6 +13552,23 @@ fold_ternary_loc (location_t loc, enum t
     } /* switch (code) */
 }
 
+/* Fold a ternary expression of code CODE and type TYPE with operands
+   OP0, OP1, and OP2.  Return the folded expression if folding is
+   successful.  Otherwise, return NULL_TREE.
+   This is a wrapper around fold_ternary_loc_1 function (which does the
+   actual folding). Set the EXPR_FOLDED flag of the folded expression
+   if folding is successful.  */
+
+tree
+fold_ternary_loc (location_t loc, enum tree_code code, tree type,
+                  tree op0, tree op1, tree op2)
+{
+  tree tem = fold_ternary_loc_1 (loc, code, type, op0, op1, op2);
+  if (tem)
+    set_expr_folded_flag (tem, 1);
+  return tem;
+}
+
 /* Perform constant folding and related simplification of EXPR.
    The related simplifications include x*1 => x, x*0 => 0, etc.,
    and application of the associative law.
Index: gcc/testsuite/gcc.dg/wself-assign-3.c
===================================================================
--- gcc/testsuite/gcc.dg/wself-assign-3.c	(revision 0)
+++ gcc/testsuite/gcc.dg/wself-assign-3.c	(revision 0)
@@ -0,0 +1,9 @@
+/* Test whether -Wself-assign is enabled by -Wall. */
+/* { dg-do compile } */
+/* { dg-options "-Wall" } */
+
+int bar() {
+  int i = 0;
+  i = i;             /* { dg-warning "assigned to itself" } */
+  return i;
+}
Index: gcc/testsuite/gcc.dg/uninit-D-O0.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-D-O0.c	(revision 162385)
+++ gcc/testsuite/gcc.dg/uninit-D-O0.c	(working copy)
@@ -1,9 +1,9 @@
-/* Test we do not warn about initializing variable with self. */
+/* Test we do warn about initializing variable with self. */
 /* { dg-do compile } */
 /* { dg-options "-Wuninitialized" } */
 
 int f()
 {
-  int i = i;
+  int i = i; /* { dg-warning "i" "uninitialized variable warning" }  */
   return i;
 }
Index: gcc/testsuite/gcc.dg/uninit-D.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-D.c	(revision 162385)
+++ gcc/testsuite/gcc.dg/uninit-D.c	(working copy)
@@ -1,9 +1,9 @@
-/* Test we do not warn about initializing variable with self. */
+/* Test we do warn about initializing variable with self. */
 /* { dg-do compile } */
 /* { dg-options "-O -Wuninitialized" } */
 
 int f()
 {
-  int i = i;
+  int i = i; /* { dg-warning "i" "uninitialized variable warning" }  */
   return i;
 }
Index: gcc/testsuite/gcc.dg/plugin/selfassign.c
===================================================================
--- gcc/testsuite/gcc.dg/plugin/selfassign.c	(revision 162385)
+++ gcc/testsuite/gcc.dg/plugin/selfassign.c	(working copy)
@@ -194,7 +194,7 @@ compare_and_warn (gimple stmt, tree lhs,
 /* Check and warn if STMT is a self-assign statement.  */
 
 static void
-warn_self_assign (gimple stmt)
+check_self_assign (gimple stmt)
 {
   tree rhs, lhs;
 
@@ -247,7 +247,7 @@ execute_warn_self_assign (void)
   FOR_EACH_BB (bb)
     {
       for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
-        warn_self_assign (gsi_stmt (gsi));
+        check_self_assign (gsi_stmt (gsi));
     }
 
   return 0;
Index: gcc/testsuite/gcc.dg/wself-assign-1.c
===================================================================
--- gcc/testsuite/gcc.dg/wself-assign-1.c	(revision 0)
+++ gcc/testsuite/gcc.dg/wself-assign-1.c	(revision 0)
@@ -0,0 +1,27 @@
+/* Test the self-assignemnt detection and warning.  */
+/* { dg-do compile } */
+/* { dg-options "-Wself-assign" } */
+
+struct Bar {
+  int b_;
+  int c_;
+};
+
+int g;
+
+int main()
+{
+  struct Bar *bar;
+  int x = x; /* { dg-warning "assigned to itself" } */
+  static int y;
+  struct Bar b_array[5];
+
+  b_array[x+g].b_ = b_array[x+g].b_; /* { dg-warning "assigned to itself" } */
+  g = g; /* { dg-warning "assigned to itself" } */
+  y = y; /* { dg-warning "assigned to itself" } */
+  bar->b_ = bar->b_; /* { dg-warning "assigned to itself" } */
+  x += 0; /* should not warn */
+  y -= 0; /* should not warn */
+  x /= x; /* should not warn */
+  y *= y; /* should not warn */
+}
Index: gcc/testsuite/gcc.dg/uninit-E.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-E.c	(revision 162385)
+++ gcc/testsuite/gcc.dg/uninit-E.c	(working copy)
@@ -1,4 +1,5 @@
-/* Test we do warn about initializing variable with self when -Winit-self is supplied. */
+/* Test we do warn about initializing variable with self and that -Winit-self
+   is preserved for backward compatibility. */
 /* { dg-do compile } */
 /* { dg-options "-O -Wuninitialized -Winit-self" } */
 
Index: gcc/testsuite/gcc.dg/wself-assign-2.c
===================================================================
--- gcc/testsuite/gcc.dg/wself-assign-2.c	(revision 0)
+++ gcc/testsuite/gcc.dg/wself-assign-2.c	(revision 0)
@@ -0,0 +1,24 @@
+/* Test how self-assignment detection handles constant-folding happening */
+/*   when parsing the RHS or the initializer.  */
+/* { dg-do compile } */
+/* { dg-options "-Wself-assign" } */
+
+struct Bar {
+  int b_;
+  float c_;
+};
+
+int g;
+
+int main()
+{
+  struct Bar *bar;
+  int x = x - 0; /* should not warn */
+  static int y;
+  struct Bar b_array[5];
+
+  b_array[x+g].b_ = b_array[x+g].b_ * 1; /* should no warn */
+  g = g + 0; /* should not warn */
+  y = y / 1; /* should not warn */
+  bar->b_ = bar->b_ - 0; /* should not warn  */
+}
Index: gcc/testsuite/gcc.dg/uninit-E-O0.c
===================================================================
--- gcc/testsuite/gcc.dg/uninit-E-O0.c	(revision 162385)
+++ gcc/testsuite/gcc.dg/uninit-E-O0.c	(working copy)
@@ -1,4 +1,5 @@
-/* Test we do warn about initializing variable with self when -Winit-self is supplied. */
+/* Test we do warn about initializing variable with self and that -Winit-self
+   is preserved for backward compatibility. */
 /* { dg-do compile } */
 /* { dg-options "-Wuninitialized -Winit-self" } */
 
Index: gcc/testsuite/g++.dg/plugin/selfassign.c
===================================================================
--- gcc/testsuite/g++.dg/plugin/selfassign.c	(revision 162385)
+++ gcc/testsuite/g++.dg/plugin/selfassign.c	(working copy)
@@ -194,7 +194,7 @@ compare_and_warn (gimple stmt, tree lhs,
 /* Check and warn if STMT is a self-assign statement.  */
 
 static void
-warn_self_assign (gimple stmt)
+check_self_assign (gimple stmt)
 {
   tree rhs, lhs;
 
@@ -247,7 +247,7 @@ execute_warn_self_assign (void)
   FOR_EACH_BB (bb)
     {
       for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
-        warn_self_assign (gsi_stmt (gsi));
+        check_self_assign (gsi_stmt (gsi));
     }
 
   return 0;
Index: gcc/testsuite/g++.dg/warn/Wself-assign-1.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wself-assign-1.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/Wself-assign-1.C	(revision 0)
@@ -0,0 +1,54 @@
+// Test the self-assignemnt detection and warning.
+// { dg-do compile }
+// { dg-options "-Wself-assign" }
+
+class Foo {
+ private:
+  int a_;
+
+ public:
+  Foo() : a_(a_) {} // { dg-warning "assigned to itself" }
+
+  void setA(int a) {
+    a_ = a_; // { dg-warning "assigned to itself" }
+  }
+
+  void operator=(Foo& rhs) {
+    this->a_ = rhs.a_;
+  }
+};
+
+struct Bar {
+  int b_;
+  int c_;
+};
+
+int g = g; // { dg-warning "assigned to itself" }
+Foo foo = foo; // { dg-warning "assigned to itself" }
+
+int func()
+{
+  Bar *bar1, bar2;
+  Foo local_foo;
+  int x = x; // { dg-warning "assigned to itself" }
+  static int y = y; // { dg-warning "assigned to itself" }
+  float *f;
+  Bar bar_array[5];
+  char n;
+  int overflow;
+
+  *f = *f; // { dg-warning "assigned to itself" }
+  bar1->b_ = bar1->b_; // { dg-warning "assigned to itself" }
+  bar2.c_ = bar2.c_; // { dg-warning "assigned to itself" }
+  local_foo = local_foo; // { dg-warning "assigned to itself" }
+  foo = foo; // { dg-warning "assigned to itself" }
+  foo.setA(5);
+  bar_array[3].c_ = bar_array[3].c_; // { dg-warning "assigned to itself" }
+  bar_array[x+g].b_ = bar_array[x+g].b_; // { dg-warning "assigned to itself" }
+  y = x;
+  x = y;
+  x += 0; // should not warn
+  y -= 0; // should not warn
+  x /= x; // should not warn
+  y *= y; // should not warn
+}
Index: gcc/testsuite/g++.dg/warn/Wself-assign-2.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wself-assign-2.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/Wself-assign-2.C	(revision 0)
@@ -0,0 +1,31 @@
+// Test the handling of expressions that depend on template parameters in
+// self-assignemnt detection.
+// { dg-do compile }
+// { dg-options "-Wself-assign" }
+
+template<typename T>
+struct Bar {
+  T x;
+  Bar operator++(int) {
+    Bar tmp = *this;
+    ++x;
+    tmp = tmp; // { dg-warning "assigned to itself" }
+    return tmp;
+  }
+};
+
+template<typename T>
+T DoSomething(T y) {
+  T a[5], *p;
+  Bar<T> b;
+  b.x = b.x; // { dg-warning "assigned to itself" }
+  *p = *p; // { dg-warning "assigned to itself" }
+  a[2] = a[2]; // { dg-warning "assigned to itself" }
+  return *p;
+}
+
+main() {
+  Bar<int> bar;
+  bar++;
+  DoSomething(5);
+}
Index: gcc/testsuite/g++.dg/warn/Wself-assign-3.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wself-assign-3.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/Wself-assign-3.C	(revision 0)
@@ -0,0 +1,35 @@
+// Test how operands_equal_p handles a NULL operand.
+// { dg-do compile }
+// { dg-options "-Wself-assign" }
+
+#include <cstdio>
+
+namespace testing {
+
+class Foo {
+  int f;
+ public:
+  Foo() { printf("Construct Foo\n"); }
+};
+
+class Bar {
+  int b;
+ public:
+  Bar(int x) { printf("Construct Bar\n"); }
+
+  void operator=(const Foo& foo) {
+    printf("Assign Foo to Bar\n");
+  }
+};
+
+}
+
+template <class T>
+void func(T t) {
+  ::testing::Bar(1) = ::testing::Foo(); // used to trigger a segfault
+  ::testing::Foo() = ::testing::Foo(); // { dg-warning "assigned to itself" }
+}
+
+main() {
+  func(2);
+}
Index: gcc/testsuite/g++.dg/warn/Wself-assign-4.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wself-assign-4.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/Wself-assign-4.C	(revision 0)
@@ -0,0 +1,48 @@
+// Test how self-assignment detection handles constant-folding happening
+// when parsing the RHS or the initializer.
+// { dg-do compile }
+// { dg-options "-Wself-assign" }
+
+class Foo {
+ private:
+  int a_;
+
+ public:
+  Foo() : a_(a_+0) {} // should not warn
+
+  void setA(int a) {
+    a_ = a_ + 0; // should not warn
+  }
+
+  void operator=(Foo& rhs) {
+    this->a_ = rhs.a_;
+  }
+};
+
+struct Bar {
+  int b_;
+  float c_;
+};
+
+int g = g * 1; // should not warn
+
+int func()
+{
+  Bar *bar1, bar2;
+  Foo foo;
+  int x = x - 0;        // should not warn
+  static int y = y / 1; // should not warn
+  float *f;
+  Bar bar_array[5];
+
+  *f = *f / 1;             // should not warn
+  bar1->b_ = bar1->b_ * 1; // should not warn
+  bar2.c_ = bar2.c_ - 0;   // should not warn
+  foo.setA(5);
+  bar_array[3].c_ = bar_array[3].c_ * 1;     // should not warn
+  bar_array[x+g].b_ = bar_array[x+g].b_ / 1; // should not warn
+  x += 0;
+  y -= 0;
+  foo = foo;           // { dg-warning "assigned to itself" }
+  foo.operator=(foo);  // should not warn
+}
Index: gcc/testsuite/g++.dg/warn/Wself-assign-5.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wself-assign-5.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/Wself-assign-5.C	(revision 0)
@@ -0,0 +1,21 @@
+// Test whether -Wself-assign is enabled by -Wall.
+// { dg-do compile }
+// { dg-options "-Wall" }
+
+class Foo {
+  int a, b;
+ public:
+  Foo() : a(a) { }   // { dg-warning "assigned to itself" }
+  int foo() { return b; }
+};
+
+int c = c;           // { dg-warning "assigned to itself" }
+
+Foo foo;
+
+int bar() {
+  static int d = d;  // { dg-warning "assigned to itself" }
+  int i = 0;
+  i = i;             // { dg-warning "assigned to itself" }
+  return d;
+}
Index: gcc/testsuite/g++.dg/warn/Wself-assign-6.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wself-assign-6.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/Wself-assign-6.C	(revision 0)
@@ -0,0 +1,10 @@
+// Test whether the expr_folded flag used by -Wself-assign is properly reset
+// after each check.
+// { dg-do compile }
+// { dg-options "-Wself-assign" }
+
+void bar() {
+  int x = 0;
+  x = x + 0;
+  x = x;             // { dg-warning "assigned to itself" }
+}
Index: gcc/cp/init.c
===================================================================
--- gcc/cp/init.c	(revision 162385)
+++ gcc/cp/init.c	(working copy)
@@ -529,8 +529,14 @@ perform_member_init (tree member, tree i
 						tf_warning_or_error);
 
       if (init)
-	finish_expr_stmt (cp_build_modify_expr (decl, INIT_EXPR, init,
-						tf_warning_or_error));
+        {
+          finish_expr_stmt (cp_build_modify_expr (decl, INIT_EXPR, init,
+                                                  tf_warning_or_error));
+          /* Check for and warn about self-initialization if -Wself-assign is
+             enabled.  */
+          if (warn_self_assign)
+            check_for_self_assign (input_location, decl, init);
+        }
     }
 
   if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (type))
Index: gcc/cp/parser.c
===================================================================
--- gcc/cp/parser.c	(revision 162385)
+++ gcc/cp/parser.c	(working copy)
@@ -6918,6 +6918,12 @@ cp_parser_assignment_expression (cp_pars
 	      if (cp_parser_non_integral_constant_expression (parser,
 							      NIC_ASSIGNMENT))
 		return error_mark_node;
+
+              /* Check for and warn about self-assignment if -Wself-assign is
+                 enabled and the assignment operator is "=".  */
+              if (warn_self_assign && assignment_operator == NOP_EXPR)
+                check_for_self_assign (input_location, expr, rhs);
+
 	      /* Build the assignment expression.  */
 	      expr = build_x_modify_expr (expr,
 					  assignment_operator,
@@ -14090,6 +14096,10 @@ cp_parser_init_declarator (cp_parser* pa
 			 `explicit' constructor cannot be used.  */
 		      ((is_direct_init || !is_initialized)
 		       ? LOOKUP_NORMAL : LOOKUP_IMPLICIT));
+      /* Check for and warn about self-initialization if -Wself-assign is
+         enabled.  */
+      if (warn_self_assign && initializer)
+        check_for_self_assign (input_location, decl, initializer);
     }
   else if ((cxx_dialect != cxx98) && friend_p
 	   && decl && TREE_CODE (decl) == FUNCTION_DECL)
Index: gcc/c-parser.c
===================================================================
--- gcc/c-parser.c	(revision 162385)
+++ gcc/c-parser.c	(working copy)
@@ -1297,6 +1297,11 @@ c_parser_declaration_or_fndef (c_parser 
 		  maybe_warn_string_init (TREE_TYPE (d), init);
 		  finish_decl (d, init_loc, init.value,
 		      	       init.original_type, asm_name);
+                  /* Check for and warn about self-initialization if
+                     -Wself-assign is enabled. Don't warn if there is
+                     already an error for the initializer.  */
+                  if (warn_self_assign && DECL_INITIAL (d) != error_mark_node)
+                    check_for_self_assign (here, d, init.value);
 		}
 	    }
 	  else
@@ -4764,7 +4769,13 @@ c_parser_expr_no_commas (c_parser *parse
 				 code, exp_location, rhs.value,
 				 rhs.original_type);
   if (code == NOP_EXPR)
-    ret.original_code = MODIFY_EXPR;
+    {
+      ret.original_code = MODIFY_EXPR;
+      /* Check for and warn about self-assignment if -Wself-assign is
+         enabled.  */
+      if (warn_self_assign)
+        check_for_self_assign (op_location, lhs.value, rhs.value);
+    }
   else
     {
       TREE_NO_WARNING (ret.value) = 1;

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

[Jack Howarth]

On Wed, Jul 21, 2010 at 05:45:16PM -0700, Le-Chun Wu wrote:
> 
> 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,
    Would it make sense to extend the -Wself-assign flag to
other languages in gcc (like fortran for example)?
                    Jack

> 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.

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

[Jason Merrill]

On 07/21/2010 08:45 PM, Le-Chun Wu wrote:
> 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?

That's certainly better, but it still makes me a bit uncomfortable to 
have a flag that's just used for transient marking.  Is it possible to 
avoid the need for this flag in the C front end, since it delays folding 
until c_fully_fold?

Jason

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

[Richard Guenther]

On Tue, 27 Jul 2010, Jason Merrill wrote:

> On 07/21/2010 08:45 PM, Le-Chun Wu wrote:
> > 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?
> 
> That's certainly better, but it still makes me a bit uncomfortable to have a
> flag that's just used for transient marking.  Is it possible to avoid the need
> for this flag in the C front end, since it delays folding until c_fully_fold?

Btw, decls and constants are shared so it's certainly bogus to
set this flag there.  The proper thing to do is wrap the trees in
sth else (like NON_LVALUE_EXPR).

Richard.