Fix maybe_report_data_members_replaced_by_anon_dm

The function maybe_report_data_members_replaced_by_anon_dm has a bug
and some minor issues. This commit tidies these up.

The function came to my attention when a broken equality test
triggered an infinite loop.

The issues were:

- dms_replaced_by_same_anon_dm declared outside loop, gets reused
- self-comparison of first decl, potential infinite loop
- anonymous_data_member, assigned but not used
- two iterators i, j used, when one would suffice

The first issue results in incorrect diff reports if data members
in a structure are replaced by more than one anonymous data member.
This commit adds additional test cases for this, following the pattern
used for the existing PR25661 ones.

The second issue only affects behaviour if equality is defined
inconsistently with object identity.

	* src/abg-reporter-priv.cc
	(maybe_report_data_members_replaced_by_anon_dm): Move
	declarations of anonymous_data_member and
	dms_replaced_by_same_anon_dm into inner loop. Use
	anonymous_data_member for testing and reporting, allowing
	iterators i and j to be replaced by just iterator i. Push
	first decl onto dms_replaced_by_same_anon_dm unconditionally
	and move control flow logic into loop condition.
	* tests/data/Makefile.am: Add new test cases.
	* tests/data/test-diff-filter/test-PR25661-7-report-1.txt: New
	test case file.
	* tests/data/test-diff-filter/test-PR25661-7-report-2.txt:
	Likewise.
	* tests/data/test-diff-filter/test-PR25661-7-report-3.txt:
	Likewise.
	* tests/data/test-diff-filter/test-PR25661-7-report-4.txt:
	Likewise.
	* tests/data/test-diff-filter/test-PR25661-7-v0.c: Likewise.
	* tests/data/test-diff-filter/test-PR25661-7-v0.o: Likewise.
	* tests/data/test-diff-filter/test-PR25661-7-v1.c: Likewise.
	* tests/data/test-diff-filter/test-PR25661-7-v1.o: Likewise.
	* tests/test-diff-filter.cc: Call new test cases.

Signed-off-by: Giuliano Procida <gprocida@google.com>

src/abg-reporter-priv.cc

@@ -1437,30 +1437,25 @@ maybe_report_data_members_replaced_by_anon_dm(const class_or_union_diff &d,
       // Let's detect all the data members that are replaced by
       // members of the same anonymous data member and report them
       // in one go.
-      changed_var_sptrs_type::const_iterator i, j;
-      i = d.ordered_data_members_replaced_by_adms().begin();
-      // This contains the data members replaced by the same
-      // anonymous data member.
-      vector<var_decl_sptr> dms_replaced_by_same_anon_dm;
-      // This contains the anonymous data member that replaced the
-      // data members in the variable above.
-      var_decl_sptr anonymous_data_member;
-
-      while (i != d.ordered_data_members_replaced_by_adms().end())
+      for (changed_var_sptrs_type::const_iterator i =
+	     d.ordered_data_members_replaced_by_adms().begin();
+	   i != d.ordered_data_members_replaced_by_adms().end();)
 	{
-	  anonymous_data_member = i->second;
+	  // This contains the data members replaced by the same
+	  // anonymous data member.
+	  vector<var_decl_sptr> dms_replaced_by_same_anon_dm;
+	  dms_replaced_by_same_anon_dm.push_back(i->first);
+	  // This contains the anonymous data member that replaced the
+	  // data members in the variable above.
+	  var_decl_sptr anonymous_data_member = i->second;
 	  // Let's look forward to see if the subsequent data
 	  // members were replaced by members of
 	  // anonymous_data_member.
-	  for (j = i;
-	       j != d.ordered_data_members_replaced_by_adms().end();
-	       ++j)
-	    {
-	      if (*i->second == *j->second)
-		dms_replaced_by_same_anon_dm.push_back(j->first);
-	      else
-		break;
-	    }
+	  for (++i;
+	       i != d.ordered_data_members_replaced_by_adms().end()
+		 && *i->second == *anonymous_data_member;
+	       ++i)
+	    dms_replaced_by_same_anon_dm.push_back(i->first);
 
 	  bool several_data_members_replaced =
 	    dms_replaced_by_same_anon_dm.size() > 1;
@@ -1490,10 +1485,8 @@ maybe_report_data_members_replaced_by_anon_dm(const class_or_union_diff &d,
 	  out << "replaced by anonymous data member:\n"
 	      << indent + "  "
 	      << "'"
-	      << i->second->get_pretty_representation()
+	      << anonymous_data_member->get_pretty_representation()
 	      << "'\n";
-
-	  i = j;
 	}
     }
 }

tests/data/Makefile.am

@@ -859,6 +859,14 @@ test-diff-filter/test-PR25661-6-report-1.txt \
 test-diff-filter/test-PR25661-6-report-2.txt \
 test-diff-filter/test-PR25661-6-report-3.txt \
 test-diff-filter/test-PR25661-6-report-4.txt \
+test-diff-filter/test-PR25661-7-v0.c \
+test-diff-filter/test-PR25661-7-v1.c \
+test-diff-filter/test-PR25661-7-v0.o \
+test-diff-filter/test-PR25661-7-v1.o \
+test-diff-filter/test-PR25661-7-report-1.txt \
+test-diff-filter/test-PR25661-7-report-2.txt \
+test-diff-filter/test-PR25661-7-report-3.txt \
+test-diff-filter/test-PR25661-7-report-4.txt \
 \
 test-diff-suppr/test0-type-suppr-v0.cc	\
 test-diff-suppr/test0-type-suppr-v1.cc	\

tests/data/test-diff-filter/test-PR25661-7-report-1.txt

@@ -0,0 +1,3 @@
+Functions changes summary: 0 Removed, 0 Changed (1 filtered out), 0 Added function
+Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
+

tests/data/test-diff-filter/test-PR25661-7-report-2.txt

@@ -0,0 +1,5 @@
+Leaf changes summary: 0 artifact changed (1 filtered out)
+Changed leaf types summary: 0 (1 filtered out) leaf type changed
+Removed/Changed/Added functions summary: 0 Removed, 0 Changed, 0 Added function
+Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
+

tests/data/test-diff-filter/test-PR25661-7-report-3.txt

@@ -0,0 +1,14 @@
+Functions changes summary: 0 Removed, 1 Changed, 0 Added function
+Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
+
+1 function with some indirect sub-type change:
+
+  [C] 'function void foo(S*)' at test-PR25661-7-v1.c:24:1 has some indirect sub-type changes:
+    parameter 1 of type 'S*' has sub-type changes:
+      in pointed to type 'struct S' at test-PR25661-7-v1.c:1:1:
+        type size hasn't changed
+        data members 'S::a', 'S::b' were replaced by anonymous data member:
+          'union {int tag1[2]; struct {int a; int b;};}'
+        data members 'S::c', 'S::d' were replaced by anonymous data member:
+          'union {int tag2[2]; struct {int c; int d;};}'
+

tests/data/test-diff-filter/test-PR25661-7-report-4.txt

@@ -0,0 +1,11 @@
+Leaf changes summary: 1 artifact changed
+Changed leaf types summary: 1 leaf type changed
+Removed/Changed/Added functions summary: 0 Removed, 0 Changed, 0 Added function
+Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
+
+'struct S at test-PR25661-7-v0.c:1:1' changed:
+  type size hasn't changed
+  data members 'S::a', 'S::b' were replaced by anonymous data member:
+    'union {int tag1[2]; struct {int a; int b;};}'
+  data members 'S::c', 'S::d' were replaced by anonymous data member:
+    'union {int tag2[2]; struct {int c; int d;};}'

tests/data/test-diff-filter/test-PR25661-7-v0.c

@@ -0,0 +1,12 @@
+struct S
+{
+  int a;
+  int b;
+  int c;
+  int d;
+};
+
+void
+foo(struct S* s __attribute__((unused)))
+{
+}

tests/data/test-diff-filter/test-PR25661-7-v1.c

@@ -0,0 +1,26 @@
+struct S
+{
+  union
+  {
+    int tag1[2];
+    struct
+    {
+      int a;
+      int b;
+    };
+  };
+  union
+  {
+    int tag2[2];
+    struct
+    {
+      int c;
+      int d;
+    };
+  };
+};
+
+void
+foo(struct S* s __attribute__((unused)))
+{
+}

tests/test-diff-filter.cc

@@ -738,6 +738,34 @@ InOutSpec in_out_specs[] =
    "data/test-diff-filter/test-PR25661-6-report-4.txt",
    "output/test-diff-filter/test-PR25661-6-report-4.txt",
   },
+  {
+   "data/test-diff-filter/test-PR25661-7-v0.o",
+   "data/test-diff-filter/test-PR25661-7-v1.o",
+   "--no-default-suppression",
+   "data/test-diff-filter/test-PR25661-7-report-1.txt",
+   "output/test-diff-filter/test-PR25661-7-report-1.txt",
+  },
+  {
+   "data/test-diff-filter/test-PR25661-7-v0.o",
+   "data/test-diff-filter/test-PR25661-7-v1.o",
+   "--no-default-suppression --leaf-changes-only",
+   "data/test-diff-filter/test-PR25661-7-report-2.txt",
+   "output/test-diff-filter/test-PR25661-7-report-2.txt",
+  },
+  {
+   "data/test-diff-filter/test-PR25661-7-v0.o",
+   "data/test-diff-filter/test-PR25661-7-v1.o",
+   "--no-default-suppression --harmless",
+   "data/test-diff-filter/test-PR25661-7-report-3.txt",
+   "output/test-diff-filter/test-PR25661-7-report-3.txt",
+  },
+  {
+   "data/test-diff-filter/test-PR25661-7-v0.o",
+   "data/test-diff-filter/test-PR25661-7-v1.o",
+   "--no-default-suppression --harmless --leaf-changes-only",
+   "data/test-diff-filter/test-PR25661-7-report-4.txt",
+   "output/test-diff-filter/test-PR25661-7-report-4.txt",
+  },
   // This should be the last entry
   {NULL, NULL, NULL, NULL, NULL}
 };