From 6feb8391616554be424411bd8d2245e304aefdcf Mon Sep 17 00:00:00 2001
From: Logan Chien <loganchien@google.com>
Date: Tue, 2 Apr 2019 06:18:55 +0800
Subject: [PATCH 1/7] header-checker: Remove dead GetTypeLinkageName

Bug: 74764811
Test: ./tests/test.py
Change-Id: Ief3dc65d47b1c045be73f0dc2b91bea3c69286be
---
 vndk/tools/header-checker/src/dumper/abi_wrappers.cpp | 6 ------
 vndk/tools/header-checker/src/dumper/abi_wrappers.h   | 2 --
 2 files changed, 8 deletions(-)

diff --git a/vndk/tools/header-checker/src/dumper/abi_wrappers.cpp b/vndk/tools/header-checker/src/dumper/abi_wrappers.cpp
index b17a7fc22..2f767f618 100644
--- a/vndk/tools/header-checker/src/dumper/abi_wrappers.cpp
+++ b/vndk/tools/header-checker/src/dumper/abi_wrappers.cpp
@@ -303,12 +303,6 @@ bool ABIWrapper::CreateBasicNamedAndTypedDecl(
   return CreateBasicNamedAndTypedDecl(referenced_type, source_file);
 }
 
-std::string ABIWrapper::GetTypeLinkageName(const clang::Type *typep) {
-  assert(typep != nullptr);
-  clang::QualType qt = typep->getCanonicalTypeInternal();
-  return QualTypeToString(qt);
-}
-
 // This method returns a TypeAndCreationStatus object. This object contains a
 // type and information to tell the clients of this method whether the caller
 // should continue creating the type.
diff --git a/vndk/tools/header-checker/src/dumper/abi_wrappers.h b/vndk/tools/header-checker/src/dumper/abi_wrappers.h
index d7c22449d..6d1844681 100644
--- a/vndk/tools/header-checker/src/dumper/abi_wrappers.h
+++ b/vndk/tools/header-checker/src/dumper/abi_wrappers.h
@@ -86,8 +86,6 @@ class ABIWrapper {
 
   bool CreateAnonymousRecord(const clang::RecordDecl *decl);
 
-  std::string GetTypeLinkageName(const clang::Type *typep);
-
   TypeAndCreationStatus SetTypeKind(const clang::QualType qtype,
                                     const std::string &source_file);
 

From 02b0ccb55b54535cdeaeb14f7fd045670f6766c8 Mon Sep 17 00:00:00 2001
From: Logan Chien <loganchien@google.com>
Date: Tue, 2 Apr 2019 06:31:25 +0800
Subject: [PATCH 2/7] header-checker: Re-group the code in abi_wrappers

Bug: 74764811
Test: ./tests/test.py
Change-Id: I86c126909f9ca14fb9af358e98a52448639283f0
---
 .../src/dumper/abi_wrappers.cpp               | 349 ++++++++++--------
 .../header-checker/src/dumper/abi_wrappers.h  |  25 +-
 2 files changed, 210 insertions(+), 164 deletions(-)

diff --git a/vndk/tools/header-checker/src/dumper/abi_wrappers.cpp b/vndk/tools/header-checker/src/dumper/abi_wrappers.cpp
index 2f767f618..cd2ed70a5 100644
--- a/vndk/tools/header-checker/src/dumper/abi_wrappers.cpp
+++ b/vndk/tools/header-checker/src/dumper/abi_wrappers.cpp
@@ -31,6 +31,30 @@ namespace header_checker {
 namespace dumper {
 
 
+//------------------------------------------------------------------------------
+// Helper Function
+//------------------------------------------------------------------------------
+
+static repr::AccessSpecifierIR AccessClangToIR(
+    const clang::AccessSpecifier sp) {
+  switch (sp) {
+    case clang::AS_private: {
+      return repr::AccessSpecifierIR::PrivateAccess;
+    }
+    case clang::AS_protected: {
+      return repr::AccessSpecifierIR::ProtectedAccess;
+    }
+    default: {
+      return repr::AccessSpecifierIR::PublicAccess;
+    }
+  }
+}
+
+
+//------------------------------------------------------------------------------
+// ABI Wrapper
+//------------------------------------------------------------------------------
+
 ABIWrapper::ABIWrapper(
     clang::MangleContext *mangle_contextp,
     clang::ASTContext *ast_contextp,
@@ -43,6 +67,21 @@ ABIWrapper::ABIWrapper(
       module_(module),
       ast_caches_(ast_caches) {}
 
+std::string ABIWrapper::GetDeclSourceFile(const clang::Decl *decl,
+                                          const clang::CompilerInstance *cip) {
+  clang::SourceManager &sm = cip->getSourceManager();
+  clang::SourceLocation location = decl->getLocation();
+  // We need to use the expansion location to identify whether we should recurse
+  // into the AST Node or not. For eg: macros specifying LinkageSpecDecl can
+  // have their spelling location defined somewhere outside a source / header
+  // file belonging to a library. This should not allow the AST node to be
+  // skipped. Its expansion location will still be the source-file / header
+  // belonging to the library.
+  clang::SourceLocation expansion_location = sm.getExpansionLoc(location);
+  llvm::StringRef file_name = sm.getFilename(expansion_location);
+  return utils::RealPath(file_name.str());
+}
+
 std::string ABIWrapper::GetCachedDeclSourceFile(
     const clang::Decl *decl, const clang::CompilerInstance *cip) {
   assert(decl != nullptr);
@@ -53,6 +92,55 @@ std::string ABIWrapper::GetCachedDeclSourceFile(
   return result->second;
 }
 
+std::string ABIWrapper::GetMangledNameDecl(
+    const clang::NamedDecl *decl, clang::MangleContext *mangle_contextp) {
+  if (!mangle_contextp->shouldMangleDeclName(decl)) {
+    clang::IdentifierInfo *identifier = decl->getIdentifier();
+    return identifier ? identifier->getName() : "";
+  }
+  std::string mangled_name;
+  llvm::raw_string_ostream ostream(mangled_name);
+  mangle_contextp->mangleName(decl, ostream);
+  ostream.flush();
+  return mangled_name;
+}
+
+bool ABIWrapper::SetupTemplateArguments(const clang::TemplateArgumentList *tl,
+                                        repr::TemplatedArtifactIR *ta,
+                                        const std::string &source_file) {
+  repr::TemplateInfoIR template_info;
+  for (int i = 0; i < tl->size(); i++) {
+    const clang::TemplateArgument &arg = (*tl)[i];
+    // TODO: More comprehensive checking needed.
+    if (arg.getKind() != clang::TemplateArgument::Type) {
+      continue;
+    }
+    clang::QualType type = arg.getAsType();
+    template_info.AddTemplateElement(
+        repr::TemplateElementIR(
+            ast_caches_->GetTypeId(GetKeyForTypeId(type))));
+    if (!CreateBasicNamedAndTypedDecl(type, source_file)) {
+      llvm::errs() << "Setting up template arguments failed\n";
+      return false;
+    }
+  }
+  ta->SetTemplateInfo(std::move(template_info));
+  return true;
+}
+
+bool ABIWrapper::SetupFunctionParameter(
+    repr::CFunctionLikeIR *functionp, const clang::QualType qual_type,
+    bool has_default_arg, const std::string &source_file, bool is_this_ptr) {
+  if (!CreateBasicNamedAndTypedDecl(qual_type, source_file)) {
+    llvm::errs() << "Setting up function parameter failed\n";
+    return false;
+  }
+  functionp->AddParameter(repr::ParamIR(
+      ast_caches_->GetTypeId(GetKeyForTypeId(qual_type)), has_default_arg,
+      is_this_ptr));
+  return true;
+}
+
 static const clang::TagDecl *GetTagDecl(clang::QualType qual_type) {
   const clang::Type *type_ptr = qual_type.getCanonicalType().getTypePtr();
   assert(type_ptr != nullptr);
@@ -105,7 +193,25 @@ static bool IsReferencingType(clang::QualType qual_type) {
   return is_array || is_ptr || is_reference || qual_type.hasLocalQualifiers();
 }
 
-static clang::QualType GetReferencedType(const clang::QualType qual_type);
+// Get type 'referenced' by qual_type. Referenced type implies, in order:
+// 1) Strip off all qualifiers if qual_type has CVR qualifiers.
+// 2) Strip off a pointer level if qual_type is a pointer.
+// 3) Strip off the reference if qual_type is a reference.
+// Note: qual_type is expected to be a canonical type.
+static clang::QualType GetReferencedType(const clang::QualType qual_type) {
+  const clang::Type *type_ptr = qual_type.getTypePtr();
+  if (qual_type.hasLocalQualifiers()) {
+    return qual_type.getLocalUnqualifiedType();
+  }
+  if (type_ptr->isPointerType()) {
+    return type_ptr->getPointeeType();
+  }
+  if (type_ptr->isArrayType()) {
+    return
+        type_ptr->getArrayElementTypeNoTypeQual()->getCanonicalTypeInternal();
+  }
+  return qual_type.getNonReferenceType();
+}
 
 static clang::QualType GetFinalReferencedType(clang::QualType qual_type) {
   while (IsReferencingType(qual_type)) {
@@ -114,7 +220,8 @@ static clang::QualType GetFinalReferencedType(clang::QualType qual_type) {
   return qual_type;
 }
 
-std::string ABIWrapper::TypeNameWithFinalDestination(clang::QualType qual_type) {
+std::string ABIWrapper::TypeNameWithFinalDestination(
+    clang::QualType qual_type) {
   clang::QualType canonical_qual_type = qual_type.getCanonicalType();
   const std::string qual_type_name = QualTypeToString(canonical_qual_type);
   clang::QualType final_destination_type =
@@ -141,65 +248,12 @@ std::string ABIWrapper::GetKeyForTypeId(clang::QualType qual_type) {
       GetTypeUniqueId(GetTagDecl(final_destination_type));
 }
 
-std::string ABIWrapper::GetDeclSourceFile(const clang::Decl *decl,
-                                          const clang::CompilerInstance *cip) {
-  clang::SourceManager &sm = cip->getSourceManager();
-  clang::SourceLocation location = decl->getLocation();
-  // We need to use the expansion location to identify whether we should recurse
-  // into the AST Node or not. For eg: macros specifying LinkageSpecDecl can
-  // have their spelling location defined somewhere outside a source / header
-  // file belonging to a library. This should not allow the AST node to be
-  // skipped. Its expansion location will still be the source-file / header
-  // belonging to the library.
-  clang::SourceLocation expansion_location = sm.getExpansionLoc(location);
-  llvm::StringRef file_name = sm.getFilename(expansion_location);
-  return utils::RealPath(file_name.str());
-}
-
-static repr::AccessSpecifierIR AccessClangToIR(
-    const clang::AccessSpecifier sp) {
-  switch (sp) {
-    case clang::AS_private: {
-      return repr::AccessSpecifierIR::PrivateAccess;
-      break;
-    }
-    case clang::AS_protected: {
-      return repr::AccessSpecifierIR::ProtectedAccess;
-      break;
-    }
-    default: {
-      return repr::AccessSpecifierIR::PublicAccess;
-      break;
-    }
-  }
-}
-
 bool ABIWrapper::CreateAnonymousRecord(const clang::RecordDecl *record_decl) {
   RecordDeclWrapper record_decl_wrapper(mangle_contextp_, ast_contextp_, cip_,
                                         record_decl, module_, ast_caches_);
   return record_decl_wrapper.GetRecordDecl();
 }
 
-// Get type 'referenced' by qual_type. Referenced type implies, in order:
-// 1) Strip off all qualifiers if qual_type has CVR qualifiers.
-// 2) Strip off a pointer level if qual_type is a pointer.
-// 3) Strip off the reference if qual_type is a reference.
-// Note: qual_type is expected to be a canonical type.
-static clang::QualType GetReferencedType(const clang::QualType qual_type) {
-  const clang::Type *type_ptr = qual_type.getTypePtr();
-  if (qual_type.hasLocalQualifiers()) {
-    return qual_type.getLocalUnqualifiedType();
-  }
-  if (type_ptr->isPointerType()) {
-    return type_ptr->getPointeeType();
-  }
-  if (type_ptr->isArrayType()) {
-    return
-        type_ptr->getArrayElementTypeNoTypeQual()->getCanonicalTypeInternal();
-  }
-  return qual_type.getNonReferenceType();
-}
-
 bool ABIWrapper::CreateExtendedType(clang::QualType qual_type,
                                     repr::TypeIR *typep) {
   const clang::QualType canonical_type = qual_type.getCanonicalType();
@@ -207,50 +261,6 @@ bool ABIWrapper::CreateExtendedType(clang::QualType qual_type,
   return CreateBasicNamedAndTypedDecl(canonical_type, typep, "");
 }
 
-// This overload takes in a qualtype and adds its information to the abi-dump on
-// its own.
-bool ABIWrapper::CreateBasicNamedAndTypedDecl(clang::QualType qual_type,
-                                              const std::string &source_file) {
-  const std::string &type_key = GetKeyForTypeId(qual_type);
-  const clang::QualType canonical_type = qual_type.getCanonicalType();
-  const clang::Type *base_type = canonical_type.getTypePtr();
-  bool is_builtin = base_type->isBuiltinType();
-  bool should_continue_with_recursive_type_creation =
-      IsReferencingType(canonical_type) || is_builtin ||
-      base_type->isFunctionType() ||
-      (GetAnonymousRecord(canonical_type) != nullptr);
-  if (!should_continue_with_recursive_type_creation ||
-      !ast_caches_->type_cache_.insert(type_key).second) {
-    return true;
-  }
-  // Do something similar to what is being done right now. Create an object
-  // extending Type and return a pointer to that and pass it to CreateBasic...
-  // CreateBasic...(qualtype, Type *) fills in size, alignemnt etc.
-  auto type_and_status = SetTypeKind(canonical_type, source_file);
-  std::unique_ptr<repr::TypeIR> typep = std::move(type_and_status.typep_);
-  if (!base_type->isVoidType() && type_and_status.should_create_type_ &&
-      !typep) {
-    llvm::errs() << "nullptr with valid type while creating basic type\n";
-    return false;
-  }
-  if (!type_and_status.should_create_type_) {
-    return true;
-  }
-  return (CreateBasicNamedAndTypedDecl(
-              canonical_type, typep.get(), source_file) &&
-          module_->AddLinkableMessage(*typep));
-}
-
-std::string RecordDeclWrapper::GetMangledRTTI(
-    const clang::CXXRecordDecl *cxx_record_decl) {
-  clang::QualType qual_type =
-      cxx_record_decl->getTypeForDecl()->getCanonicalTypeInternal();
-  llvm::SmallString<256> uid;
-  llvm::raw_svector_ostream out(uid);
-  mangle_contextp_->mangleCXXRTTI(qual_type, out);
-  return uid.str();
-}
-
 std::string ABIWrapper::GetTypeUniqueId(const clang::TagDecl *tag_decl) {
   if (!tag_decl) {
     return "";
@@ -277,20 +287,22 @@ bool ABIWrapper::CreateBasicNamedAndTypedDecl(
   const clang::Type *base_type = canonical_type.getTypePtr();
   assert(base_type != nullptr);
   clang::Type::TypeClass type_class = base_type->getTypeClass();
-  // Temporary hack for auto type sizes. Not determinable.
-  if ((type_class != clang::Type::Auto) && !base_type->isIncompleteType() &&
-      !(base_type->isDependentType())) {
+
+  // Set the size and alignment of the type.
+  // Temporary hack: Skip the auto types, incomplete types and dependent types.
+  if (type_class != clang::Type::Auto && !base_type->isIncompleteType() &&
+      !base_type->isDependentType()) {
     std::pair<clang::CharUnits, clang::CharUnits> size_and_alignment =
-    ast_contextp_->getTypeInfoInChars(canonical_type);
-    size_t size = size_and_alignment.first.getQuantity();
-    size_t alignment = size_and_alignment.second.getQuantity();
-    typep->SetSize(size);
-    typep->SetAlignment(alignment);
+        ast_contextp_->getTypeInfoInChars(canonical_type);
+    typep->SetSize(size_and_alignment.first.getQuantity());
+    typep->SetAlignment(size_and_alignment.second.getQuantity());
   }
+
   std::string type_name_with_destination =
       TypeNameWithFinalDestination(canonical_type);
   typep->SetName(type_name_with_destination);
   typep->SetLinkerSetKey(type_name_with_destination);
+
   // Default values are false, we don't set them since explicitly doing that
   // makes the ABI dumps more verbose.
   // This type has a reference type if its a pointer / reference OR it has CVR
@@ -298,11 +310,50 @@ bool ABIWrapper::CreateBasicNamedAndTypedDecl(
   clang::QualType referenced_type = GetReferencedType(canonical_type);
   typep->SetReferencedType(
       ast_caches_->GetTypeId(GetKeyForTypeId(referenced_type)));
+
   typep->SetSelfType(ast_caches_->GetTypeId(GetKeyForTypeId(canonical_type)));
+
   // Create the type for referenced type.
   return CreateBasicNamedAndTypedDecl(referenced_type, source_file);
 }
 
+// This overload takes in a qualtype and adds its information to the abi-dump on
+// its own.
+bool ABIWrapper::CreateBasicNamedAndTypedDecl(clang::QualType qual_type,
+                                              const std::string &source_file) {
+  const std::string &type_key = GetKeyForTypeId(qual_type);
+  const clang::QualType canonical_type = qual_type.getCanonicalType();
+  const clang::Type *base_type = canonical_type.getTypePtr();
+  bool is_builtin = base_type->isBuiltinType();
+  bool should_continue_with_recursive_type_creation =
+      IsReferencingType(canonical_type) || is_builtin ||
+      base_type->isFunctionType() ||
+      (GetAnonymousRecord(canonical_type) != nullptr);
+  if (!should_continue_with_recursive_type_creation ||
+      !ast_caches_->type_cache_.insert(type_key).second) {
+    return true;
+  }
+
+  // Do something similar to what is being done right now. Create an object
+  // extending Type and return a pointer to that and pass it to CreateBasic...
+  // CreateBasic...(qualtype, Type *) fills in size, alignemnt etc.
+  auto type_and_status = SetTypeKind(canonical_type, source_file);
+  std::unique_ptr<repr::TypeIR> typep = std::move(type_and_status.typep_);
+  if (!base_type->isVoidType() && type_and_status.should_create_type_ &&
+      !typep) {
+    llvm::errs() << "nullptr with valid type while creating basic type\n";
+    return false;
+  }
+
+  if (!type_and_status.should_create_type_) {
+    return true;
+  }
+
+  return (CreateBasicNamedAndTypedDecl(
+              canonical_type, typep.get(), source_file) &&
+          module_->AddLinkableMessage(*typep));
+}
+
 // This method returns a TypeAndCreationStatus object. This object contains a
 // type and information to tell the clients of this method whether the caller
 // should continue creating the type.
@@ -373,19 +424,6 @@ TypeAndCreationStatus ABIWrapper::SetTypeKind(
   return TypeAndCreationStatus(nullptr, false);
 }
 
-std::string ABIWrapper::GetMangledNameDecl(
-    const clang::NamedDecl *decl, clang::MangleContext *mangle_contextp) {
-  if (!mangle_contextp->shouldMangleDeclName(decl)) {
-    clang::IdentifierInfo *identifier = decl->getIdentifier();
-    return identifier ? identifier->getName() : "";
-  }
-  std::string mangled_name;
-  llvm::raw_string_ostream ostream(mangled_name);
-  mangle_contextp->mangleName(decl, ostream);
-  ostream.flush();
-  return mangled_name;
-}
-
 std::string ABIWrapper::GetTagDeclQualifiedName(const clang::TagDecl *decl) {
   if (decl->getTypedefNameForAnonDecl()) {
     return decl->getTypedefNameForAnonDecl()->getQualifiedNameAsString();
@@ -393,29 +431,6 @@ std::string ABIWrapper::GetTagDeclQualifiedName(const clang::TagDecl *decl) {
   return decl->getQualifiedNameAsString();
 }
 
-bool ABIWrapper::SetupTemplateArguments(const clang::TemplateArgumentList *tl,
-                                        repr::TemplatedArtifactIR *ta,
-                                        const std::string &source_file) {
-  repr::TemplateInfoIR template_info;
-  for (int i = 0; i < tl->size(); i++) {
-    const clang::TemplateArgument &arg = (*tl)[i];
-    // TODO: More comprehensive checking needed.
-    if (arg.getKind() != clang::TemplateArgument::Type) {
-      continue;
-    }
-    clang::QualType type = arg.getAsType();
-    template_info.AddTemplateElement(
-        repr::TemplateElementIR(
-            ast_caches_->GetTypeId(GetKeyForTypeId(type))));
-    if (!CreateBasicNamedAndTypedDecl(type, source_file)) {
-      llvm::errs() << "Setting up template arguments failed\n";
-      return false;
-    }
-  }
-  ta->SetTemplateInfo(std::move(template_info));
-  return true;
-}
-
 std::string ABIWrapper::QualTypeToString(const clang::QualType &sweet_qt) {
   const clang::QualType salty_qt = sweet_qt.getCanonicalType();
   // clang::TypeName::getFullyQualifiedName removes the part of the type related
@@ -427,6 +442,11 @@ std::string ABIWrapper::QualTypeToString(const clang::QualType &sweet_qt) {
       salty_qt, *ast_contextp_, ast_contextp_->getPrintingPolicy());
 }
 
+
+//------------------------------------------------------------------------------
+// Function Type Wrapper
+//------------------------------------------------------------------------------
+
 FunctionTypeWrapper::FunctionTypeWrapper(
     clang::MangleContext *mangle_contextp, clang::ASTContext *ast_contextp,
     const clang::CompilerInstance *compiler_instance_p,
@@ -469,6 +489,11 @@ bool FunctionTypeWrapper::GetFunctionType() {
       module_->AddLinkableMessage(*abi_decl);
 }
 
+
+//------------------------------------------------------------------------------
+// Function Decl Wrapper
+//------------------------------------------------------------------------------
+
 FunctionDeclWrapper::FunctionDeclWrapper(
     clang::MangleContext *mangle_contextp,
     clang::ASTContext *ast_contextp,
@@ -492,19 +517,6 @@ bool FunctionDeclWrapper::SetupThisParameter(repr::FunctionIR *functionp,
   return SetupFunctionParameter(functionp, this_type, false, source_file, true);
 }
 
-bool ABIWrapper::SetupFunctionParameter(
-    repr::CFunctionLikeIR *functionp, const clang::QualType qual_type,
-    bool has_default_arg, const std::string &source_file, bool is_this_ptr) {
-  if (!CreateBasicNamedAndTypedDecl(qual_type, source_file)) {
-    llvm::errs() << "Setting up function parameter failed\n";
-    return false;
-  }
-  functionp->AddParameter(repr::ParamIR(
-      ast_caches_->GetTypeId(GetKeyForTypeId(qual_type)), has_default_arg,
-      is_this_ptr));
-  return true;
-}
-
 bool FunctionDeclWrapper::SetupFunctionParameters(
     repr::FunctionIR *functionp,
     const std::string &source_file) {
@@ -574,6 +586,11 @@ std::unique_ptr<repr::FunctionIR> FunctionDeclWrapper::GetFunctionDecl() {
   return abi_decl;
 }
 
+
+//------------------------------------------------------------------------------
+// Record Decl Wrapper
+//------------------------------------------------------------------------------
+
 RecordDeclWrapper::RecordDeclWrapper(
     clang::MangleContext *mangle_contextp,
     clang::ASTContext *ast_contextp,
@@ -852,6 +869,21 @@ bool RecordDeclWrapper::GetRecordDecl() {
   return module_->AddLinkableMessage(*abi_decl);
 }
 
+std::string RecordDeclWrapper::GetMangledRTTI(
+    const clang::CXXRecordDecl *cxx_record_decl) {
+  clang::QualType qual_type =
+      cxx_record_decl->getTypeForDecl()->getCanonicalTypeInternal();
+  llvm::SmallString<256> uid;
+  llvm::raw_svector_ostream out(uid);
+  mangle_contextp_->mangleCXXRTTI(qual_type, out);
+  return uid.str();
+}
+
+
+//------------------------------------------------------------------------------
+// Enum Decl Wrapper
+//------------------------------------------------------------------------------
+
 EnumDeclWrapper::EnumDeclWrapper(
     clang::MangleContext *mangle_contextp,
     clang::ASTContext *ast_contextp,
@@ -904,6 +936,11 @@ bool EnumDeclWrapper::GetEnumDecl() {
   return module_->AddLinkableMessage(*abi_decl);
 }
 
+
+//------------------------------------------------------------------------------
+// Global Decl Wrapper
+//------------------------------------------------------------------------------
+
 GlobalVarDeclWrapper::GlobalVarDeclWrapper(
     clang::MangleContext *mangle_contextp,
     clang::ASTContext *ast_contextp,
diff --git a/vndk/tools/header-checker/src/dumper/abi_wrappers.h b/vndk/tools/header-checker/src/dumper/abi_wrappers.h
index 6d1844681..7337bce21 100644
--- a/vndk/tools/header-checker/src/dumper/abi_wrappers.h
+++ b/vndk/tools/header-checker/src/dumper/abi_wrappers.h
@@ -46,30 +46,36 @@ class ABIWrapper {
              repr::ModuleIR *module,
              ASTCaches *ast_caches);
 
+ public:
   static std::string GetDeclSourceFile(const clang::Decl *decl,
                                        const clang::CompilerInstance *cip);
 
-  static std::string GetMangledNameDecl(const clang::NamedDecl *decl,
-                                        clang::MangleContext *mangle_context);
-
  protected:
   std::string GetCachedDeclSourceFile(const clang::Decl *decl,
                                       const clang::CompilerInstance *cip);
 
-  std::string GetKeyForTypeId(clang::QualType qual_type);
-
-  std::string TypeNameWithFinalDestination(clang::QualType qual_type);
+ public:
+  static std::string GetMangledNameDecl(const clang::NamedDecl *decl,
+                                        clang::MangleContext *mangle_context);
 
+ protected:
+  // Shared between FunctionDeclWrapper and RecordDeclWrapper.
   bool SetupTemplateArguments(const clang::TemplateArgumentList *tl,
                               repr::TemplatedArtifactIR *ta,
                               const std::string &source_file);
 
+ protected:
+  // Shared between FunctionTypeWrapper and FunctionDeclWrapper.
   bool SetupFunctionParameter(repr::CFunctionLikeIR *functionp,
                               const clang::QualType qual_type,
                               bool has_default_arg,
                               const std::string &source_file,
                               bool is_this_parameter = false);
 
+ protected:
+  // Type-related functions
+  std::string GetKeyForTypeId(clang::QualType qual_type);
+
   std::string QualTypeToString(const clang::QualType &sweet_qt);
 
   std::string GetTagDeclQualifiedName(const clang::TagDecl *decl);
@@ -84,12 +90,15 @@ class ABIWrapper {
   bool CreateExtendedType(clang::QualType canonical_type,
                           repr::TypeIR *typep);
 
-  bool CreateAnonymousRecord(const clang::RecordDecl *decl);
+  std::string GetTypeUniqueId(const clang::TagDecl *tag_decl);
+
+ private:
+  std::string TypeNameWithFinalDestination(clang::QualType qual_type);
 
   TypeAndCreationStatus SetTypeKind(const clang::QualType qtype,
                                     const std::string &source_file);
 
-  std::string GetTypeUniqueId(const clang::TagDecl *tag_decl);
+  bool CreateAnonymousRecord(const clang::RecordDecl *decl);
 
  protected:
   const clang::CompilerInstance *cip_;

From f10fcc533ab20366cc4333f6241673ec8d976f80 Mon Sep 17 00:00:00 2001
From: Logan Chien <loganchien@google.com>
Date: Tue, 2 Apr 2019 07:36:56 +0800
Subject: [PATCH 3/7] header-checker: Remove indecipherable comment

Bug: 74764811
Test: ./tests/test.py
Change-Id: Ib1351092c7cd2c45aefc69291e558eed264a7cd8
---
 vndk/tools/header-checker/src/dumper/abi_wrappers.cpp | 2 --
 1 file changed, 2 deletions(-)

diff --git a/vndk/tools/header-checker/src/dumper/abi_wrappers.cpp b/vndk/tools/header-checker/src/dumper/abi_wrappers.cpp
index cd2ed70a5..14702a96b 100644
--- a/vndk/tools/header-checker/src/dumper/abi_wrappers.cpp
+++ b/vndk/tools/header-checker/src/dumper/abi_wrappers.cpp
@@ -303,8 +303,6 @@ bool ABIWrapper::CreateBasicNamedAndTypedDecl(
   typep->SetName(type_name_with_destination);
   typep->SetLinkerSetKey(type_name_with_destination);
 
-  // Default values are false, we don't set them since explicitly doing that
-  // makes the ABI dumps more verbose.
   // This type has a reference type if its a pointer / reference OR it has CVR
   // qualifiers.
   clang::QualType referenced_type = GetReferencedType(canonical_type);

From 565d361c1d29cdce629a3205f2472a0b6bf60fc0 Mon Sep 17 00:00:00 2001
From: Logan Chien <loganchien@google.com>
Date: Tue, 2 Apr 2019 12:47:03 +0800
Subject: [PATCH 4/7] header-checker: Remove unused variable and dead code

Bug: 74764811
Test: ./tests/test.py
Change-Id: I5b3e03d65283565866157502fb09a1191edc9ef8
---
 vndk/tools/header-checker/src/dumper/abi_wrappers.cpp | 9 ---------
 vndk/tools/header-checker/src/dumper/abi_wrappers.h   | 2 --
 2 files changed, 11 deletions(-)

diff --git a/vndk/tools/header-checker/src/dumper/abi_wrappers.cpp b/vndk/tools/header-checker/src/dumper/abi_wrappers.cpp
index 14702a96b..53adbf781 100644
--- a/vndk/tools/header-checker/src/dumper/abi_wrappers.cpp
+++ b/vndk/tools/header-checker/src/dumper/abi_wrappers.cpp
@@ -422,13 +422,6 @@ TypeAndCreationStatus ABIWrapper::SetTypeKind(
   return TypeAndCreationStatus(nullptr, false);
 }
 
-std::string ABIWrapper::GetTagDeclQualifiedName(const clang::TagDecl *decl) {
-  if (decl->getTypedefNameForAnonDecl()) {
-    return decl->getTypedefNameForAnonDecl()->getQualifiedNameAsString();
-  }
-  return decl->getQualifiedNameAsString();
-}
-
 std::string ABIWrapper::QualTypeToString(const clang::QualType &sweet_qt) {
   const clang::QualType salty_qt = sweet_qt.getCanonicalType();
   // clang::TypeName::getFullyQualifiedName removes the part of the type related
@@ -636,7 +629,6 @@ bool RecordDeclWrapper::SetupCXXBases(
   clang::CXXRecordDecl::base_class_const_iterator base_class =
       cxx_record_decl->bases_begin();
   while (base_class != cxx_record_decl->bases_end()) {
-    std::string name = QualTypeToString(base_class->getType());
     bool is_virtual = base_class->isVirtual();
     repr::AccessSpecifierIR access =
         AccessClangToIR(base_class->getAccessSpecifier());
@@ -908,7 +900,6 @@ bool EnumDeclWrapper::SetupEnumFields(repr::EnumTypeIR *enump) {
 
 bool EnumDeclWrapper::SetupEnum(repr::EnumTypeIR *enum_type,
                                 const std::string &source_file) {
-  std::string enum_name = GetTagDeclQualifiedName(enum_decl_);
   clang::QualType enum_qual_type =
       enum_decl_->getTypeForDecl()->getCanonicalTypeInternal();
   if (!CreateExtendedType(enum_qual_type, enum_type)) {
diff --git a/vndk/tools/header-checker/src/dumper/abi_wrappers.h b/vndk/tools/header-checker/src/dumper/abi_wrappers.h
index 7337bce21..799e4626f 100644
--- a/vndk/tools/header-checker/src/dumper/abi_wrappers.h
+++ b/vndk/tools/header-checker/src/dumper/abi_wrappers.h
@@ -78,8 +78,6 @@ class ABIWrapper {
 
   std::string QualTypeToString(const clang::QualType &sweet_qt);
 
-  std::string GetTagDeclQualifiedName(const clang::TagDecl *decl);
-
   bool CreateBasicNamedAndTypedDecl(clang::QualType,
                                     const std::string &source_file);
 

From 5eb6aeb9f508007cc542e5aceaa09a6f3e8a068d Mon Sep 17 00:00:00 2001
From: Logan Chien <loganchien@google.com>
Date: Tue, 2 Apr 2019 13:23:52 +0800
Subject: [PATCH 5/7] header-checker: Hide GetKeyForTypeId implementation

Bug: 74764811
Test: ./tests/test.py
Change-Id: Idcbc6e676d8d69274271ff23756be89e2414f2fd
---
 .../src/dumper/abi_wrappers.cpp               | 41 ++++++++-----------
 .../header-checker/src/dumper/abi_wrappers.h  |  8 ++--
 2 files changed, 22 insertions(+), 27 deletions(-)

diff --git a/vndk/tools/header-checker/src/dumper/abi_wrappers.cpp b/vndk/tools/header-checker/src/dumper/abi_wrappers.cpp
index 53adbf781..1692849a9 100644
--- a/vndk/tools/header-checker/src/dumper/abi_wrappers.cpp
+++ b/vndk/tools/header-checker/src/dumper/abi_wrappers.cpp
@@ -117,8 +117,7 @@ bool ABIWrapper::SetupTemplateArguments(const clang::TemplateArgumentList *tl,
     }
     clang::QualType type = arg.getAsType();
     template_info.AddTemplateElement(
-        repr::TemplateElementIR(
-            ast_caches_->GetTypeId(GetKeyForTypeId(type))));
+        repr::TemplateElementIR(GetTypeId(type)));
     if (!CreateBasicNamedAndTypedDecl(type, source_file)) {
       llvm::errs() << "Setting up template arguments failed\n";
       return false;
@@ -136,8 +135,7 @@ bool ABIWrapper::SetupFunctionParameter(
     return false;
   }
   functionp->AddParameter(repr::ParamIR(
-      ast_caches_->GetTypeId(GetKeyForTypeId(qual_type)), has_default_arg,
-      is_this_ptr));
+      GetTypeId(qual_type), has_default_arg, is_this_ptr));
   return true;
 }
 
@@ -236,6 +234,10 @@ std::string ABIWrapper::TypeNameWithFinalDestination(
   return qual_type_name;
 }
 
+std::string ABIWrapper::GetTypeId(clang::QualType qual_type) {
+  return ast_caches_->GetTypeId(GetKeyForTypeId(qual_type));
+}
+
 std::string ABIWrapper::GetKeyForTypeId(clang::QualType qual_type) {
   clang::QualType canonical_qual_type = qual_type.getCanonicalType();
   clang::QualType final_destination_type =
@@ -306,10 +308,9 @@ bool ABIWrapper::CreateBasicNamedAndTypedDecl(
   // This type has a reference type if its a pointer / reference OR it has CVR
   // qualifiers.
   clang::QualType referenced_type = GetReferencedType(canonical_type);
-  typep->SetReferencedType(
-      ast_caches_->GetTypeId(GetKeyForTypeId(referenced_type)));
+  typep->SetReferencedType(GetTypeId(referenced_type));
 
-  typep->SetSelfType(ast_caches_->GetTypeId(GetKeyForTypeId(canonical_type)));
+  typep->SetSelfType(GetTypeId(canonical_type));
 
   // Create the type for referenced type.
   return CreateBasicNamedAndTypedDecl(referenced_type, source_file);
@@ -451,8 +452,7 @@ FunctionTypeWrapper::FunctionTypeWrapper(
 bool FunctionTypeWrapper::SetupFunctionType(
     repr::FunctionTypeIR *function_type_ir) {
   // Add ReturnType
-  function_type_ir->SetReturnType(
-      ast_caches_->GetTypeId(GetKeyForTypeId(function_type_->getReturnType())));
+  function_type_ir->SetReturnType(GetTypeId(function_type_->getReturnType()));
   function_type_ir->SetSourceFile(source_file_);
   const clang::FunctionProtoType *function_pt =
       llvm::dyn_cast<clang::FunctionProtoType>(function_type_);
@@ -541,8 +541,7 @@ bool FunctionDeclWrapper::SetupFunction(repr::FunctionIR *functionp,
   functionp->SetSourceFile(source_file);
   clang::QualType return_type = function_decl_->getReturnType();
 
-  functionp->SetReturnType(
-      ast_caches_->GetTypeId(GetKeyForTypeId(return_type)));
+  functionp->SetReturnType(GetTypeId(return_type));
   functionp->SetAccess(AccessClangToIR(function_decl_->getAccess()));
   return CreateBasicNamedAndTypedDecl(return_type, source_file) &&
       SetupFunctionParameters(functionp, source_file) &&
@@ -600,11 +599,9 @@ bool RecordDeclWrapper::SetupRecordFields(repr::RecordTypeIR *recordp,
       ast_contextp_->getASTRecordLayout(record_decl_);
   while (field != record_decl_->field_end()) {
     clang::QualType field_type = field->getType();
-    std::string key_for_type_id = GetKeyForTypeId(field_type);
-    if (const clang::EnumDecl *enum_decl =
-               GetAnonymousEnum(field_type)) {
+    if (const clang::EnumDecl *enum_decl = GetAnonymousEnum(field_type)) {
       // Handle anonymous enums.
-      key_for_type_id = GetKeyForTypeId(enum_decl->getIntegerType());
+      field_type = enum_decl->getIntegerType();
     }
     if (!CreateBasicNamedAndTypedDecl(field_type, source_file)) {
       llvm::errs() << "Creation of Type failed\n";
@@ -613,7 +610,7 @@ bool RecordDeclWrapper::SetupRecordFields(repr::RecordTypeIR *recordp,
     std::string field_name = field->getName();
     uint64_t field_offset = record_layout.getFieldOffset(field_index);
     recordp->AddRecordField(repr::RecordFieldIR(
-        field_name, ast_caches_->GetTypeId(key_for_type_id), field_offset,
+        field_name, GetTypeId(field_type), field_offset,
         AccessClangToIR(field->getAccess())));
     field++;
     field_index++;
@@ -632,10 +629,8 @@ bool RecordDeclWrapper::SetupCXXBases(
     bool is_virtual = base_class->isVirtual();
     repr::AccessSpecifierIR access =
         AccessClangToIR(base_class->getAccessSpecifier());
-    cxxp->AddCXXBaseSpecifier(
-        repr::CXXBaseSpecifierIR(
-            ast_caches_->GetTypeId(GetKeyForTypeId(base_class->getType())),
-            is_virtual, access));
+    cxxp->AddCXXBaseSpecifier(repr::CXXBaseSpecifierIR(
+        GetTypeId(base_class->getType()), is_virtual, access));
     base_class++;
   }
   return true;
@@ -906,8 +901,7 @@ bool EnumDeclWrapper::SetupEnum(repr::EnumTypeIR *enum_type,
     return false;
   }
   enum_type->SetSourceFile(source_file);
-  enum_type->SetUnderlyingType(
-      ast_caches_->GetTypeId(GetKeyForTypeId(enum_decl_->getIntegerType())));
+  enum_type->SetUnderlyingType(GetTypeId(enum_decl_->getIntegerType()));
   enum_type->SetAccess(AccessClangToIR(enum_decl_->getAccess()));
   enum_type->SetUniqueId(GetTypeUniqueId(enum_decl_));
   return SetupEnumFields(enum_type) &&
@@ -953,8 +947,7 @@ bool GlobalVarDeclWrapper::SetupGlobalVar(repr::GlobalVarIR *global_varp,
   global_varp->SetName(global_var_decl_->getQualifiedNameAsString());
   global_varp->SetLinkerSetKey(mangled_name);
   global_varp->SetAccess(AccessClangToIR(global_var_decl_->getAccess()));
-  global_varp->SetReferencedType(
-      ast_caches_->GetTypeId(GetKeyForTypeId(global_var_decl_->getType())));
+  global_varp->SetReferencedType(GetTypeId(global_var_decl_->getType()));
   return true;
 }
 
diff --git a/vndk/tools/header-checker/src/dumper/abi_wrappers.h b/vndk/tools/header-checker/src/dumper/abi_wrappers.h
index 799e4626f..d97a9094b 100644
--- a/vndk/tools/header-checker/src/dumper/abi_wrappers.h
+++ b/vndk/tools/header-checker/src/dumper/abi_wrappers.h
@@ -74,9 +74,7 @@ class ABIWrapper {
 
  protected:
   // Type-related functions
-  std::string GetKeyForTypeId(clang::QualType qual_type);
-
-  std::string QualTypeToString(const clang::QualType &sweet_qt);
+  std::string GetTypeId(clang::QualType qual_type);
 
   bool CreateBasicNamedAndTypedDecl(clang::QualType,
                                     const std::string &source_file);
@@ -91,6 +89,10 @@ class ABIWrapper {
   std::string GetTypeUniqueId(const clang::TagDecl *tag_decl);
 
  private:
+  std::string QualTypeToString(const clang::QualType &sweet_qt);
+
+  std::string GetKeyForTypeId(clang::QualType qual_type);
+
   std::string TypeNameWithFinalDestination(clang::QualType qual_type);
 
   TypeAndCreationStatus SetTypeKind(const clang::QualType qtype,

From 89ebe73754e9186aaa98e8a3fabe93350d1e0ac1 Mon Sep 17 00:00:00 2001
From: Logan Chien <loganchien@google.com>
Date: Wed, 3 Apr 2019 05:28:36 +0800
Subject: [PATCH 6/7] header-checker: Re-use GetODRListMapKey

Bug: 74764811
Test: ./tests/test.py
Change-Id: Idb524a7e783c65aee08875bc1cec2481c06e880a
---
 vndk/tools/header-checker/src/repr/ir_representation.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/vndk/tools/header-checker/src/repr/ir_representation.cpp b/vndk/tools/header-checker/src/repr/ir_representation.cpp
index e755002de..50d1bf974 100644
--- a/vndk/tools/header-checker/src/repr/ir_representation.cpp
+++ b/vndk/tools/header-checker/src/repr/ir_representation.cpp
@@ -130,8 +130,8 @@ void ModuleIR::AddEnumType(EnumTypeIR &&enum_type) {
   }
   auto it = AddToMapAndTypeGraph(
       std::move(enum_type), &enum_types_, &type_graph_);
-  AddToODRListMap(it->second.GetUniqueId() + it->second.GetSourceFile(),
-                  (&it->second));
+  const std::string &key = GetODRListMapKey(&(it->second));
+  AddToODRListMap(key, (&it->second));
 }
 
 

From 068a6d1e7e9c5e0a76124612ba6e74bd5a86614e Mon Sep 17 00:00:00 2001
From: Logan Chien <loganchien@google.com>
Date: Wed, 3 Apr 2019 11:20:19 +0800
Subject: [PATCH 7/7] header-checker: Add file name to error messages

This commit adds file name to the error messages when IRReader cannot
read the input file.

Bug: 74764811
Test: ./tests/test.py
Change-Id: I3295f029d8195750da9f3cba18eacbde13e3371f
---
 vndk/tools/header-checker/src/diff/abi_diff.cpp | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/vndk/tools/header-checker/src/diff/abi_diff.cpp b/vndk/tools/header-checker/src/diff/abi_diff.cpp
index 0d2d759dc..c06654b68 100644
--- a/vndk/tools/header-checker/src/diff/abi_diff.cpp
+++ b/vndk/tools/header-checker/src/diff/abi_diff.cpp
@@ -32,13 +32,18 @@ namespace diff {
 repr::CompatibilityStatusIR HeaderAbiDiff::GenerateCompatibilityReport() {
   std::unique_ptr<repr::IRReader> old_reader =
       repr::IRReader::CreateIRReader(text_format_old_);
-  std::unique_ptr<repr::IRReader> new_reader =
-      repr::IRReader::CreateIRReader(text_format_new_);
-  if (!old_reader || !new_reader || !old_reader->ReadDump(old_dump_) ||
-      !new_reader->ReadDump(new_dump_)) {
-    llvm::errs() << "Could not create Text Format readers\n";
+  if (!old_reader || !old_reader->ReadDump(old_dump_)) {
+    llvm::errs() << "Failed to read old ABI dump: " << old_dump_ << "\n";
     ::exit(1);
   }
+
+  std::unique_ptr<repr::IRReader> new_reader =
+      repr::IRReader::CreateIRReader(text_format_new_);
+  if (!new_reader || !new_reader->ReadDump(new_dump_)) {
+    llvm::errs() << "Failed to read new ABI dump: " << new_dump_ << "\n";
+    ::exit(1);
+  }
+
   std::unique_ptr<repr::IRDiffDumper> ir_diff_dumper =
       repr::IRDiffDumper::CreateIRDiffDumper(text_format_diff_, cr_);
   repr::CompatibilityStatusIR status =