You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Tim Wilson-Brown (JIRA)" <ji...@apache.org> on 2010/04/02 06:32:29 UTC

[jira] Created: (THRIFT-750) C++ Compiler Virtual Function Option

C++ Compiler Virtual Function Option
------------------------------------

                 Key: THRIFT-750
                 URL: https://issues.apache.org/jira/browse/THRIFT-750
             Project: Thrift
          Issue Type: New Feature
          Components: Compiler (C++)
    Affects Versions: 0.2, 0.3
         Environment: Cygwin 1.7.1 on Windows XP SP3, Thrift 0.2.0 & r760184 & Trunk 
            Reporter: Tim Wilson-Brown
            Priority: Minor


The C++ Compiler currelty emits most functions in the *Client class as non-virtual.

This makes it impossible to subclass the generated class and override its functions.
A workaround is to inherit from the interface class *If, override the functions, and use them to call a *Client class member pointer.
But this can be cumbersome in some situations.

I propose to add a virtual option to the C++ compiler that emits function declarations as virtual.

I have attached a patched version of t_cpp_generator.cc from Thrift 0.2.0 - I can work out how to turn it into a patch file if needed.

Is this worth merging into the trunk?

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (THRIFT-750) C++ Compiler Virtual Function Option

Posted by "David Reiss (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-750?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12852699#action_12852699 ] 

David Reiss commented on THRIFT-750:
------------------------------------

You know that the ones inherited from the *If are automatically virtual, even though they are not explicitly declared as such, right?

> C++ Compiler Virtual Function Option
> ------------------------------------
>
>                 Key: THRIFT-750
>                 URL: https://issues.apache.org/jira/browse/THRIFT-750
>             Project: Thrift
>          Issue Type: New Feature
>          Components: Compiler (C++)
>    Affects Versions: 0.2, 0.3
>         Environment: Cygwin 1.7.1 on Windows XP SP3, Thrift 0.2.0 & r760184 & Trunk 
>            Reporter: Tim Wilson-Brown
>            Priority: Minor
>         Attachments: t_cpp_generator.cc
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> The C++ Compiler currelty emits most functions in the *Client class as non-virtual.
> This makes it impossible to subclass the generated class and override its functions.
> A workaround is to inherit from the interface class *If, override the functions, and use them to call a *Client class member pointer.
> But this can be cumbersome in some situations.
> I propose to add a virtual option to the C++ compiler that emits function declarations as virtual.
> I have attached a patched version of t_cpp_generator.cc from Thrift 0.2.0 - I can work out how to turn it into a patch file if needed.
> Is this worth merging into the trunk?

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (THRIFT-750) C++ Compiler Virtual Function Option

Posted by "Tim Wilson-Brown (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-750?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Tim Wilson-Brown updated THRIFT-750:
------------------------------------

    Attachment: cpp_virtual_thrift020.patch

Corrected patch against thrift-0.2.0
The original patch was broken

Please let me know if a patch against svn would be easier.

> C++ Compiler Virtual Function Option
> ------------------------------------
>
>                 Key: THRIFT-750
>                 URL: https://issues.apache.org/jira/browse/THRIFT-750
>             Project: Thrift
>          Issue Type: New Feature
>          Components: Compiler (C++)
>    Affects Versions: 0.2, 0.3
>         Environment: Cygwin 1.7.1 on Windows XP SP3, Thrift 0.2.0 & r760184 & Trunk 
>            Reporter: Tim Wilson-Brown
>            Priority: Minor
>         Attachments: cpp_virtual_thrift020.patch, t_cpp_generator.cc
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> The C++ Compiler currelty emits most functions in the *Client class as non-virtual.
> This makes it difficult to subclass the generated *Client class and override its functions.
> In particular, if a subclass overrides the *_send and *_recv functions, it must also override the function itself.
> Otherwise, the *Client version of the function calls the *Client versions of *_send and *_recv.
> A workaround is to inherit from the interface class *If, which has virtual functions, 
> and use them to call *Client class member functions.
> But this can be cumbersome in some situations, and still requires additional functions to be overridden.
> I propose to add a virtual option to the C++ compiler that emits function declarations as virtual.
> I have attached a patched version of t_cpp_generator.cc from Thrift 0.2.0 - I can work out how to turn it into a patch file if needed.
> Is this worth merging into the trunk?

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (THRIFT-750) C++ Compiler Virtual Function Option

Posted by "Tim Wilson-Brown (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-750?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Tim Wilson-Brown updated THRIFT-750:
------------------------------------

    Attachment: t_cpp_generator.cc

Attached patched t_cpp_generator.cc from Thrift 0.2.0

> C++ Compiler Virtual Function Option
> ------------------------------------
>
>                 Key: THRIFT-750
>                 URL: https://issues.apache.org/jira/browse/THRIFT-750
>             Project: Thrift
>          Issue Type: New Feature
>          Components: Compiler (C++)
>    Affects Versions: 0.2, 0.3
>         Environment: Cygwin 1.7.1 on Windows XP SP3, Thrift 0.2.0 & r760184 & Trunk 
>            Reporter: Tim Wilson-Brown
>            Priority: Minor
>         Attachments: t_cpp_generator.cc
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> The C++ Compiler currelty emits most functions in the *Client class as non-virtual.
> This makes it impossible to subclass the generated class and override its functions.
> A workaround is to inherit from the interface class *If, override the functions, and use them to call a *Client class member pointer.
> But this can be cumbersome in some situations.
> I propose to add a virtual option to the C++ compiler that emits function declarations as virtual.
> I have attached a patched version of t_cpp_generator.cc from Thrift 0.2.0 - I can work out how to turn it into a patch file if needed.
> Is this worth merging into the trunk?

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (THRIFT-750) C++ Compiler Virtual Function Option

Posted by "David Reiss (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-750?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12852720#action_12852720 ] 

David Reiss commented on THRIFT-750:
------------------------------------

Okay.  This seems acceptable.  There is a little bit of a performance hit, but it is not major.  Can you attach a patch?

> C++ Compiler Virtual Function Option
> ------------------------------------
>
>                 Key: THRIFT-750
>                 URL: https://issues.apache.org/jira/browse/THRIFT-750
>             Project: Thrift
>          Issue Type: New Feature
>          Components: Compiler (C++)
>    Affects Versions: 0.2, 0.3
>         Environment: Cygwin 1.7.1 on Windows XP SP3, Thrift 0.2.0 & r760184 & Trunk 
>            Reporter: Tim Wilson-Brown
>            Priority: Minor
>         Attachments: t_cpp_generator.cc
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> The C++ Compiler currelty emits most functions in the *Client class as non-virtual.
> This makes it difficult to subclass the generated *Client class and override its functions.
> In particular, if a subclass overrides the *_send and *_recv functions, it must also override the function itself.
> Otherwise, the *Client version of the function calls the *Client versions of *_send and *_recv.
> A workaround is to inherit from the interface class *If, which has virtual functions, 
> and use them to call *Client class member functions.
> But this can be cumbersome in some situations, and still requires additional functions to be overridden.
> I propose to add a virtual option to the C++ compiler that emits function declarations as virtual.
> I have attached a patched version of t_cpp_generator.cc from Thrift 0.2.0 - I can work out how to turn it into a patch file if needed.
> Is this worth merging into the trunk?

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (THRIFT-750) C++ Compiler Virtual Function Option

Posted by "Tim Wilson-Brown (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-750?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Tim Wilson-Brown updated THRIFT-750:
------------------------------------

    Description: 
The C++ Compiler currelty emits most functions in the *Client class as non-virtual.

This makes it difficult to subclass the generated *Client class and override its functions.

In particular, if a subclass overrides the *_send and *_recv functions, it must also override the function itself.
Otherwise, the *Client version of the function calls the *Client versions of *_send and *_recv.

A workaround is to inherit from the interface class *If, which has virtual functions, 
and use them to call *Client class member functions.
But this can be cumbersome in some situations, and still requires additional functions to be overridden.

I propose to add a virtual option to the C++ compiler that emits function declarations as virtual.

I have attached a patched version of t_cpp_generator.cc from Thrift 0.2.0 - I can work out how to turn it into a patch file if needed.

Is this worth merging into the trunk?

  was:
The C++ Compiler currelty emits most functions in the *Client class as non-virtual.

This makes it impossible to subclass the generated class and override its functions.
A workaround is to inherit from the interface class *If, override the functions, and use them to call a *Client class member pointer.
But this can be cumbersome in some situations.

I propose to add a virtual option to the C++ compiler that emits function declarations as virtual.

I have attached a patched version of t_cpp_generator.cc from Thrift 0.2.0 - I can work out how to turn it into a patch file if needed.

Is this worth merging into the trunk?


Yes, the *If functions are fine.

It's the member functions in the *Client class that aren't virtual, meaning that if the *_send or *_recv functions are overridden in a subclass, the original function needs to be overridden in the subclass to call the new versions.

I've tried to clarify the description.

> C++ Compiler Virtual Function Option
> ------------------------------------
>
>                 Key: THRIFT-750
>                 URL: https://issues.apache.org/jira/browse/THRIFT-750
>             Project: Thrift
>          Issue Type: New Feature
>          Components: Compiler (C++)
>    Affects Versions: 0.2, 0.3
>         Environment: Cygwin 1.7.1 on Windows XP SP3, Thrift 0.2.0 & r760184 & Trunk 
>            Reporter: Tim Wilson-Brown
>            Priority: Minor
>         Attachments: t_cpp_generator.cc
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> The C++ Compiler currelty emits most functions in the *Client class as non-virtual.
> This makes it difficult to subclass the generated *Client class and override its functions.
> In particular, if a subclass overrides the *_send and *_recv functions, it must also override the function itself.
> Otherwise, the *Client version of the function calls the *Client versions of *_send and *_recv.
> A workaround is to inherit from the interface class *If, which has virtual functions, 
> and use them to call *Client class member functions.
> But this can be cumbersome in some situations, and still requires additional functions to be overridden.
> I propose to add a virtual option to the C++ compiler that emits function declarations as virtual.
> I have attached a patched version of t_cpp_generator.cc from Thrift 0.2.0 - I can work out how to turn it into a patch file if needed.
> Is this worth merging into the trunk?

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (THRIFT-750) C++ Compiler Virtual Function Option

Posted by "Tim Wilson-Brown (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-750?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Tim Wilson-Brown updated THRIFT-750:
------------------------------------

    Attachment:     (was: cpp_virtual_thrift020.patch)

> C++ Compiler Virtual Function Option
> ------------------------------------
>
>                 Key: THRIFT-750
>                 URL: https://issues.apache.org/jira/browse/THRIFT-750
>             Project: Thrift
>          Issue Type: New Feature
>          Components: Compiler (C++)
>    Affects Versions: 0.2, 0.3
>         Environment: Cygwin 1.7.1 on Windows XP SP3, Thrift 0.2.0 & r760184 & Trunk 
>            Reporter: Tim Wilson-Brown
>            Priority: Minor
>         Attachments: t_cpp_generator.cc
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> The C++ Compiler currelty emits most functions in the *Client class as non-virtual.
> This makes it difficult to subclass the generated *Client class and override its functions.
> In particular, if a subclass overrides the *_send and *_recv functions, it must also override the function itself.
> Otherwise, the *Client version of the function calls the *Client versions of *_send and *_recv.
> A workaround is to inherit from the interface class *If, which has virtual functions, 
> and use them to call *Client class member functions.
> But this can be cumbersome in some situations, and still requires additional functions to be overridden.
> I propose to add a virtual option to the C++ compiler that emits function declarations as virtual.
> I have attached a patched version of t_cpp_generator.cc from Thrift 0.2.0 - I can work out how to turn it into a patch file if needed.
> Is this worth merging into the trunk?

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (THRIFT-750) C++ Compiler Virtual Function Option

Posted by "Tim Wilson-Brown (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/THRIFT-750?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Tim Wilson-Brown updated THRIFT-750:
------------------------------------

    Attachment: cpp_virtual_thrift020.patch

Patch for this issue against thrift-0.2.0.

I can provide a patch for HEAD if needed, but it may take me a while

> C++ Compiler Virtual Function Option
> ------------------------------------
>
>                 Key: THRIFT-750
>                 URL: https://issues.apache.org/jira/browse/THRIFT-750
>             Project: Thrift
>          Issue Type: New Feature
>          Components: Compiler (C++)
>    Affects Versions: 0.2, 0.3
>         Environment: Cygwin 1.7.1 on Windows XP SP3, Thrift 0.2.0 & r760184 & Trunk 
>            Reporter: Tim Wilson-Brown
>            Priority: Minor
>         Attachments: cpp_virtual_thrift020.patch, t_cpp_generator.cc
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> The C++ Compiler currelty emits most functions in the *Client class as non-virtual.
> This makes it difficult to subclass the generated *Client class and override its functions.
> In particular, if a subclass overrides the *_send and *_recv functions, it must also override the function itself.
> Otherwise, the *Client version of the function calls the *Client versions of *_send and *_recv.
> A workaround is to inherit from the interface class *If, which has virtual functions, 
> and use them to call *Client class member functions.
> But this can be cumbersome in some situations, and still requires additional functions to be overridden.
> I propose to add a virtual option to the C++ compiler that emits function declarations as virtual.
> I have attached a patched version of t_cpp_generator.cc from Thrift 0.2.0 - I can work out how to turn it into a patch file if needed.
> Is this worth merging into the trunk?

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Issue Comment Edited: (THRIFT-750) C++ Compiler Virtual Function Option

Posted by "Tim Wilson-Brown (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/THRIFT-750?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12852737#action_12852737 ] 

Tim Wilson-Brown edited comment on THRIFT-750 at 4/2/10 7:24 AM:
-----------------------------------------------------------------

Edit: this patch was broken

      was (Author: twilsonb):
    Patch for this issue against thrift-0.2.0.

I can provide a patch for HEAD if needed, but it may take me a while
  
> C++ Compiler Virtual Function Option
> ------------------------------------
>
>                 Key: THRIFT-750
>                 URL: https://issues.apache.org/jira/browse/THRIFT-750
>             Project: Thrift
>          Issue Type: New Feature
>          Components: Compiler (C++)
>    Affects Versions: 0.2, 0.3
>         Environment: Cygwin 1.7.1 on Windows XP SP3, Thrift 0.2.0 & r760184 & Trunk 
>            Reporter: Tim Wilson-Brown
>            Priority: Minor
>         Attachments: cpp_virtual_thrift020.patch, t_cpp_generator.cc
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> The C++ Compiler currelty emits most functions in the *Client class as non-virtual.
> This makes it difficult to subclass the generated *Client class and override its functions.
> In particular, if a subclass overrides the *_send and *_recv functions, it must also override the function itself.
> Otherwise, the *Client version of the function calls the *Client versions of *_send and *_recv.
> A workaround is to inherit from the interface class *If, which has virtual functions, 
> and use them to call *Client class member functions.
> But this can be cumbersome in some situations, and still requires additional functions to be overridden.
> I propose to add a virtual option to the C++ compiler that emits function declarations as virtual.
> I have attached a patched version of t_cpp_generator.cc from Thrift 0.2.0 - I can work out how to turn it into a patch file if needed.
> Is this worth merging into the trunk?

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.