[GitHub] [incubator-tvm] tqchen edited a comment on issue #4628: [Object] Add String container

[GitBox]

tqchen edited a comment on issue #4628: [Object] Add String container
URL: https://github.com/apache/incubator-tvm/pull/4628#issuecomment-584734563
 
 
   @wweic please update per comments :)


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] tqchen edited a comment on issue #4628: [Object] Add String container

[GitBox]

tqchen edited a comment on issue #4628: [Object] Add String container
URL: https://github.com/apache/incubator-tvm/pull/4628#issuecomment-583871162
 
 
   @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.
   
   
   
   
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] tqchen edited a comment on issue #4628: [Object] Add String container

[GitBox]

tqchen edited a comment on issue #4628: [Object] Add String container
URL: https://github.com/apache/incubator-tvm/pull/4628#issuecomment-583765522
 
 
   @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
   
   ```c++
   #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 
   #include 
   
   #if TVM_USE_CXX17_STRING_VIEW
   #include 
   #elif TVM_USE_CXX14_STRING_VIEW
   #include 
   #endif
   
   int main(int argc, char* argv[]) {
 std::string xyz = "xyz";
   
   #if TVM_USE_CXX17_STRING_VIEW
 LOG(INFO) << "C++17=" << std::hash()(xyz);
   #elif TVM_USE_CXX14_STRING_VIEW
 LOG(INFO) << "C++14=" << std::hash()(xyz);
   #else
 LOG(INFO) << "C++11=" << std::hash()(xyz);
   #endif
 return 0;
   }
   
   ```


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] [incubator-tvm] tqchen edited a comment on issue #4628: [Object] Add String container

[GitBox]

tqchen edited a comment on issue #4628: [Object] Add String container
URL: https://github.com/apache/incubator-tvm/pull/4628#issuecomment-583765522
 
 
   @wweic here is an code snippet that we can reuse for hash. We should 
consider migrate to require c++14, which will give us string view support
   
   ```c++
   #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 
   #include 
   
   #if TVM_USE_CXX17_STRING_VIEW
   #include 
   #elif TVM_USE_CXX14_STRING_VIEW
   #include 
   #endif
   
   int main(int argc, char* argv[]) {
 std::string xyz = "xyz";
   
   #if TVM_USE_CXX17_STRING_VIEW
 LOG(INFO) << "C++17=" << std::hash()(xyz);
   #elif TVM_USE_CXX14_STRING_VIEW
 LOG(INFO) << "C++14=" << std::hash()(xyz);
   #else
 LOG(INFO) << "C++11=" << std::hash()(xyz);
   #endif
 return 0;
   }
   
   ```


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services