Address comments

include/tvm/runtime/container.h

@@ -285,6 +285,10 @@ class StringObj : public Object {
   /*! \brief The pointer to string data. */
   const char* data;
 
+  static constexpr const uint32_t _type_index = TypeIndex::kDynamic;
+  static constexpr const char* _type_key = "vm.String";
+  TVM_DECLARE_FINAL_OBJECT_INFO(StringObj, Object);
+
  private:
   /*! \brief String object which is moved from std::string container. */
   class FromStd;
@@ -310,14 +314,17 @@ class String : public ObjectRef {
    *
    * \param other The other string
    */
-  inline bool operator==(std::string other) const;
+  bool operator==(const std::string& other) const {
+    return size() == other.size() &&
+           other.compare(0, other.size(), get()->data, size()) == 0;
+  }
 
   /*!
    * \brief Compare is not equal to other std::string
    *
    * \param other The other string
    */
-  inline bool operator!=(std::string other) const;
+  bool operator!=(const std::string& other) const { return !operator==(other); }
 
   /*!
    * \brief Compare is equal to other char string
@@ -333,33 +340,79 @@ class String : public ObjectRef {
    */
   inline bool operator!=(const char* other) const;
 
+  /*!
+   * \brief Compares this to other for at most len chars
+   *
+   * \return zero if both char sequences compare equal. negative if this appear
+   * before other, positive otherwise.
+   */
+  int compare(const String& other, size_t len) const {
+    return compare(other.data(), len);
+  }
+
+  /*!
+   * \brief Compares this to other for at most len chars
+   *
+   * \return zero if both char sequences compare equal. negative if this appear
+   * before other, positive otherwise.
+   */
+  int compare(const std::string& other, size_t len) const {
+    return compare(other.data(), len);
+  }
+
+  /*!
+   * \brief Compares this to other for at most len chars
+   *
+   * \return zero if both char sequences compare equal. negative if this appear
+   * before other, positive otherwise.
+   */
+  int compare(const char* other, size_t len) const {
+    return std::strncmp(get()->data, other, len);
+  }
+
   /*!
    * \brief Returns a pointer to the char array in the string.
    *
    * \return const char*
    */
-  inline const char* c_str() const;
+  inline const char* c_str() const { return get()->data; }
+
+  /*!
+   * \brief Return the length of the string
+   *
+   * \return size_t string length
+   */
+  inline size_t size() const {
+    const auto* ptr = get();
+    if (ptr == nullptr) {
+      return 0;
+    }
+    return ptr->size;
+  }
 
   /*!
    * \brief Return the length of the string
    *
    * \return size_t string length
    */
-  inline size_t size() const;
+  inline size_t length() const { return size(); }
 
   /*!
    * \brief Return the data pointer
    *
    * \return const char* data pointer
    */
-  inline const char* data() const;
+  inline const char* data() const { return get()->data; }
+
+  /*! \return the internal StringObj pointer */
+  const StringObj* get() const { return operator->(); }
 
   /*!
    * \brief Convert String to an std::sting object
    *
    * \return std::string
    */
-  inline operator std::string() const;
+  operator std::string() const { return std::string{get()->data, size()}; }
 
   TVM_DEFINE_OBJECT_REF_METHODS(String, ObjectRef, StringObj);
 };
@@ -379,35 +432,25 @@ inline String::String(std::string other) {
   data_ = std::move(ptr);
 }
 
-inline bool String::operator==(std::string other) const {
-  return !operator!=(other);
-}
-
-inline bool String::operator!=(std::string other) const {
-  return operator->()->size != other.size() ||
-         std::strncmp(operator->()->data, other.data(), other.size()) != 0;
-}
-
 inline bool String::operator==(const char* other) const {
   return !operator!=(other);
 }
 
 inline bool String::operator!=(const char* other) const {
-  return operator->()->size != std::strlen(other) ||
-         std::strncmp(operator->()->data, other, operator->()->size) != 0;
-}
-
-inline const char* String::c_str() const { return operator->()->data; }
-
-inline size_t String::size() const { return operator->()->size; }
-
-inline const char* String::data() const { return operator->()->data; }
-
-inline String::operator std::string() const {
-  return std::string{operator->()->data, operator->()->size};
+  return size() != std::strlen(other) || compare(other, size()) != 0;
 }
 
 }  // namespace runtime
 }  // namespace tvm
 
+namespace std {
+
+template <>
+struct hash<::tvm::runtime::String> {
+  std::size_t operator()(const ::tvm::runtime::String& str) const {
+    return std::hash<std::string>()((std::string)str);
+  }
+};
+}  // namespace std
+
 #endif  // TVM_RUNTIME_CONTAINER_H_

tests/cpp/container_test.cc

@@ -248,8 +248,8 @@ TEST(String, CopyFromStd) {
 
 TEST(String, Comparisons) {
   using namespace std;
-  std::string source = "this is a string";
-  std::string mismatch = "different string";
+  std::string source = "a string";
+  std::string mismatch = "a string but longer";
   String s{source};
 
   CHECK_EQ(s == source, true);
@@ -268,6 +268,21 @@ TEST(String, c_str) {
   CHECK_NE(std::strcmp(s.c_str(), mismatch.data()), 0);
 }
 
+TEST(String, hash) {
+  using namespace std;
+  std::string source = "this is a string";
+  String s{source};
+  size_t hash = std::hash<String>()(s);
+}
+
+TEST(String, Cast) {
+  using namespace std;
+  std::string source = "this is a string";
+  String s{source};
+  ObjectRef r = s;
+  String s2 = Downcast<String>(r);
+}
+
 int main(int argc, char** argv) {
   testing::InitGoogleTest(&argc, argv);
   testing::FLAGS_gtest_death_test_style = "threadsafe";