[ 
https://issues.apache.org/jira/browse/THRIFT-5497?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17476958#comment-17476958
 ] 

Richard H. Gumpertz edited comment on THRIFT-5497 at 1/17/22, 5:40 AM:
-----------------------------------------------------------------------

I think I have a solution that shouldn't break anything: create a wrapper for 
*std::string* called "{*}::apache::thrift::binary_string{*}" that basically 
passes everything through to std::string.  In template resolution, however, it 
will be distinguishable from std::string.   The thrift compiler for C++ will 
then use {*}"::apache::thrift::binary_string{*}" instead of "{*}std::string{*}" 
when generating binary fields,

I love it when a plan comes together ;).  No thrift option name will be needed 
and it should do everything that I and others want.

Attached is my working definition: [^binary_string.h] – I would very much 
appreciate receiving feedback!  (I know that many of the source lines are a bit 
long for some people's taste; that can be resolved later.)  The 
/{*}*{*}constexpr*/ comments indicate where constexpr _might_ be added for 
C++20.

 


was (Author: JIRAUSER283319):
I think I have a solution that shouldn't break anything: create a wrapper for 
*std::string* called "{*}::apache::thrift::binary_string{*}" that basically 
passes everything through to std::string.  In template resolution, however, it 
will be distinguishable from std::string.   The thrift compiler for C++ will 
then use {*}"::apache::thrift::binary_string{*}" instead of "{*}std::string{*}" 
when generating binary fields,

I love it when a plan comes together ;).  No thrift option name will be needed 
and it should do everything that I and others want.

Attached is my working definition: [^binary_string.h] – I would very much 
appreciate receiving feedback!  (I know that many of the source lines are a bit 
long for some people's taste; that can be resolved later.)  The 
/*{*}constexpr*{*}/ comments indicate where constexpr _might_ be added for 
C++20.

 

> Change the C++ representation of binary be std::basic_string<unsigned_char>
> ---------------------------------------------------------------------------
>
>                 Key: THRIFT-5497
>                 URL: https://issues.apache.org/jira/browse/THRIFT-5497
>             Project: Thrift
>          Issue Type: Improvement
>            Reporter: Richard H. Gumpertz
>            Priority: Minor
>         Attachments: binary_string.h
>
>
> I would like to change the C++ representation of binary from "std::string" to 
> "std::basic_string<unsigned_char>".  This would allow C++ templates to 
> distinguish the Thrift type "string" from "binary".
> If nothing else, ::apached::thrift::to_string could print binary differently 
> than string.  Currently, it can get pretty ugly when one calls printTo on a 
> struct containing a binary field that has data that is not readable!
> There is an issue as to how they should be displayed by default; using 
> \xDE\xAD\xBE\xEF with a \ before every byte seems ugly.  Alternativlely, 
> "DEADBEEF"_x because it could also be used as input if the C++ operator""_x 
> is defined by the user?  Alternatively just <binary> or <27-byte binary> 
> (mirroring <null> for unset fields), hiding the contents?  Perhaps leave 
> to_string(std::basic_string<unsigned char> &value) undefined and force the 
> user to define it if needed?  Thoughts & suggestions???
> Given that such a change could break existing code, it should probably be 
> enabled under a new thrift compiler option to --gen cpp.  I haven't come up 
> with a good name for such an option yet: --gen cpp:binary_as_unsigned_string 
> is explicit but wordy.  Any suggestions???
> I would be willing to do the work necessary to add the relevant code if there 
> are no volunteers who have a better understanding of precisely what code 
> would have to be updated (or at least help me find such code)



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

Reply via email to