Report DiffStatus for access specifiers

If a class member's access specifier is less restricted than the old
declaration, header-abi-diff reports it as an extension rather than
no difference.

Bug: 323447559
Test: ANDROID_BUILD_TOP=`realpath .` \
      PATH=$PATH:`realpath out/soong/dist/bin` \
      development/vndk/tools/header-checker/tests/test.py
Change-Id: I91b5e47d3d118db2c7bf07579cc60189f4e6c139

4 files changed, 23 insertions, 12 deletions

diff --git a/vndk/tools/header-checker/src/repr/abi_diff_helpers.cpp b/vndk/tools/header-checker/src/repr/abi_diff_helpers.cpp
index 57c68c688..f99aa615b 100644
--- a/vndk/tools/header-checker/src/repr/abi_diff_helpers.cpp
+++ b/vndk/tools/header-checker/src/repr/abi_diff_helpers.cpp
@@ -346,6 +346,17 @@ static bool CompareSizeAndAlignment(const TypeIR *old_type,
       old_type->GetAlignment() == new_type->GetAlignment();
 }
 
+DiffStatus AbiDiffHelper::CompareAccess(AccessSpecifierIR old_access,
+                                        AccessSpecifierIR new_access) {
+  if (old_access == new_access) {
+    return DiffStatus::kNoDiff;
+  }
+  if (old_access > new_access) {
+    return DiffStatus::kDirectExt;
+  }
+  return DiffStatus::kDirectDiff;
+}
+
 DiffStatus AbiDiffHelper::CompareCommonRecordFields(
     const RecordFieldIR *old_field, const RecordFieldIR *new_field,
     DiffMessageIR::DiffKind diff_kind) {
@@ -355,12 +366,11 @@ DiffStatus AbiDiffHelper::CompareCommonRecordFields(
   // CompareAndDumpTypeDiff should not return kDirectExt.
   // In case it happens, report an incompatible diff for review.
   if (field_diff_status.IsExtension() ||
-      old_field->GetOffset() != new_field->GetOffset() ||
-      // TODO: Should this be an inquality check instead ? Some compilers can
-      // make signatures dependant on absolute values of access specifiers.
-      IsAccessDowngraded(old_field->GetAccess(), new_field->GetAccess())) {
+      old_field->GetOffset() != new_field->GetOffset()) {
     field_diff_status.CombineWith(DiffStatus::kDirectDiff);
   }
+  field_diff_status.CombineWith(
+      CompareAccess(old_field->GetAccess(), new_field->GetAccess()));
   return field_diff_status;
 }
 
@@ -603,8 +613,10 @@ DiffStatus AbiDiffHelper::CompareRecordTypes(
   record_type_diff_ir->SetName(old_type->GetName());
   record_type_diff_ir->SetLinkerSetKey(old_type->GetLinkerSetKey());
 
-  if (IsAccessDowngraded(old_type->GetAccess(), new_type->GetAccess())) {
-    final_diff_status.CombineWith(DiffStatus::kDirectDiff);
+  DiffStatus access_diff_status =
+      CompareAccess(old_type->GetAccess(), new_type->GetAccess());
+  final_diff_status.CombineWith(access_diff_status);
+  if (access_diff_status.HasDiff()) {
     record_type_diff_ir->SetAccessDiff(
         std::make_unique<AccessSpecifierDiffIR>(
             old_type->GetAccess(), new_type->GetAccess()));
diff --git a/vndk/tools/header-checker/src/repr/abi_diff_helpers.h b/vndk/tools/header-checker/src/repr/abi_diff_helpers.h
index a0bd384c4..5604358ba 100644
--- a/vndk/tools/header-checker/src/repr/abi_diff_helpers.h
+++ b/vndk/tools/header-checker/src/repr/abi_diff_helpers.h
@@ -176,6 +176,9 @@ class AbiDiffHelper {
   FixupDiffedFieldTypeIds(
       const std::vector<RecordFieldDiffIR> &field_diffs);
 
+  DiffStatus CompareAccess(AccessSpecifierIR old_access,
+                           AccessSpecifierIR new_access);
+
   DiffStatus CompareCommonRecordFields(const RecordFieldIR *old_field,
                                        const RecordFieldIR *new_field,
                                        IRDiffDumper::DiffKind diff_kind);
diff --git a/vndk/tools/header-checker/src/repr/ir_representation.h b/vndk/tools/header-checker/src/repr/ir_representation.h
index ee186ae2b..8f76f650c 100644
--- a/vndk/tools/header-checker/src/repr/ir_representation.h
+++ b/vndk/tools/header-checker/src/repr/ir_representation.h
@@ -65,16 +65,12 @@ static inline CompatibilityStatusIR operator&(CompatibilityStatusIR f,
 }
 
 enum AccessSpecifierIR {
+  // Ordered from the least to the most restricted.
   PublicAccess = 1,
   ProtectedAccess = 2,
   PrivateAccess = 3
 };
 
-static inline bool IsAccessDowngraded(AccessSpecifierIR old_access,
-                                      AccessSpecifierIR new_access) {
-  return old_access < new_access;
-}
-
 enum LinkableMessageKind {
   RecordTypeKind,
   EnumTypeKind,
diff --git a/vndk/tools/header-checker/tests/test.py b/vndk/tools/header-checker/tests/test.py
index 027a1a69a..0ca37e073 100755
--- a/vndk/tools/header-checker/tests/test.py
+++ b/vndk/tools/header-checker/tests/test.py
@@ -296,7 +296,7 @@ class HeaderCheckerTest(unittest.TestCase):
     def test_libgolden_cpp_internal_struct_access_upgraded(self):
         self.prepare_and_run_abi_diff_all_archs(
             "libgolden_cpp_internal_private_struct",
-            "libgolden_cpp_internal_public_struct", 0, [], True, True)
+            "libgolden_cpp_internal_public_struct", 4, [], True, True)
 
     def test_libgolden_cpp_internal_struct_access_downgraded(self):
         self.prepare_and_run_abi_diff_all_archs(