A builtin type name change is not harmless - fix that

* include/abg-comp-filter.h (has_harmless_name_change): New
	function declaration.
	* include/abg-comparison.h
	(diff_category::DECL_NAME_CHANGE_CATEGORY): Renamed this into
	HARMLESS_DECL_NAME_CHANGE_CATEGORY.
	(diff_category::EVERYTHING_CATEGORY): Update.
	* include/abg-fwd.h (is_enum): New function declaration.
	(is_var_decl): Return the shared_ptr<var_decl> rather than a bool.
	(is_data_member): New overload that takes a shared_ptr<decl_base>.
	* src/abg-comp-filter.cc (decl_name_changed): Consider the
	qualified name here.
	(has_harmless_name_change): Define new function declaration.
	(harmless_filter::visit): Use the new has_harmless_name_change
	function.
	* src/abg-comparison.cc (represent)
	(report_name_size_and_alignment_changes, enum_diff::report)
	(typedef_diff::report, is_data_member): Use the new
	filtering::has_harmless_name_change function to simplify logic of
	emitting the name change related diff
	* tools/bidiff.cc (set_diff_context_from_opts): Adjust
	DECL_NAME_CHANGE_CATEGORY -> HARMLESS_DECL_NAME_CHANGE_CATEGORY.
	* src/abg-ir.cc (is_data_member, is_enum): New function definitions.
	(is_var_decl): Return the var_decl_sptr rather than just a bool.
	* tests/data/test-diff-filter/test13-report.txt: Adjust.
	* tests/data/test-diff-filter/test6-report.txt: Adjust.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>

include/abg-comp-filter.h

@@ -39,6 +39,9 @@ namespace comparison
 namespace filtering
 {
 
+bool
+has_harmless_name_change(decl_base_sptr f, decl_base_sptr s);
+
 class filter_base;
 /// Convenience typedef for a shared pointer to filter_base
 typedef shared_ptr<filter_base> filter_base_sptr;

include/abg-comparison.h

@@ -243,9 +243,10 @@ enum diff_category
   /// instance a type and its typedefs.
   COMPATIBLE_TYPE_CHANGE_CATEGORY = 1 << 2,
 
-  /// This means that a diff node in the sub-tree carries a
-  /// declaration name change.
-  DECL_NAME_CHANGE_CATEGORY = 1 << 3,
+  /// This means that a diff node in the sub-tree carries a harmless
+  /// declaration name change.  This is set only for name changes for
+  /// data members and typedefs.
+  HARMLESS_DECL_NAME_CHANGE_CATEGORY = 1 << 3,
 
   /// This means that a diff node in the sub-tree carries an addition
   /// or removal of a non-virtual member function.
@@ -273,7 +274,7 @@ enum diff_category
   NOT_REDUNDANT_CATEGORY
   | ACCESS_CHANGE_CATEGORY
   | COMPATIBLE_TYPE_CHANGE_CATEGORY
-  | DECL_NAME_CHANGE_CATEGORY
+  | HARMLESS_DECL_NAME_CHANGE_CATEGORY
   | NON_VIRT_MEM_FUN_CHANGE_CATEGORY
   | STATIC_DATA_MEMBER_CHANGE_CATEGORY
   | SIZE_OR_OFFSET_CHANGE_CATEGORY

include/abg-fwd.h

@@ -175,6 +175,9 @@ is_typedef(const shared_ptr<type_base>);
 shared_ptr<typedef_decl>
 is_typedef(const shared_ptr<decl_base>);
 
+shared_ptr<enum_type_decl>
+is_enum(const shared_ptr<decl_base>&);
+
 shared_ptr<class_decl>
 is_class_type(const shared_ptr<type_base>);
 
@@ -184,7 +187,8 @@ is_class_type(const shared_ptr<decl_base>);
 shared_ptr<class_decl>
 look_through_decl_only_class(shared_ptr<class_decl>);
 
-bool
+
+shared_ptr<var_decl>
 is_var_decl(const shared_ptr<decl_base>);
 
 bool
@@ -246,12 +250,10 @@ bool
 get_member_is_static(const shared_ptr<decl_base>);
 
 void
-set_member_is_static(decl_base&,
-		     bool);
+set_member_is_static(decl_base&, bool);
 
 void
-set_member_is_static(shared_ptr<decl_base>,
-		     bool);
+set_member_is_static(shared_ptr<decl_base>, bool);
 
 bool
 is_data_member(const var_decl&);
@@ -262,6 +264,9 @@ is_data_member(const var_decl*);
 bool
 is_data_member(const shared_ptr<var_decl>);
 
+bool
+is_data_member(const shared_ptr<decl_base>&);
+
 shared_ptr<array_type_def>
 is_array_def(const shared_ptr<type_base> decl);
 

src/abg-comp-filter.cc

@@ -1,6 +1,6 @@
 // -*- Mode: C++ -*-
 //
-// Copyright (C) 2013 Red Hat, Inc.
+// Copyright (C) 2013-2014 Red Hat, Inc.
 //
 // This file is part of the GNU Application Binary Interface Generic
 // Analysis and Instrumentation Library (libabigail).  This library is
@@ -254,13 +254,32 @@ decl_name_changed(decl_base_sptr d1, decl_base_sptr d2)
   string d1_name, d2_name;
 
   if (d1)
-    d1_name = d1->get_name();
+    d1_name = d1->get_qualified_name();
   if (d2)
-    d2_name = d2->get_name();
+    d2_name = d2->get_qualified_name();
 
   return d1_name != d2_name;
 }
 
+/// Test if two decls represents a harmless name change.
+///
+/// For now, a harmless name change is a name change for a typedef,
+/// enum or data member.
+///
+/// @param f the first decl to consider in the comparison.
+///
+/// @param s the second decl to consider in the comparison.
+///
+/// @return true iff decl @p s represents a harmless change over @p f.
+bool
+has_harmless_name_change(decl_base_sptr f, decl_base_sptr s)
+{
+  return (decl_name_changed(f, s)
+	  && ((is_typedef(f) && is_typedef(s))
+	      || (is_data_member(f) && is_data_member(s))
+	      || (is_enum(f) && is_enum(s))));
+}
+
 /// Test if a class_diff node has non-static members added or
 /// removed.
 ///
@@ -531,8 +550,8 @@ harmless_filter::visit(diff* d, bool pre)
       if (is_compatible_change(f, s))
 	category |= COMPATIBLE_TYPE_CHANGE_CATEGORY;
 
-      if (decl_name_changed(f, s))
-	category |= DECL_NAME_CHANGE_CATEGORY;
+      if (has_harmless_name_change(f, s))
+	category |= HARMLESS_DECL_NAME_CHANGE_CATEGORY;
 
       if (has_non_virtual_mem_fn_change(d))
 	category |= NON_VIRT_MEM_FUN_CHANGE_CATEGORY;

src/abg-comparison.cc

@@ -1342,15 +1342,21 @@ represent(var_decl_sptr	o,
   string name2 = n->get_qualified_name();
   string pretty_representation = o->get_pretty_representation();
 
-  if (ctxt->get_allowed_category() & DECL_NAME_CHANGE_CATEGORY
-      && name1 !=  name2)
+  if (name1 !=  name2)
     {
-      if (!emitted)
-	out << indent << "'" << pretty_representation << "' ";
+      if (filtering::has_harmless_name_change(o, n)
+	  && !(ctxt->get_allowed_category()
+	       & HARMLESS_DECL_NAME_CHANGE_CATEGORY))
+	;
       else
-	out << ", ";
-      out << "name changed to '" << name2 << "'";
-      emitted = true;
+	{
+	  if (!emitted)
+	    out << indent << "'" << pretty_representation << "' ";
+	  else
+	    out << ", ";
+	  out << "name changed to '" << name2 << "'";
+	  emitted = true;
+	}
     }
 
   if (get_data_member_is_laid_out(o)
@@ -1535,14 +1541,21 @@ report_name_size_and_alignment_changes(decl_base_sptr		first,
   string fn = first->get_qualified_name(),
     sn = second->get_qualified_name();
 
-  if (fn != sn
-      && ctxt->get_allowed_category() & DECL_NAME_CHANGE_CATEGORY)
+  if (fn != sn)
     {
-      if (nl)
-	out << "\n";
-      out << indent << "name changed from '"
-	  << fn << "' to '" << sn << "'";
-      nl = true;
+      if (!(ctxt->get_allowed_category() & HARMLESS_DECL_NAME_CHANGE_CATEGORY)
+	  && filtering::has_harmless_name_change(first, second))
+	// This is a harmless name change.  but then
+	// HARMLESS_DECL_NAME_CHANGE_CATEGORY doesn't seem allowed.
+	;
+      else
+	{
+	  if (nl)
+	    out << "\n";
+	  out << indent << "name changed from '"
+	      << fn << "' to '" << sn << "'";
+	  nl = true;
+	}
     }
 
   nl |= report_size_and_alignment_changes(first, second, ctxt,
@@ -2769,13 +2782,6 @@ enum_diff::report(ostream& out, const string& indent) const
     out << "\n";
   maybe_report_diff_for_member(first, second, context(), out, indent);
 
-  // name
-  if (first->get_name() != second->get_name()
-      && context()->get_allowed_category() & DECL_NAME_CHANGE_CATEGORY)
-    out << indent << "enum name changed from '"
-	<< first->get_qualified_name() << "' to '"
-	<< second->get_qualified_name() << "'\n";
-
   //underlying type
   underlying_type_diff()->report(out, indent);
 
@@ -5884,8 +5890,8 @@ typedef_diff::report(ostream& out, const string& indent) const
 
   maybe_report_diff_for_member(f, s, context(), out, indent);
 
-  if (f->get_name() != s->get_name()
-      && context()->get_allowed_category() & DECL_NAME_CHANGE_CATEGORY)
+  if (filtering::has_harmless_name_change(f, s)
+      && context()->get_allowed_category() & HARMLESS_DECL_NAME_CHANGE_CATEGORY)
     {
       out << indent << "typedef name changed from "
 	  << f->get_qualified_name()

src/abg-ir.cc

@@ -1595,6 +1595,19 @@ bool
 is_data_member(const var_decl_sptr d)
 {return is_at_class_scope(d);}
 
+/// Test if a decl is a data member.
+///
+/// @param d the decl to consider.
+///
+/// @return true iff @p d is a data member.
+bool
+is_data_member(const decl_base_sptr& d)
+{
+  if (var_decl_sptr v = is_var_decl(d))
+    return is_data_member(v);
+  return false;
+}
+
 /// Set the offset of a data member into its containing class.
 ///
 /// @param m the data member to consider.
@@ -2593,6 +2606,24 @@ typedef_decl_sptr
 is_typedef(const decl_base_sptr d)
 {return is_typedef(is_type(d));}
 
+/// Test if a decl is an enum_type_decl
+///
+/// @param d the decl to test for.
+///
+/// @return the enum_type_decl_sptr if @p d is an enum, nil otherwise.
+enum_type_decl_sptr
+is_enum(const decl_base_sptr& d)
+{return dynamic_pointer_cast<enum_type_decl>(d);}
+
+/// Test if a type is an enum_type_decl
+///
+/// @param t the type to test for.
+///
+/// @return the enum_type_decl_sptr if @p t is an enum, nil otherwise.
+enum_type_decl_sptr
+is_enum(const type_base_sptr& t)
+{return dynamic_pointer_cast<enum_type_decl>(t);}
+
 /// Test whether a type is a class.
 ///
 /// This function looks through typedefs.
@@ -2645,10 +2676,11 @@ look_through_decl_only_class(class_decl_sptr klass)
 ///
 /// @param decl the decl to test.
 ///
-/// @return true iff decl is a variable declaration.
-bool
+/// @return the var_decl_sptr iff decl is a variable declaration; nil
+/// otherwise.
+var_decl_sptr
 is_var_decl(const shared_ptr<decl_base> decl)
-{return decl && dynamic_pointer_cast<var_decl>(decl);}
+{return dynamic_pointer_cast<var_decl>(decl);}
 
 /// Tests whether a decl is a template parameter composition type.
 ///

tests/data/test-diff-filter/test13-report.txt

@@ -13,6 +13,7 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
         1 data member change:
          'int S::m0' offset changed from 32 to 64
          and its type 'int' changed:
+           name changed from 'int' to 'char'
            size changed from 32 to 8 bits
            alignment changed from 32 to 8 bits
 

tests/data/test-diff-filter/test6-report.txt

@@ -6,6 +6,7 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
   [C]'function return_type foo()' has some indirect sub-type changes:
     return type changed:
       underlying type 'unsigned char' changed:
+        name changed from 'unsigned char' to 'unsigned int'
         size changed from 8 to 32 bits
         alignment changed from 8 to 32 bits
 

tools/bidiff.cc

@@ -335,7 +335,7 @@ set_diff_context_from_opts(diff_context_sptr ctxt,
       ctxt->switch_categories_off
 	(abigail::comparison::ACCESS_CHANGE_CATEGORY
 	 | abigail::comparison::COMPATIBLE_TYPE_CHANGE_CATEGORY
-	 | abigail::comparison::DECL_NAME_CHANGE_CATEGORY
+	 | abigail::comparison::HARMLESS_DECL_NAME_CHANGE_CATEGORY
 	 | abigail::comparison::NON_VIRT_MEM_FUN_CHANGE_CATEGORY
 	 | abigail::comparison::STATIC_DATA_MEMBER_CHANGE_CATEGORY);
   if (!opts.show_harmful_changes)