Categorize CV qualifier changes on fn return types as harmless
authorDodji Seketeli <dodji@redhat.com>
Tue, 2 Oct 2018 11:49:55 +0000 (13:49 +0200)
committerDodji Seketeli <dodji@redhat.com>
Tue, 2 Oct 2018 12:02:22 +0000 (14:02 +0200)
This partially fixes PR23700.

A change in the CV qualifiers of a function return value type should
be categorized as harmless.  And this is what this patch does.

* include/abg-comparison.h (FN_RETURN_TYPE_CV_CHANGE_CATEGORY):
New enumerator for diff_category.
(EVERYTHING_CATEGORY): Update.
* src/abg-comp-filter.cc (type_diff_has_cv_qual_change_only):
Factorize this function out of ...
(has_fn_parm_type_cv_qual_change): ... this one.
(has_fn_return_type_cv_qual_change): Define new static function.
(categorize_harmless_diff_node): Use the new
has_fn_return_type_cv_qual_change.
* src/abg-comparison.cc (get_default_harmless_categories_bitmap):
Adjust to add the new FN_RETURN_TYPE_CV_CHANGE_CATEGORY category.
(operator<<(ostream& o, diff_category c)): Support the new
FN_RETURN_TYPE_CV_CHANGE_CATEGORY.
* tests/data/Makefile.am: Add the new test material below to
source distribution.
* tests/data/test-diff-filter/test46-fn-return-qual-change-report-0.txt:
New reference output for the new input test.
* tests/data/test-diff-filter/test46-fn-return-qual-change-v{0,1}.c:
New source code for the new binary test input.
* tests/data/test-diff-filter/test46-fn-return-qual-change-v{0,1}.o:
New binary test input files.
* tests/test-diff-filter.cc: Add the new test input above to test
harness.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
include/abg-comparison.h
src/abg-comp-filter.cc
src/abg-comparison.cc
tests/data/Makefile.am
tests/data/test-diff-filter/test46-fn-return-qual-change-report-0.txt [new file with mode: 0644]
tests/data/test-diff-filter/test46-fn-return-qual-change-v0.c [new file with mode: 0644]
tests/data/test-diff-filter/test46-fn-return-qual-change-v0.o [new file with mode: 0644]
tests/data/test-diff-filter/test46-fn-return-qual-change-v1.c [new file with mode: 0644]
tests/data/test-diff-filter/test46-fn-return-qual-change-v1.o [new file with mode: 0644]

index bc1f221..51c5db5 100644 (file)
@@ -386,10 +386,14 @@ enum diff_category
   /// top cv-qualifiers change.
   FN_PARM_TYPE_TOP_CV_CHANGE_CATEGORY = 1 << 13,
 
-  /// A diff node in this category is a function parameter type which
+  /// A diff node in this category has a function parameter type with a
   /// cv-qualifiers change.
   FN_PARM_TYPE_CV_CHANGE_CATEGORY = 1 << 14,
 
+  /// A diff node in this category is a function return type with a
+  /// cv-qualifier change.
+  FN_RETURN_TYPE_CV_CHANGE_CATEGORY = 1 << 15,
+
   /// A special enumerator that is the logical 'or' all the
   /// enumerators above.
   ///
@@ -411,6 +415,7 @@ enum diff_category
   | CLASS_DECL_ONLY_DEF_CHANGE_CATEGORY
   | FN_PARM_TYPE_TOP_CV_CHANGE_CATEGORY
   | FN_PARM_TYPE_CV_CHANGE_CATEGORY
+  | FN_RETURN_TYPE_CV_CHANGE_CATEGORY
 }; // enum diff_category
 
 diff_category
index 057acc7..b3653cb 100644 (file)
@@ -1076,27 +1076,17 @@ has_fn_parm_type_top_cv_qual_change(const diff* diff)
   return false;
 }
 
-/// Test if an @ref fn_parm_diff node has a cv qualifier change on the
-/// type of the function parameter. That is, we are looking for
-/// changes like 'const char*' to 'char*'.
+/// Test if a type diff only carries a CV qualifier-only change.
 ///
-/// @param diff the diff node to consider.  It should be a @ref
-/// fn_parm_diff, otherwise the function returns 'false' directly.
+/// @param type_dif the type dif to consider.
 ///
-/// @return true iff @p diff is a @ref fn_parm_diff node that has a
-/// cv qualifier change on the type of the function parameter.
+/// @return true iff the type_diff carries a CV qualifier only change.
 static bool
-has_fn_parm_type_cv_qual_change(const diff* dif)
+type_diff_has_cv_qual_change_only(const diff *type_dif)
 {
-  // is diff a "function parameter diff node?
-  const fn_parm_diff* parm_diff = is_fn_parm_diff(dif);
-
-  if (!parm_diff || !parm_diff->has_changes())
-    // diff either carries no change or is not a function parameter
-    // diff node.  So bail out.
+  if (!is_type_diff(type_dif))
     return false;
 
-  const diff *type_dif = parm_diff->type_diff().get();
   if (is_pointer_diff(type_dif))
     type_dif = peel_pointer_diff(type_dif);
   else if (is_reference_diff(type_dif))
@@ -1106,13 +1096,13 @@ has_fn_parm_type_cv_qual_change(const diff* dif)
   const type_base *s = 0;
   if (const distinct_diff *d = is_distinct_diff(type_dif))
     {
-      if (is_qualified_type(d->first()) != is_qualified_type(d->second()))
+      if (is_qualified_type(d->first()) == is_qualified_type(d->second()))
+       return false;
+      else
        {
          f = is_type(d->first()).get();
          s = is_type(d->second()).get();
        }
-      else
-       return false;
     }
   else if (const qualified_type_diff *d = is_qualified_type_diff(type_dif))
     {
@@ -1128,6 +1118,53 @@ has_fn_parm_type_cv_qual_change(const diff* dif)
   return *f == *s;
 }
 
+/// Test if an @ref fn_parm_diff node has a cv qualifier change on the
+/// type of the function parameter. That is, we are looking for
+/// changes like 'const char*' to 'char*'.
+///
+/// @param diff the diff node to consider.  It should be a @ref
+/// fn_parm_diff, otherwise the function returns 'false' directly.
+///
+/// @return true iff @p diff is a @ref fn_parm_diff node that has a
+/// cv qualifier change on the type of the function parameter.
+static bool
+has_fn_parm_type_cv_qual_change(const diff* dif)
+{
+  // is diff a "function parameter diff node?
+  const fn_parm_diff* parm_diff = is_fn_parm_diff(dif);
+
+  if (!parm_diff || !parm_diff->has_changes())
+    // diff either carries no change or is not a function parameter
+    // diff node.  So bail out.
+    return false;
+
+  const diff *type_dif = parm_diff->type_diff().get();
+  return type_diff_has_cv_qual_change_only(type_dif);
+}
+
+/// Test if a function type or decl diff node carries a CV
+/// qualifier-only change on its return type.
+///
+/// @param the dif to consider.  Note that if this is neither a
+/// function type nor decl diff node, the function returns false.
+///
+/// @return true iff @p dif is a function decl or type diff node which
+/// carries a CV qualifier-only change on its return type.
+static bool
+has_fn_return_type_cv_qual_change(const diff* dif)
+{
+  const function_type_diff* fn_type_diff = is_function_type_diff(dif);
+  if (!fn_type_diff)
+    if (const function_decl_diff* fn_decl_diff = is_function_decl_diff(dif))
+      fn_type_diff = fn_decl_diff->type_diff().get();
+
+  if (!fn_type_diff)
+    return false;
+
+  const diff* return_type_diff = fn_type_diff->return_type_diff().get();
+  return type_diff_has_cv_qual_change_only(return_type_diff);
+}
+
 /// Detect if the changes carried by a given diff node are deemed
 /// harmless and do categorize the diff node accordingly.
 ///
@@ -1184,6 +1221,9 @@ categorize_harmless_diff_node(diff *d, bool pre)
       if (has_fn_parm_type_cv_qual_change(d))
        category |= FN_PARM_TYPE_CV_CHANGE_CATEGORY;
 
+      if (has_fn_return_type_cv_qual_change(d))
+       category |= FN_RETURN_TYPE_CV_CHANGE_CATEGORY;
+
       if (category)
        {
          d->add_to_local_and_inherited_categories(category);
index 5919abd..012c0eb 100644 (file)
@@ -2922,7 +2922,8 @@ get_default_harmless_categories_bitmap()
          | abigail::comparison::HARMLESS_SYMBOL_ALIAS_CHANGE_CATEORY
          | abigail::comparison::CLASS_DECL_ONLY_DEF_CHANGE_CATEGORY
          | abigail::comparison::FN_PARM_TYPE_TOP_CV_CHANGE_CATEGORY
-         | abigail::comparison::FN_PARM_TYPE_CV_CHANGE_CATEGORY);
+         | abigail::comparison::FN_PARM_TYPE_CV_CHANGE_CATEGORY
+         | abigail::comparison::FN_RETURN_TYPE_CV_CHANGE_CATEGORY);
 }
 
 /// Getter of a bitmap made of the set of change categories that are
@@ -3075,6 +3076,14 @@ operator<<(ostream& o, diff_category c)
       emitted_a_category |= true;
     }
 
+    if (c & FN_RETURN_TYPE_CV_CHANGE_CATEGORY)
+    {
+      if (emitted_a_category)
+       o << "|";
+      o << "FN_RETURN_TYPE_CV_CHANGE_CATEGORY";
+      emitted_a_category |= true;
+    }
+
   return o;
 }
 
index 09740f1..b0e78dd 100644 (file)
@@ -673,6 +673,11 @@ test-diff-filter/libtest45-basic-type-change-v0.so \
 test-diff-filter/libtest45-basic-type-change-v1.so \
 test-diff-filter/test45-basic-type-change-v0.cc \
 test-diff-filter/test45-basic-type-change-v1.cc \
+test-diff-filter/test46-fn-return-qual-change-report-0.txt \
+test-diff-filter/test46-fn-return-qual-change-v0.c \
+test-diff-filter/test46-fn-return-qual-change-v1.c \
+test-diff-filter/test46-fn-return-qual-change-v0.o \
+test-diff-filter/test46-fn-return-qual-change-v1.o \
 \
 test-diff-suppr/test0-type-suppr-v0.cc \
 test-diff-suppr/test0-type-suppr-v1.cc \
diff --git a/tests/data/test-diff-filter/test46-fn-return-qual-change-report-0.txt b/tests/data/test-diff-filter/test46-fn-return-qual-change-report-0.txt
new file mode 100644 (file)
index 0000000..9666a8f
--- /dev/null
@@ -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
+
diff --git a/tests/data/test-diff-filter/test46-fn-return-qual-change-v0.c b/tests/data/test-diff-filter/test46-fn-return-qual-change-v0.c
new file mode 100644 (file)
index 0000000..b1a25ab
--- /dev/null
@@ -0,0 +1,4 @@
+const char* foo()
+{
+  return 0;
+}
diff --git a/tests/data/test-diff-filter/test46-fn-return-qual-change-v0.o b/tests/data/test-diff-filter/test46-fn-return-qual-change-v0.o
new file mode 100644 (file)
index 0000000..e7a7211
Binary files /dev/null and b/tests/data/test-diff-filter/test46-fn-return-qual-change-v0.o differ
diff --git a/tests/data/test-diff-filter/test46-fn-return-qual-change-v1.c b/tests/data/test-diff-filter/test46-fn-return-qual-change-v1.c
new file mode 100644 (file)
index 0000000..f5451d6
--- /dev/null
@@ -0,0 +1,4 @@
+char* foo()
+{
+  return 0;
+}
diff --git a/tests/data/test-diff-filter/test46-fn-return-qual-change-v1.o b/tests/data/test-diff-filter/test46-fn-return-qual-change-v1.o
new file mode 100644 (file)
index 0000000..bd8e639
Binary files /dev/null and b/tests/data/test-diff-filter/test46-fn-return-qual-change-v1.o differ