You are viewing a plain text version of this content. The canonical link for it is here.
Posted to proton@qpid.apache.org by "Cliff Jansen (JIRA)" <ji...@apache.org> on 2012/11/26 10:16:58 UTC
[jira] [Created] (PROTON-159) port proton to C++
Cliff Jansen created PROTON-159:
-----------------------------------
Summary: port proton to C++
Key: PROTON-159
URL: https://issues.apache.org/jira/browse/PROTON-159
Project: Qpid Proton
Issue Type: New Feature
Components: proton-c
Affects Versions: 0.3
Reporter: Cliff Jansen
Make code compile in both C99 and C++, using the gnu toolchain. This is a necessary first step
towards a Microsoft Visual Studio port (where the compiler supports C++ but not C99).
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (PROTON-159) port proton to C++
Posted by "Rafael H. Schloming (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PROTON-159?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13509068#comment-13509068 ]
Rafael H. Schloming commented on PROTON-159:
--------------------------------------------
So looking at this patch I'm a bit overwhelmed by how huge it is. I don't think it's feasible to actually review it and then apply it because I think in the time it would take to actually review it, trunk will move far enough that it won't apply anymore. For example since this patch was created, I've checked in changes to codec.c which I'm guessing will conflict since there are 76 diffs against it. Given that this patch combines both semantic changes (like the VLA stuff) with mechanical ones, I also don't think it's a very good idea to short-circuit the review process.
Why don't we break out the build system changes into a separate patch and get that in as a first step? I realize this will result in the BUILD_WITH_CXX=On version of the build spewing out lots of errors, but we should then be able to pull in patches that incrementally reduce the errors. I'd suggest maybe a second patch limited only to the mechanical code changes and then subsequent patches addressing the semantic changes, with that pattern possibly repeated on a per-file basis as dictated by the build order.
Other than the comment on the overall size and approach to getting these changes in, I did a random spot check on a few of the files to see the general shape of these changes. I'm a little concerned with the apparent randomness of some of the changes, e.g. changing loop counters from int to size_t or changing declarations to unsigned. It's not at all obvious to someone working with the C build why things like that would be necessary. I don't want to create a situation where you pretty much always need to build every change against both the C and C++ compiler in order to know if it is going to break the larger build.
> port proton to C++
> ------------------
>
> Key: PROTON-159
> URL: https://issues.apache.org/jira/browse/PROTON-159
> Project: Qpid Proton
> Issue Type: New Feature
> Components: proton-c
> Affects Versions: 0.3
> Reporter: Cliff Jansen
> Attachments: proton-159-0.diff, proton-159-0-partial.diff
>
>
> Make code compile in both C99 and C++, using the gnu toolchain. This is a necessary first step
> towards a Microsoft Visual Studio port (where the compiler supports C++ but not C99).
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (PROTON-159) port proton to C++
Posted by "Cliff Jansen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PROTON-159?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Cliff Jansen updated PROTON-159:
--------------------------------
Attachment: proton-159-0.diff
To test drive, apply the patch and
cmake -DBUILD_WITH_CXX=ON [ plus your regularly scheduled cmake args]
The patch intent was
minimal change to the generated C99 code
minimal change to the original source code
VLA repurposing as per Rafi's comments in PROTON-57
The issues encountered varied from boring to mildly challenging
explicit cast fussiness
const fussiness
signed/unsigned/size_t comparison fussiness
aggregate assignments
CMake magic dust
%z, and PRIxxx in printf
VLAs
redefinition of pn_dtag (ick, an interface in engine.h)
redefinition of PN_ENSURE for type cast fussiness yet again
The VLAs were replaced depending on context:
malloc if perf not an issue and free obvious
other-wise stack if normal case fits, and heap if too big (to avoid perf hit)
stack only with fail check (if failure cases very rare)
PN_ENSURE if re-useable scratch space makes most sense
PRIxxx was handled by a new sys/io.h header file
%z was handled by the introduction on pn_fprintf. For g++, just hide the %z via the intermediate function call. Presumably Microsoft Visual Studio will require a substituted format string with their size_t equivalent.
This patch is strongly related to information provided or discussed in PROTON-57, PROTON-67, PROTON-98, PROTON-99, PROTON-148.
> port proton to C++
> ------------------
>
> Key: PROTON-159
> URL: https://issues.apache.org/jira/browse/PROTON-159
> Project: Qpid Proton
> Issue Type: New Feature
> Components: proton-c
> Affects Versions: 0.3
> Reporter: Cliff Jansen
> Attachments: proton-159-0.diff
>
>
> Make code compile in both C99 and C++, using the gnu toolchain. This is a necessary first step
> towards a Microsoft Visual Studio port (where the compiler supports C++ but not C99).
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (PROTON-159) port proton to C++
Posted by "Cliff Jansen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PROTON-159?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Cliff Jansen updated PROTON-159:
--------------------------------
Attachment: proton-159-0-partial.diff
proton-159-0-partial.diff is the same patch minus many boiler plate casts of the form:
foo_type *afoo = /* (foo_type *) */ malloc(n * sizeof(foo_type));
or
func(/* (bar_enum_type_t) */ 0);
It won't compile on its own, but is a bit eaier to read.
> port proton to C++
> ------------------
>
> Key: PROTON-159
> URL: https://issues.apache.org/jira/browse/PROTON-159
> Project: Qpid Proton
> Issue Type: New Feature
> Components: proton-c
> Affects Versions: 0.3
> Reporter: Cliff Jansen
> Attachments: proton-159-0.diff, proton-159-0-partial.diff
>
>
> Make code compile in both C99 and C++, using the gnu toolchain. This is a necessary first step
> towards a Microsoft Visual Studio port (where the compiler supports C++ but not C99).
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Comment Edited] (PROTON-159) port proton to C++
Posted by "Andrew Stitcher (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PROTON-159?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13503990#comment-13503990 ]
Andrew Stitcher edited comment on PROTON-159 at 11/26/12 7:08 PM:
------------------------------------------------------------------
In order to avoid the nuisance (unreadable) malloc casts why not introduce a macro (I think this might be one of the few places where such might be really justified).
something like:
#define NEW(t) ((t*) malloc(sizeof(t)))
or
#define NEWARRAY(t, n) ((t*) malloc((n)*sizeof(t))
or perhaps to be insufferable:
#ifdef __cplusplus__
#define NEW(t) new t
#define FREE(x) delete x
#else
#define NEW malloc(sizeof(t))
#define FREE(x) free(x)
#endif
I think it's just possible that using these (latter) macros and having no casts might allow the compiler to give diagnostics that it couldn't if it had casts which might be a good thing in the longer run.
was (Author: astitcher):
In order to avoid the nuisance (unreadable) malloc casts why not introduce a macro (I think this might be one of the few places where such might be really justified).
something like:
#define NEW(t) ((t*) malloc(sizeof(t)))
or
#define NEWARRAY(t, n) ((t*) malloc((n)*sizeof(t))
or perhaps to be insufferable:
#ifdef __cplusplus__
#define NEW(t) new t
#define FREE(x) delete x
#else
#define NEW malloc(sizeof(t))
#define FREE(x) free(x)
#endif
I think it's just possible that using these (latter) macros and having no casts might allow the compiler to give diagnostics that it couldn't before which might be a good thing.
> port proton to C++
> ------------------
>
> Key: PROTON-159
> URL: https://issues.apache.org/jira/browse/PROTON-159
> Project: Qpid Proton
> Issue Type: New Feature
> Components: proton-c
> Affects Versions: 0.3
> Reporter: Cliff Jansen
> Attachments: proton-159-0.diff, proton-159-0-partial.diff
>
>
> Make code compile in both C99 and C++, using the gnu toolchain. This is a necessary first step
> towards a Microsoft Visual Studio port (where the compiler supports C++ but not C99).
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (PROTON-159) port proton to C++
Posted by "Cliff Jansen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PROTON-159?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13504062#comment-13504062 ]
Cliff Jansen commented on PROTON-159:
-------------------------------------
Note that there are just as many realloc() calls to deal with, including indirectly via PN_ENSURE() in util.h. Is the benefit of embracing compiler support of checking types worth extending the macros to this and calloc()? I'm not sure there is an easy way to do this without pulling in std::vector as well, which feels heavy handed.
If you like the new PN_ENSURE in the patch, which associates the correct type with the realloc, then a middle ground could be to just replace all uses of malloc and realloc with something that looks like PN_ENSURE (and calloc with PN_ENSUREZ) without the power of two stuff.
At the back of my mind, I was trying to leave the door open for a platform to be able to compile a pure C program using the C++ compiler, provided there were compiler/linker options to e.g. turn off exception handling or exclude the C++ runtime. I confess I am far from sure this is remotely feasible on any platform, but itroducing C++ syntax or C++ Standard Library containers is moving in the opposite direction.
In any event, the number of changes are not huge so weigh in on your favorite route and I will put it in place.
> port proton to C++
> ------------------
>
> Key: PROTON-159
> URL: https://issues.apache.org/jira/browse/PROTON-159
> Project: Qpid Proton
> Issue Type: New Feature
> Components: proton-c
> Affects Versions: 0.3
> Reporter: Cliff Jansen
> Attachments: proton-159-0.diff, proton-159-0-partial.diff
>
>
> Make code compile in both C99 and C++, using the gnu toolchain. This is a necessary first step
> towards a Microsoft Visual Studio port (where the compiler supports C++ but not C99).
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (PROTON-159) port proton to C++
Posted by "Andrew Stitcher (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PROTON-159?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13509074#comment-13509074 ]
Andrew Stitcher commented on PROTON-159:
----------------------------------------
I think maybe we could break this down into a smaller number of changes, each with a single "theme".
For instance changing all the memory allocation into macros/static inlines would be one; fixing up the vlas might be another; the struct initialiseers another; the char* -> const char* changes might be another etc.
> port proton to C++
> ------------------
>
> Key: PROTON-159
> URL: https://issues.apache.org/jira/browse/PROTON-159
> Project: Qpid Proton
> Issue Type: New Feature
> Components: proton-c
> Affects Versions: 0.3
> Reporter: Cliff Jansen
> Attachments: proton-159-0.diff, proton-159-0-partial.diff
>
>
> Make code compile in both C99 and C++, using the gnu toolchain. This is a necessary first step
> towards a Microsoft Visual Studio port (where the compiler supports C++ but not C99).
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (PROTON-159) port proton to C++
Posted by "Andrew Stitcher (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PROTON-159?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13503990#comment-13503990 ]
Andrew Stitcher commented on PROTON-159:
----------------------------------------
In order to avoid the nuisance (unreadable) malloc casts why not introduce a macro (I think this might be one of the few places where such might be really justified).
something like:
#define NEW(t) ((t*) malloc(sizeof(t)))
or
#define NEWARRAY(t, n) ((t*) malloc((n)*sizeof(t))
or perhaps to be insufferable:
#ifdef __cplusplus__
#define NEW(t) new t
#define FREE(x) delete x
#else
#define NEW malloc(sizeof(t))
#define FREE(x) free(x)
#endif
I think it's just possible that using these (latter) macros and having no casts might allow the compiler to give diagnostics that it couldn't before which might be a good thing.
> port proton to C++
> ------------------
>
> Key: PROTON-159
> URL: https://issues.apache.org/jira/browse/PROTON-159
> Project: Qpid Proton
> Issue Type: New Feature
> Components: proton-c
> Affects Versions: 0.3
> Reporter: Cliff Jansen
> Attachments: proton-159-0.diff, proton-159-0-partial.diff
>
>
> Make code compile in both C99 and C++, using the gnu toolchain. This is a necessary first step
> towards a Microsoft Visual Studio port (where the compiler supports C++ but not C99).
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira