You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by "infdahai (via GitHub)" <gi...@apache.org> on 2023/05/21 15:37:41 UTC

[GitHub] [incubator-kvrocks] infdahai opened a new issue, #1462: avoid the implicit conversion from `const char *` to `string` in `Status.h`

infdahai opened a new issue, #1462:
URL: https://github.com/apache/incubator-kvrocks/issues/1462

   ### Search before asking
   
   - [X] I had searched in the [issues](https://github.com/apache/incubator-kvrocks/issues) and found no similar issues.
   
   
   ### Motivation
   
   https://github.com/apache/incubator-kvrocks/blob/fb0e3d42031a6e0691c7a736711d0a01d72bdb1a/src/common/status.h#L75
   
   
   https://github.com/apache/incubator-kvrocks/blob/fb0e3d42031a6e0691c7a736711d0a01d72bdb1a/src/common/status.h#L101-L126
   
   Maybe we call the implicit copy currently and the implicit conversion from `const char *` to `string` in `Status.h` can confuse the compiler and the editor because we usually invoke `{NotOK, str(type: const char*)}` as the return String.
   
   ### Solution
   
   We can use the `Slice` type to replace the string type of  `msg` and do `Slice(const char*)` funcs by referring to `leveldb`.
   
   ### Are you willing to submit a PR?
   
   - [X] I'm willing to submit a PR!


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] infdahai commented on issue #1462: avoid the implicit conversion from `const char *` to `string` in `Status.h`

Posted by "infdahai (via GitHub)" <gi...@apache.org>.
infdahai commented on issue #1462:
URL: https://github.com/apache/incubator-kvrocks/issues/1462#issuecomment-1556213164

   OK,I get it. Should we solve the implicit conversion?


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] infdahai commented on issue #1462: avoid the implicit conversion from `const char *` to `string` in `Status.h`

Posted by "infdahai (via GitHub)" <gi...@apache.org>.
infdahai commented on issue #1462:
URL: https://github.com/apache/incubator-kvrocks/issues/1462#issuecomment-1556215197

   Ok, thanks for your answer.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] PragmaTwice commented on issue #1462: avoid the implicit conversion from `const char *` to `string` in `Status.h`

Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on issue #1462:
URL: https://github.com/apache/incubator-kvrocks/issues/1462#issuecomment-1556211361

   `Slice` is just a **view**, which has no **ownership**.
   For example, if we want to concat some string dynamicly, we cannot use `Slice` since it cannot extend lifetime of the object, e.g.
   ```
   string x = to_string(count) + " apples";
   return Slice(x); // dangling reference!
   ```
   
   BTW `Slice` is not a well-designed type, instead we have `std::string_view` since c++17.
   


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] infdahai commented on issue #1462: avoid the implicit conversion from `const char *` to `string` in `Status.h`

Posted by "infdahai (via GitHub)" <gi...@apache.org>.
infdahai commented on issue #1462:
URL: https://github.com/apache/incubator-kvrocks/issues/1462#issuecomment-1556212576

   >string x = to_string(count) + " apples";
   return Slice(x); // dangling reference!
   
   Something I'm wondering about is that I don't know what happens in kvrocks.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] infdahai closed issue #1462: avoid the implicit conversion from `const char *` to `string` in `Status.h`

Posted by "infdahai (via GitHub)" <gi...@apache.org>.
infdahai closed issue #1462: avoid the implicit conversion from `const char *` to `string` in `Status.h`
URL: https://github.com/apache/incubator-kvrocks/issues/1462


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] PragmaTwice commented on issue #1462: avoid the implicit conversion from `const char *` to `string` in `Status.h`

Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on issue #1462:
URL: https://github.com/apache/incubator-kvrocks/issues/1462#issuecomment-1556214530

   I think nope. It is by design.
   
   Refer to [the 5th constructor of std::basic_string](https://en.cppreference.com/w/cpp/string/basic_string/basic_string).
   
   BTW `{NotOK, "something"}` is more intuitive than `{NotOK, string("something")}`.


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] PragmaTwice commented on issue #1462: avoid the implicit conversion from `const char *` to `string` in `Status.h`

Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on issue #1462:
URL: https://github.com/apache/incubator-kvrocks/issues/1462#issuecomment-1556211451

   You can close it if no further questions :)


-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org