[Object] Add String container

[wweic]

Discussion: https://discuss.tvm.ai/t/discuss-runtime-array-containers-array-adt-string/4582/38

cc @tqchen @icemelon9 @FrozenGene

[wweic]

@tqchen @FrozenGene Thanks for the comments. Please take a look again.

[tqchen]

we can do it in a followup PR but as a reminder, we need python side wrapper for String and perhaps we need to make some special treatment to make sure python could treat it as customized string types (e.g. by subclass string or related).

[tqchen]

gentle ping

[wweic]

@tqchen I'll send new revision soon.

[tqchen]

@wweic here is a code snippet that we can reuse for hash. We should consider migrate to require c++14, which will give us string view support

#define TVM_USE_CXX14_STRING_VIEW                                       \
  defined(__cpp_lib_experimental_string_view) && __cpp_lib_experimental_string_view >= 201411

#define TVM_USE_CXX17_STRING_VIEW                                       \
  defined(__cpp_lib_string_view) && __cpp_lib_string_view >= 201606

#include <string>
#include <dmlc/logging.h>

#if TVM_USE_CXX17_STRING_VIEW
#include <string_view>
#elif TVM_USE_CXX14_STRING_VIEW
#include <experimental/string_view>
#endif

int main(int argc, char* argv[]) {
  std::string xyz = "xyz";

#if TVM_USE_CXX17_STRING_VIEW
  LOG(INFO) << "C++17=" << std::hash<std::string_view>()(xyz);
#elif TVM_USE_CXX14_STRING_VIEW
  LOG(INFO) << "C++14=" << std::hash<std::experimental::string_view>()(xyz);
#else
  LOG(INFO) << "C++11=" << std::hash<std::string>()(xyz);
#endif
  return 0;
}

[wweic]

Thanks @tqchen! I have incorporated the snippet, please let me if I should put the macro in other places.

[FrozenGene]

IMO, it is bad idea to introduce std::experimental to this core part. As C++ standard says: The behavior of a C++ program is undefined if it adds declarations or definitions to namespace std or to a namespace within namespace std unless otherwise specified. That is to say std::experimental’s behavior is not guaranteed. Different version of compilers / compared with C++17 std, std::experimental maybe have different result too. I strongly suggest we remove std::experimental from this pr. Or we could consider implementing our string::view if we think it is very important.

[tqchen]

@FrozenGene In general I agree that we should avoid std::experimental.

In this particular case, i think the usage is fair, because it is guarded by marco tests and is only under a very limited case we a std::hash function that can hash a string without copying it(instead of using the string_view data structure).

• T0: We could have implemented a hash function by ourselves, but the hash itself may be inconsistent with the std version.
• T1: While the std::experimental::string_view's hash could have been inconsistent with the std::string version as per compiler version(because of experimental), in practice it is consistent with std::string as per string_view proposal(and can be confirmed using different compilers). More importantly, it is also fine if the hash is inconsistent with the std ver(then we will be the case of T1.

Given the above consideration, I think it is fine to permit the limited usecase. However, I agree that we should have a more careful documentation about the std::experimental use case here and only limit it to the specific usecase.

[tqchen]

@FrozenGene @icemelon9 please help to take another look. The standard lib is the core part of the system so we need extra caution in terms of reviewing. Thanks @wweic for pushing it through.

@zhiics @Hzfengsy @jroesch @ZihengJiang @yzhliu please also help to take a look if you have time.

[yzhliu]

good to me.

[zhiics]

LGTM, please rebase.

[FrozenGene]

LGTM

[wweic]

CI has passed after retry 2 times.

[tqchen]

Spot another potential problem, added comments about more test coverage

[tqchen]

ping @wweic :)

[wweic]

@tqchen sorry for the delay. Was adjusting working from china schedules. I have addressed your comments.

[tqchen]

Thanks @wweic for all the patience to polish this core data structure.
Thanks @FrozenGene @zhiics @icemelon9 for reviews.

This PR is now merged. Perhaps we can move on to add python side wrappers to the String container. And followup with the final container -- Array.

[wweic]

@tqchen Yes. I'll explore Python interface for String.