Remove Any::GetType() and rely on type name when comparing types

std::type_info::operator==() uses pointers to type names on some
implementations (e.g. gcc with __GXX_MERGED_TYPEINFO_NAMES defined).

This leads to problems such that types created in different translation
units could be treated as different even though they refer to the
same type.

Working with type_info directly seems unreliable and names of the
types should be used in comparisons directly.

Fixed the implementation of Any::IsTypeCompatible<T>() to use type
names and remove Any::GetType() to avoid any future problems for
client code to compare the type information manually.

Added Any::GetTypeName() and Any::GetUndecoratedTypeName() for
convenience.

BUG: 25132472
Change-Id: I8ba27a611c8edad2260dbed1c2f15633c8b3a3fe

5 files changed, 78 insertions, 34 deletions

diff --git a/brillo/any.cc b/brillo/any.cc
index 30e875b..d2313c5 100644
--- a/brillo/any.cc
+++ b/brillo/any.cc
@@ -34,7 +34,7 @@ Any& Any::operator=(Any&& rhs) {
 
 bool Any::operator==(const Any& rhs) const {
   // Make sure both objects contain data of the same type.
-  if (GetType() != rhs.GetType())
+  if (strcmp(GetTypeNameInternal(), rhs.GetTypeNameInternal()) != 0)
     return false;
 
   if (IsEmpty())
@@ -43,12 +43,11 @@ bool Any::operator==(const Any& rhs) const {
   return data_buffer_.GetDataPtr()->CompareEqual(rhs.data_buffer_.GetDataPtr());
 }
 
-const std::type_info& Any::GetType() const {
+const char* Any::GetTypeNameInternal() const {
   if (!IsEmpty())
-    return data_buffer_.GetDataPtr()->GetType();
+    return data_buffer_.GetDataPtr()->GetTypeName();
 
-  struct NullType {};  // Special helper type representing an empty variant.
-  return typeid(NullType);
+  return "";
 }
 
 void Any::Swap(Any& other) {
diff --git a/brillo/any.h b/brillo/any.h
index b872962..3cb2830 100644
--- a/brillo/any.h
+++ b/brillo/any.h
@@ -83,8 +83,8 @@ class BRILLO_EXPORT Any final {
     // to make sure the requested type matches the type of data actually stored,
     // so this "canonical" type is used for type checking below.
     using CanonicalDestType = typename std::decay<DestType>::type;
-    const std::type_info& ContainedTypeId = GetType();
-    if (typeid(CanonicalDestType) == ContainedTypeId)
+    const char* contained_type = GetTypeNameInternal();
+    if (strcmp(typeid(CanonicalDestType).name(), contained_type) == 0)
       return true;
 
     if (!std::is_pointer<CanonicalDestType>::value)
@@ -97,14 +97,18 @@ class BRILLO_EXPORT Any final {
     using NonPointer = typename std::remove_pointer<CanonicalDestType>::type;
     using CanonicalDestTypeNoConst = typename std::add_pointer<
         typename std::remove_const<NonPointer>::type>::type;
+    if (strcmp(typeid(CanonicalDestTypeNoConst).name(), contained_type) == 0)
+      return true;
+
     using CanonicalDestTypeNoVolatile = typename std::add_pointer<
         typename std::remove_volatile<NonPointer>::type>::type;
+    if (strcmp(typeid(CanonicalDestTypeNoVolatile).name(), contained_type) == 0)
+      return true;
+
     using CanonicalDestTypeNoConstOrVolatile = typename std::add_pointer<
         typename std::remove_cv<NonPointer>::type>::type;
-
-    return typeid(CanonicalDestTypeNoConst) == ContainedTypeId ||
-           typeid(CanonicalDestTypeNoVolatile) == ContainedTypeId ||
-           typeid(CanonicalDestTypeNoConstOrVolatile) == ContainedTypeId;
+    return strcmp(typeid(CanonicalDestTypeNoConstOrVolatile).name(),
+                  contained_type) == 0;
   }
 
   // Returns immutable data contained in Any.
@@ -113,8 +117,8 @@ class BRILLO_EXPORT Any final {
   template<typename T>
   const T& Get() const {
     CHECK(IsTypeCompatible<T>())
-        << "Requesting value of type '" << GetUndecoratedTypeName<T>()
-        << "' from variant containing '" << UndecorateTypeName(GetType().name())
+        << "Requesting value of type '" << brillo::GetUndecoratedTypeName<T>()
+        << "' from variant containing '" << GetUndecoratedTypeName()
         << "'";
     return data_buffer_.GetData<T>();
   }
@@ -158,9 +162,14 @@ class BRILLO_EXPORT Any final {
     return TryGet<T>(typename std::decay<T>::type());
   }
 
-  // Returns the type information about the contained data. For most cases,
-  // instead of using this function, you should be calling IsTypeCompatible<>().
-  const std::type_info& GetType() const;
+  // Returns the name of the type contained within Any. The string is a mangled
+  // type name returned by type_info::name(). For most cases, instead of using
+  // this function, you should be calling IsTypeCompatible<>().
+  inline std::string GetTypeName() const { return GetTypeNameInternal(); }
+  // Returns the undecorated name of the type contained within Any.
+  inline std::string GetUndecoratedTypeName() const {
+    return UndecorateTypeName(GetTypeNameInternal());
+  }
   // Swaps the value of this object with that of |other|.
   void Swap(Any& other);
   // Checks if Any is empty, that is, not containing a value of any type.
@@ -187,6 +196,10 @@ class BRILLO_EXPORT Any final {
   void AppendToDBusMessageWriter(dbus::MessageWriter* writer) const;
 
  private:
+  // Internal implementation of GetTypeName() which returns just a char* to
+  // static type name buffer.
+  const char* GetTypeNameInternal() const;
+
   // The data buffer for contained object.
   internal_details::Buffer data_buffer_;
 };
diff --git a/brillo/any_internal_impl.h b/brillo/any_internal_impl.h
index 932f0ee..fea0aac 100644
--- a/brillo/any_internal_impl.h
+++ b/brillo/any_internal_impl.h
@@ -142,8 +142,8 @@ class Buffer;  // Forward declaration of data buffer container.
 // Abstract base class for contained variant data.
 struct Data {
   virtual ~Data() {}
-  // Returns the type information for the contained data.
-  virtual const std::type_info& GetType() const = 0;
+  // Returns the type name for the contained data.
+  virtual const char* GetTypeName() const = 0;
   // Copies the contained data to the output |buffer|.
   virtual void CopyTo(Buffer* buffer) const = 0;
   // Moves the contained data to the output |buffer|.
@@ -165,7 +165,7 @@ struct TypedData : public Data {
   // NOLINTNEXTLINE(build/c++11)
   explicit TypedData(T&& value) : value_(std::move(value)) {}
 
-  const std::type_info& GetType() const override { return typeid(T); }
+  const char* GetTypeName() const override { return typeid(T).name(); }
   void CopyTo(Buffer* buffer) const override;
   void MoveTo(Buffer* buffer) override;
   bool IsConvertibleToInteger() const override {
@@ -268,7 +268,7 @@ class Buffer final {
     using Type = typename std::decay<T>::type;
     using DataType = TypedData<Type>;
     Data* ptr = GetDataPtr();
-    if (ptr && ptr->GetType() == typeid(Type)) {
+    if (ptr && strcmp(ptr->GetTypeName(), typeid(Type).name()) == 0) {
       // We assign the data to the variant container, which already
       // has the data of the same type. Do fast copy/move with no memory
       // reallocation.
diff --git a/brillo/any_internal_impl_unittest.cc b/brillo/any_internal_impl_unittest.cc
index e782cd0..5e64926 100644
--- a/brillo/any_internal_impl_unittest.cc
+++ b/brillo/any_internal_impl_unittest.cc
@@ -21,7 +21,7 @@ TEST(Buffer, Store_Int) {
   buffer.Assign(2);
   EXPECT_FALSE(buffer.IsEmpty());
   EXPECT_EQ(Buffer::kContained, buffer.storage_);
-  EXPECT_EQ(typeid(int), buffer.GetDataPtr()->GetType());
+  EXPECT_STREQ(typeid(int).name(), buffer.GetDataPtr()->GetTypeName());
 }
 
 TEST(Buffer, Store_Double) {
@@ -29,7 +29,7 @@ TEST(Buffer, Store_Double) {
   buffer.Assign(2.3);
   EXPECT_FALSE(buffer.IsEmpty());
   EXPECT_EQ(Buffer::kContained, buffer.storage_);
-  EXPECT_EQ(typeid(double), buffer.GetDataPtr()->GetType());
+  EXPECT_STREQ(typeid(double).name(), buffer.GetDataPtr()->GetTypeName());
 }
 
 TEST(Buffer, Store_Pointers) {
@@ -38,13 +38,14 @@ TEST(Buffer, Store_Pointers) {
   buffer.Assign(nullptr);
   EXPECT_FALSE(buffer.IsEmpty());
   EXPECT_EQ(Buffer::kContained, buffer.storage_);
-  EXPECT_EQ(typeid(std::nullptr_t), buffer.GetDataPtr()->GetType());
+  EXPECT_STREQ(typeid(std::nullptr_t).name(),
+               buffer.GetDataPtr()->GetTypeName());
 
   // char *
   buffer.Assign("abcd");
   EXPECT_FALSE(buffer.IsEmpty());
   EXPECT_EQ(Buffer::kContained, buffer.storage_);
-  EXPECT_EQ(typeid(const char*), buffer.GetDataPtr()->GetType());
+  EXPECT_STREQ(typeid(const char*).name(), buffer.GetDataPtr()->GetTypeName());
 
   // pointer to non-trivial object
   class NonTrivial {
@@ -54,7 +55,7 @@ TEST(Buffer, Store_Pointers) {
   buffer.Assign(&non_trivial);
   EXPECT_FALSE(buffer.IsEmpty());
   EXPECT_EQ(Buffer::kContained, buffer.storage_);
-  EXPECT_EQ(typeid(NonTrivial*), buffer.GetDataPtr()->GetType());
+  EXPECT_STREQ(typeid(NonTrivial*).name(), buffer.GetDataPtr()->GetTypeName());
 }
 
 TEST(Buffer, Store_NonTrivialObjects) {
@@ -66,7 +67,7 @@ TEST(Buffer, Store_NonTrivialObjects) {
   buffer.Assign(non_trivial);
   EXPECT_FALSE(buffer.IsEmpty());
   EXPECT_EQ(Buffer::kExternal, buffer.storage_);
-  EXPECT_EQ(typeid(NonTrivial), buffer.GetDataPtr()->GetType());
+  EXPECT_STREQ(typeid(NonTrivial).name(), buffer.GetDataPtr()->GetTypeName());
 }
 
 TEST(Buffer, Store_Objects) {
@@ -78,7 +79,7 @@ TEST(Buffer, Store_Objects) {
   buffer.Assign(small);
   EXPECT_FALSE(buffer.IsEmpty());
   EXPECT_EQ(Buffer::kContained, buffer.storage_);
-  EXPECT_EQ(typeid(Small), buffer.GetDataPtr()->GetType());
+  EXPECT_STREQ(typeid(Small).name(), buffer.GetDataPtr()->GetTypeName());
 
   struct Large {
     char c[10];
@@ -86,7 +87,7 @@ TEST(Buffer, Store_Objects) {
   buffer.Assign(large);
   EXPECT_FALSE(buffer.IsEmpty());
   EXPECT_EQ(Buffer::kExternal, buffer.storage_);
-  EXPECT_EQ(typeid(Large), buffer.GetDataPtr()->GetType());
+  EXPECT_STREQ(typeid(Large).name(), buffer.GetDataPtr()->GetTypeName());
 }
 
 TEST(Buffer, Copy) {
@@ -97,8 +98,8 @@ TEST(Buffer, Copy) {
   buffer1.CopyTo(&buffer2);
   EXPECT_FALSE(buffer1.IsEmpty());
   EXPECT_FALSE(buffer2.IsEmpty());
-  EXPECT_EQ(typeid(int), buffer1.GetDataPtr()->GetType());
-  EXPECT_EQ(typeid(int), buffer2.GetDataPtr()->GetType());
+  EXPECT_STREQ(typeid(int).name(), buffer1.GetDataPtr()->GetTypeName());
+  EXPECT_STREQ(typeid(int).name(), buffer2.GetDataPtr()->GetTypeName());
   EXPECT_EQ(30, buffer1.GetData<int>());
   EXPECT_EQ(30, buffer2.GetData<int>());
 
@@ -106,8 +107,8 @@ TEST(Buffer, Copy) {
   buffer1.CopyTo(&buffer2);
   EXPECT_FALSE(buffer1.IsEmpty());
   EXPECT_FALSE(buffer2.IsEmpty());
-  EXPECT_EQ(typeid(std::string), buffer1.GetDataPtr()->GetType());
-  EXPECT_EQ(typeid(std::string), buffer2.GetDataPtr()->GetType());
+  EXPECT_STREQ(typeid(std::string).name(), buffer1.GetDataPtr()->GetTypeName());
+  EXPECT_STREQ(typeid(std::string).name(), buffer2.GetDataPtr()->GetTypeName());
   EXPECT_EQ("abc", buffer1.GetData<std::string>());
   EXPECT_EQ("abc", buffer2.GetData<std::string>());
 }
@@ -127,7 +128,7 @@ TEST(Buffer, Move) {
   // the data and any retains the actual type.
   EXPECT_FALSE(buffer1.IsEmpty());
   EXPECT_FALSE(buffer2.IsEmpty());
-  EXPECT_EQ(typeid(int), buffer2.GetDataPtr()->GetType());
+  EXPECT_STREQ(typeid(int).name(), buffer2.GetDataPtr()->GetTypeName());
   EXPECT_EQ(30, buffer2.GetData<int>());
 
   buffer1.Assign(std::string("abc"));
@@ -136,6 +137,6 @@ TEST(Buffer, Move) {
   // This will make the source object effectively "Empty".
   EXPECT_TRUE(buffer1.IsEmpty());
   EXPECT_FALSE(buffer2.IsEmpty());
-  EXPECT_EQ(typeid(std::string), buffer2.GetDataPtr()->GetType());
+  EXPECT_STREQ(typeid(std::string).name(), buffer2.GetDataPtr()->GetTypeName());
   EXPECT_EQ("abc", buffer2.GetData<std::string>());
 }
diff --git a/brillo/any_unittest.cc b/brillo/any_unittest.cc
index 4fd23d7..506559b 100644
--- a/brillo/any_unittest.cc
+++ b/brillo/any_unittest.cc
@@ -304,3 +304,34 @@ TEST(Any, Compare_NonComparable) {
   EXPECT_NE(person1, person3);
   EXPECT_NE(person2, person3);
 }
+
+TEST(Any, GetTypeName) {
+  Any val;
+  EXPECT_TRUE(val.GetTypeName().empty());
+
+  val = 1;
+  EXPECT_EQ(typeid(int).name(), val.GetTypeName());
+
+  val = 3.1415926;
+  EXPECT_EQ(typeid(double).name(), val.GetTypeName());
+
+  val = std::string("blah");
+  EXPECT_EQ(typeid(std::string).name(), val.GetTypeName());
+}
+
+TEST(Any, GetUndecoratedTypeName) {
+  Any val;
+  EXPECT_TRUE(val.GetUndecoratedTypeName().empty());
+
+  val = 1;
+  EXPECT_EQ(brillo::GetUndecoratedTypeName<int>(),
+            val.GetUndecoratedTypeName());
+
+  val = 3.1415926;
+  EXPECT_EQ(brillo::GetUndecoratedTypeName<double>(),
+            val.GetUndecoratedTypeName());
+
+  val = std::string("blah");
+  EXPECT_EQ(brillo::GetUndecoratedTypeName<std::string>(),
+            val.GetUndecoratedTypeName());
+}