re PR c/16202 (The -Wsequence-point warning misses many important instances)

2009-04-21  Manuel López-Ibáñez  <manu@gcc.gnu.org>

	PR 16202
	* c-typeck.c (lvalue_p): Move declaration ...
	* c-common.h (lvalue_p): ... to here.
	* c-common.c (candidate_equal_p): New.
	(add_tlist): Use it.
	(merge_tlist): Use it.
	(warn_for_collisions_1): Likewise.
	(warning_candidate_p): Accept more candidates.
	(verify_tree): A warning candidate can be an expression. Use
	candidate_equal_p.
cp/
	* tree.c (lvalue_p_1): Use const_tree.
	Use CONST_CAST_TREE to avoid warning.
	(lvalue_p): Returns bool, receives const_tree.
testsuite/
	* gcc.dg/sequence-pt-1.c: Remove XFAILs.
	* gcc.dg/sequence-pt-2.c: New.
	* gcc.dg/sequence-pt-3.c: New.
	* g++.dg/warn/sequence-pt-1.C: Remove XFAILs.
	* g++.dg/warn/sequence-pt-2.c: New.
	* g++.dg/warn/sequence-pt-3.c: New.

From-SVN: r146472

14 files changed, 185 insertions, 25 deletions

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index a73e3c9..a62edc8 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,16 @@
+2009-04-21  Manuel López-Ibáñez  <manu@gcc.gnu.org>
+
+	PR 16202
+	* c-typeck.c (lvalue_p): Move declaration ...
+	* c-common.h (lvalue_p): ... to here.
+	* c-common.c (candidate_equal_p): New.
+	(add_tlist): Use it.
+	(merge_tlist): Use it.
+	(warn_for_collisions_1): Likewise.
+	(warning_candidate_p): Accept more candidates.
+	(verify_tree): A warning candidate can be an expression. Use
+	candidate_equal_p.
+
 2009-04-21  Ben Elliston  <bje@au.ibm.com>
 
 	PR target/5267
diff --git a/gcc/c-common.c b/gcc/c-common.c
index 7e62258..eab4582 100644
--- a/gcc/c-common.c
+++ b/gcc/c-common.c
@@ -2381,6 +2381,7 @@ static void add_tlist (struct tlist **, struct tlist *, tree, int);
 static void merge_tlist (struct tlist **, struct tlist *, int);
 static void verify_tree (tree, struct tlist **, struct tlist **, tree);
 static int warning_candidate_p (tree);
+static bool candidate_equal_p (const_tree, const_tree);
 static void warn_for_collisions (struct tlist *);
 static void warn_for_collisions_1 (tree, tree, struct tlist *, int);
 static struct tlist *new_tlist (struct tlist *, tree, tree);
@@ -2408,7 +2409,7 @@ add_tlist (struct tlist **to, struct tlist *add, tree exclude_writer, int copy)
       struct tlist *next = add->next;
       if (!copy)
 	add->next = *to;
-      if (!exclude_writer || add->writer != exclude_writer)
+      if (!exclude_writer || !candidate_equal_p (add->writer, exclude_writer))
 	*to = copy ? new_tlist (*to, add->expr, add->writer) : add;
       add = next;
     }
@@ -2435,7 +2436,7 @@ merge_tlist (struct tlist **to, struct tlist *add, int copy)
       struct tlist *next = add->next;
 
       for (tmp2 = *to; tmp2; tmp2 = tmp2->next)
-	if (tmp2->expr == add->expr)
+	if (candidate_equal_p (tmp2->expr, add->expr))
 	  {
 	    found = 1;
 	    if (!tmp2->writer)
@@ -2463,15 +2464,14 @@ warn_for_collisions_1 (tree written, tree writer, struct tlist *list,
 
   /* Avoid duplicate warnings.  */
   for (tmp = warned_ids; tmp; tmp = tmp->next)
-    if (tmp->expr == written)
+    if (candidate_equal_p (tmp->expr, written))
       return;
 
   while (list)
     {
-      if (list->expr == written
-	  && list->writer != writer
-	  && (!only_writes || list->writer)
-	  && DECL_NAME (list->expr))
+      if (candidate_equal_p (list->expr, written)
+	  && !candidate_equal_p (list->writer, writer)
+	  && (!only_writes || list->writer))
 	{
 	  warned_ids = new_tlist (warned_ids, written, NULL_TREE);
 	  warning_at (EXPR_HAS_LOCATION (writer)
@@ -2503,7 +2503,17 @@ warn_for_collisions (struct tlist *list)
 static int
 warning_candidate_p (tree x)
 {
-  return TREE_CODE (x) == VAR_DECL || TREE_CODE (x) == PARM_DECL;
+  /* !VOID_TYPE_P (TREE_TYPE (x)) is workaround for cp/tree.c
+     (lvalue_p) crash on TRY/CATCH. */
+  return !(DECL_P (x) && DECL_ARTIFICIAL (x))
+    && TREE_TYPE (x) && !VOID_TYPE_P (TREE_TYPE (x)) && lvalue_p (x);
+}
+
+/* Return nonzero if X and Y appear to be the same candidate (or NULL) */
+static bool
+candidate_equal_p (const_tree x, const_tree y)
+{
+  return (x == y) || (x && y && operand_equal_p (x, y, 0));
 }
 
 /* Walk the tree X, and record accesses to variables.  If X is written by the
@@ -2549,10 +2559,7 @@ verify_tree (tree x, struct tlist **pbefore_sp, struct tlist **pno_sp,
   cl = TREE_CODE_CLASS (code);
 
   if (warning_candidate_p (x))
-    {
-      *pno_sp = new_tlist (*pno_sp, x, writer);
-      return;
-    }
+    *pno_sp = new_tlist (*pno_sp, x, writer);
 
   switch (code)
     {
@@ -2665,7 +2672,7 @@ verify_tree (tree x, struct tlist **pbefore_sp, struct tlist **pno_sp,
       {
 	struct tlist_cache *t;
 	for (t = save_expr_cache; t; t = t->next)
-	  if (t->expr == x)
+	  if (candidate_equal_p (t->expr, x))
 	    break;
 
 	if (!t)
diff --git a/gcc/c-common.h b/gcc/c-common.h
index dcadbdd..a46da6b 100644
--- a/gcc/c-common.h
+++ b/gcc/c-common.h
@@ -932,6 +932,7 @@ extern tree finish_label_address_expr (tree, location_t);
    different implementations.  Used in c-common.c.  */
 extern tree lookup_label (tree);
 extern tree lookup_name (tree);
+extern bool lvalue_p (const_tree);
 
 extern bool vector_targets_convertible_p (const_tree t1, const_tree t2);
 extern bool vector_types_convertible_p (const_tree t1, const_tree t2, bool emit_lax_note);
diff --git a/gcc/c-typeck.c b/gcc/c-typeck.c
index 7fc55e2..f5cfc64 100644
--- a/gcc/c-typeck.c
+++ b/gcc/c-typeck.c
@@ -110,7 +110,6 @@ static tree find_init_member (tree);
 static void readonly_error (tree, enum lvalue_use);
 static void readonly_warning (tree, enum lvalue_use);
 static int lvalue_or_else (const_tree, enum lvalue_use);
-static int lvalue_p (const_tree);
 static void record_maybe_used_decl (tree);
 static int comptypes_internal (const_tree, const_tree);
 
@@ -3503,7 +3502,7 @@ build_unary_op (location_t location,
    Lvalues can be assigned, unless their type has TYPE_READONLY.
    Lvalues can have their address taken, unless they have C_DECL_REGISTER.  */
 
-static int
+bool
 lvalue_p (const_tree ref)
 {
   const enum tree_code code = TREE_CODE (ref);
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index df0a91a..c3374e3 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,10 @@
+2009-04-21  Manuel López-Ibáñez  <manu@gcc.gnu.org>
+
+	PR 16202
+	* tree.c (lvalue_p_1): Use const_tree.
+	Use CONST_CAST_TREE to avoid warning.
+	(lvalue_p): Returns bool, receives const_tree.
+
 2009-04-21  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>
 
 	PR c++/13358
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index c80037d..d66e0de 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -5014,7 +5014,6 @@ extern tree convert_member_func_to_ptr		(tree, tree);
 extern tree convert_ptrmem			(tree, tree, bool, bool);
 extern int lvalue_or_else			(tree, enum lvalue_use,
                                                  tsubst_flags_t);
-extern int lvalue_p				(tree);
 
 /* in typeck2.c */
 extern void require_complete_eh_spec_types	(tree, tree);
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index b4b977e..c95c11c 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -44,7 +44,7 @@ static tree build_cplus_array_type_1 (tree, tree);
 static int list_hash_eq (const void *, const void *);
 static hashval_t list_hash_pieces (tree, tree, tree);
 static hashval_t list_hash (const void *);
-static cp_lvalue_kind lvalue_p_1 (tree, int);
+static cp_lvalue_kind lvalue_p_1 (const_tree, int);
 static tree build_target_expr (tree, tree);
 static tree count_trees_r (tree *, int *, void *);
 static tree verify_stmt_tree_r (tree *, int *, void *);
@@ -59,7 +59,7 @@ static tree handle_init_priority_attribute (tree *, tree, tree, int, bool *);
    nonzero, rvalues of class type are considered lvalues.  */
 
 static cp_lvalue_kind
-lvalue_p_1 (tree ref,
+lvalue_p_1 (const_tree ref,
 	    int treat_class_rvalues_as_lvalues)
 {
   cp_lvalue_kind op1_lvalue_kind = clk_none;
@@ -207,7 +207,9 @@ lvalue_p_1 (tree ref,
     case BASELINK:
       /* We now represent a reference to a single static member function
 	 with a BASELINK.  */
-      return lvalue_p_1 (BASELINK_FUNCTIONS (ref),
+      /* This CONST_CAST is okay because BASELINK_FUNCTIONS returns
+	 its argument unmodified and we assign it to a const_tree.  */
+      return lvalue_p_1 (BASELINK_FUNCTIONS (CONST_CAST_TREE (ref)),
 			 treat_class_rvalues_as_lvalues);
 
     case NON_DEPENDENT_EXPR:
@@ -251,8 +253,8 @@ real_lvalue_p (tree ref)
 /* This differs from real_lvalue_p in that class rvalues are
    considered lvalues.  */
 
-int
-lvalue_p (tree ref)
+bool
+lvalue_p (const_tree ref)
 {
   return
     (lvalue_p_1 (ref, /*class rvalue ok*/ 1) != clk_none);
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index c9f2a5e..8b6d742 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,13 @@
+2009-04-21  Manuel López-Ibáñez  <manu@gcc.gnu.org>
+
+	PR 16202
+	* gcc.dg/sequence-pt-1.c: Remove XFAILs.
+	* gcc.dg/sequence-pt-2.c: New.
+	* gcc.dg/sequence-pt-3.c: New.
+	* g++.dg/warn/sequence-pt-1.C: Remove XFAILs.
+	* g++.dg/warn/sequence-pt-2.c: New.
+	* g++.dg/warn/sequence-pt-3.c: New.
+
 2009-04-21  Jakub Jelinek  <jakub@redhat.com>
 
 	* gcc.dg/torture/pr39678.c (struct X): Make c field signed char.
diff --git a/gcc/testsuite/g++.dg/warn/sequence-pt-1.C b/gcc/testsuite/g++.dg/warn/sequence-pt-1.C
index 3b92142..05eee82 100644
--- a/gcc/testsuite/g++.dg/warn/sequence-pt-1.C
+++ b/gcc/testsuite/g++.dg/warn/sequence-pt-1.C
@@ -30,10 +30,10 @@ foo (int a, int b, int n, int p, int *ptr, struct s *sptr,
   ap[n] = bp[n++]; /* { dg-warning "undefined" "sequence point warning" } */
   ap[--n] = bp[n]; /* { dg-warning "undefined" "sequence point warning" } */
   ap[++n] = bp[--n]; /* { dg-warning "undefined" "sequence point warning" } */
-  cp[n][n] = cp[n][n]++; /* { dg-warning "undefined" "sequence point warning" { xfail *-*-* } } */
+  cp[n][n] = cp[n][n]++; /* { dg-warning "undefined" "sequence point warning" } */
   cp[n][p] = cp[n][n++]; /* { dg-warning "undefined" "sequence point warning" } */
   *ptr++ = (size_t)ptr++; /* { dg-warning "undefined" "sequence point warning" } */
-  sptr->a = sptr->a++; /* { dg-warning "undefined" "sequence point warning" { xfail *-*-* } } */
+  sptr->a = sptr->a++; /* { dg-warning "undefined" "sequence point warning" } */
   sptr->a = (size_t)(sptr++); /* { dg-warning "undefined" "sequence point warning" } */
   *ptr++ = fn (*ptr); /* { dg-warning "undefined" "sequence point warning" } */
   a = b = a++; /* { dg-warning "undefined" "sequence point warning" } */
diff --git a/gcc/testsuite/g++.dg/warn/sequence-pt-2.C b/gcc/testsuite/g++.dg/warn/sequence-pt-2.C
new file mode 100644
index 0000000..9a4b618
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/sequence-pt-2.C
@@ -0,0 +1,46 @@
+/* More sequence point warning tests  */
+/* { dg-do compile } */
+/* { dg-options "-Wsequence-point" } */
+
+struct s { struct s *nxt; int v; } q;
+    
+int x[10];
+    
+int foo(int *p)
+{
+  int i = 0;
+
+  /* Test general-lvalue sequence point warnings  */
+  (*p) = (*p)++; /* { dg-warning "undefined" "sequence point warning" } */
+  p[3] = p[3]++; /* { dg-warning "undefined" "sequence point warning" } */
+  p[i] = p[i]++; /* { dg-warning "undefined" "sequence point warning" } */
+  x[3] = x[3]++; /* { dg-warning "undefined" "sequence point warning" } */
+  q.nxt->nxt->v = q.nxt->nxt->v++; /* { dg-warning "undefined" "sequence point warning" } */
+
+  /* test expressions that appear elsewhere in the C grammar */
+
+  { int a = i-i++; (void)a;} /* { dg-warning "undefined" "sequence point warning" } */
+
+  if ((i-i++) != 0) /* { dg-warning "undefined" "sequence point warning" } */
+    return i-i++; /* { dg-warning "undefined" "sequence point warning" } */
+
+  for (i-i++;;)  /* { dg-warning "undefined" "sequence point warning" } */
+    ;
+
+  for (; (i-i++) != 0; )  /* { dg-warning "undefined" "sequence point warning" } */
+    ;
+
+  for (;;i-i++)  /* { dg-warning "undefined" "sequence point warning" } */
+    ;
+
+  while ((i-i++) != 0)  /* { dg-warning "undefined" "sequence point warning" } */
+    ;
+
+  do {} while ((i-i++) != 0);  /* { dg-warning "undefined" "sequence point warning" } */
+
+  switch (i-i++) {  /* { dg-warning "undefined" "sequence point warning" } */
+  case 0: return 1;
+  }
+
+  return 0;
+}
diff --git a/gcc/testsuite/g++.dg/warn/sequence-pt-3.C b/gcc/testsuite/g++.dg/warn/sequence-pt-3.C
new file mode 100644
index 0000000..58971ca
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/sequence-pt-3.C
@@ -0,0 +1,15 @@
+/* More sequence point warning tests  */
+/* { dg-do compile } */
+/* { dg-options "-Wsequence-point" } */
+
+void bar(int i, int j)
+{
+  return;
+}
+
+void foo (int i)
+{
+   int a = i-i++; (void)a; /* { dg-warning "undefined" "sequence point warning" } */
+
+   bar (i--, i++); /* { dg-warning "undefined" "sequence point warning" } */
+}
diff --git a/gcc/testsuite/gcc.dg/sequence-pt-1.c b/gcc/testsuite/gcc.dg/sequence-pt-1.c
index 3b92142..05eee82 100644
--- a/gcc/testsuite/gcc.dg/sequence-pt-1.c
+++ b/gcc/testsuite/gcc.dg/sequence-pt-1.c
@@ -30,10 +30,10 @@ foo (int a, int b, int n, int p, int *ptr, struct s *sptr,
   ap[n] = bp[n++]; /* { dg-warning "undefined" "sequence point warning" } */
   ap[--n] = bp[n]; /* { dg-warning "undefined" "sequence point warning" } */
   ap[++n] = bp[--n]; /* { dg-warning "undefined" "sequence point warning" } */
-  cp[n][n] = cp[n][n]++; /* { dg-warning "undefined" "sequence point warning" { xfail *-*-* } } */
+  cp[n][n] = cp[n][n]++; /* { dg-warning "undefined" "sequence point warning" } */
   cp[n][p] = cp[n][n++]; /* { dg-warning "undefined" "sequence point warning" } */
   *ptr++ = (size_t)ptr++; /* { dg-warning "undefined" "sequence point warning" } */
-  sptr->a = sptr->a++; /* { dg-warning "undefined" "sequence point warning" { xfail *-*-* } } */
+  sptr->a = sptr->a++; /* { dg-warning "undefined" "sequence point warning" } */
   sptr->a = (size_t)(sptr++); /* { dg-warning "undefined" "sequence point warning" } */
   *ptr++ = fn (*ptr); /* { dg-warning "undefined" "sequence point warning" } */
   a = b = a++; /* { dg-warning "undefined" "sequence point warning" } */
diff --git a/gcc/testsuite/gcc.dg/sequence-pt-2.c b/gcc/testsuite/gcc.dg/sequence-pt-2.c
new file mode 100644
index 0000000..9a4b618
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/sequence-pt-2.c
@@ -0,0 +1,46 @@
+/* More sequence point warning tests  */
+/* { dg-do compile } */
+/* { dg-options "-Wsequence-point" } */
+
+struct s { struct s *nxt; int v; } q;
+    
+int x[10];
+    
+int foo(int *p)
+{
+  int i = 0;
+
+  /* Test general-lvalue sequence point warnings  */
+  (*p) = (*p)++; /* { dg-warning "undefined" "sequence point warning" } */
+  p[3] = p[3]++; /* { dg-warning "undefined" "sequence point warning" } */
+  p[i] = p[i]++; /* { dg-warning "undefined" "sequence point warning" } */
+  x[3] = x[3]++; /* { dg-warning "undefined" "sequence point warning" } */
+  q.nxt->nxt->v = q.nxt->nxt->v++; /* { dg-warning "undefined" "sequence point warning" } */
+
+  /* test expressions that appear elsewhere in the C grammar */
+
+  { int a = i-i++; (void)a;} /* { dg-warning "undefined" "sequence point warning" } */
+
+  if ((i-i++) != 0) /* { dg-warning "undefined" "sequence point warning" } */
+    return i-i++; /* { dg-warning "undefined" "sequence point warning" } */
+
+  for (i-i++;;)  /* { dg-warning "undefined" "sequence point warning" } */
+    ;
+
+  for (; (i-i++) != 0; )  /* { dg-warning "undefined" "sequence point warning" } */
+    ;
+
+  for (;;i-i++)  /* { dg-warning "undefined" "sequence point warning" } */
+    ;
+
+  while ((i-i++) != 0)  /* { dg-warning "undefined" "sequence point warning" } */
+    ;
+
+  do {} while ((i-i++) != 0);  /* { dg-warning "undefined" "sequence point warning" } */
+
+  switch (i-i++) {  /* { dg-warning "undefined" "sequence point warning" } */
+  case 0: return 1;
+  }
+
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/sequence-pt-3.c b/gcc/testsuite/gcc.dg/sequence-pt-3.c
new file mode 100644
index 0000000..58971ca
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/sequence-pt-3.c
@@ -0,0 +1,15 @@
+/* More sequence point warning tests  */
+/* { dg-do compile } */
+/* { dg-options "-Wsequence-point" } */
+
+void bar(int i, int j)
+{
+  return;
+}
+
+void foo (int i)
+{
+   int a = i-i++; (void)a; /* { dg-warning "undefined" "sequence point warning" } */
+
+   bar (i--, i++); /* { dg-warning "undefined" "sequence point warning" } */
+}