You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Christian Lavoie (JIRA)" <ji...@apache.org> on 2010/11/08 21:56:08 UTC
[jira] Commented: (THRIFT-993) Some improvements in C++ stubs for
oneway operations
[ https://issues.apache.org/jira/browse/THRIFT-993?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12929729#action_12929729 ]
Christian Lavoie commented on THRIFT-993:
-----------------------------------------
Hey Rich,
There are a few problems with your patch:
# It doesn't apply in a clean client -- are you sure your client is up to date? Or are you trying to patch a released version? (It's much better if your stuff applies cleanly to a recent svn checkout)
# {{is_oneway}} is a simple {{const}} function -- any non-zero optimization level will optimize that away, there's no need to cache the result
# It's best to submit patches that clean code (like your oneway caching, though I don't think it's better with your change) separate from patches that fix bugs (like your oprot/seqid fix).
Could you please re-submit with a patch that satisfies all criteria?
> Some improvements in C++ stubs for oneway operations
> ----------------------------------------------------
>
> Key: THRIFT-993
> URL: https://issues.apache.org/jira/browse/THRIFT-993
> Project: Thrift
> Issue Type: Improvement
> Components: C++ - Compiler
> Reporter: Rich Salz
> Priority: Minor
> Attachments: diff.txt
>
> Original Estimate: 1h
> Remaining Estimate: 1h
>
> Oneway operations don't use the seqid or oprot parameters, so the generated stub should use unnamed parameters for those.
> While changing this I noticed that tfunction->is_oneway() is called many times, so cache that result in a local variable.
> While making that change, I noticed what seems to be an extraneous test for that, inside a loop which is already inside an if branch. I just commented that.
> So far, these changes make generated thrift stubs pass g++ -Wunused cleanly.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.