Contribution: C++Builder Compiling

156 views
Skip to first unread message

Scott Saad

unread,
Sep 20, 2011, 2:33:58 PM9/20/11
to prot...@googlegroups.com
Hi folks,

I'm looking at using Protocol Buffers in a project where Embarcadero's (aka CodeGear, aka Borland) compiler, C++ Builder is being used. 

So far I've pulled down the read-only version of the source. I'm actively making some minor changes while building project files for the IDE. 

If I am able to get things compiling and tests running would it be possible to get a code review and potential merge these contributions in?

Thanks in advance!
Scott Saad

Scott Saad

unread,
Oct 11, 2011, 11:32:31 AM10/11/11
to prot...@googlegroups.com
A quick status update on this effort:
  • I forked the source at GitHub under the protobuf-cppbuilder project.
  • The biggest overarching changes revolved around namespaces. These changes were minimal but touched many files.
    • Main cause of this change was a "using namespace std;" statement in common.h. I removed this but then had to fully qualify std types when there were used (i.e. string became std::string).
    • C++Builder sometimes had problems resolving namespaces. For example if we were in google::protobuf and something was referenced in google::protobuf::internal, I had to qualify that by adding internal:: to whatever was being referenced. 
  • Other changes revolved around #ifdef/#ifndef certain includes, etc. (similar to MSVC).
  • A very interesting compiler complaint that took me a while to track down was how dependent names were being sometimes used/declared. When they referred to types and templates I had to disambiguate. This cleaned things up and made the source cleaning compile across all platforms.
  • The libraries are compiling under C++Builder and can be linked against.
  • protoc is not fully working. It compiles fine but gets a runtime error when parsing options. I'm not yet sure what's going on here. A workaround is to not use the protoc that gets compiled with this project, but use the distributed binary to generate the C++ protocol buffers. Hoping I can make some headway on this issue soon.
  • I've compiled all changes on both gcc and MSVC. All unit tests are passing.
In my opinion these changes would be great to have merged back into the protobuf proper branch as most of them aim to make the code base more cross compiler compliant. I suppose the C++Builder is a bit more strict on a few items. 

I would be happy to produce is diff/patch to be looked at by a main contributor on the google project. Please just let me know what I can do to help facilitate this.

Thank you,
Scott Saad

Josh Kelley

unread,
Oct 12, 2011, 5:03:51 PM10/12/11
to Protocol Buffers
On Oct 11, 11:32 am, Scott Saad <saa...@gmail.com> wrote:
> In my opinion these changes would be great to have merged back into the
> protobuf proper branch as most of them aim to make the code base more cross
> compiler compliant. I suppose the C++Builder is a bit more strict on a few
> items.

My experience has been that C++Builder is worse (less standards-
compliant / less capable) than GCC and MSVC, so most changes to
accommodate it are actually working around its shortcomings.

I was working some on getting protobuf to work in C++Builder as well.
I'm curious why you had to remove "using namespace std"? I was able
to get it working without doing that.

I think that you should not have removed
MessageType_WorkAroundCppLookupDefect (in wire_format_lite_inl.h) for
all compilers. My understanding is that its intent is to avoid the
overhead of virtual function dispatch, although I couldn't get it to
work in C++Builder.

I'll have to give your version a try later. It looks like you solved
a couple of problems that were giving me trouble.

--
Josh Kelley

Scott Saad

unread,
Oct 17, 2011, 2:45:31 PM10/17/11
to prot...@googlegroups.com


My experience has been that C++Builder is worse (less standards-
compliant / less capable) than GCC and MSVC, so most changes to
accommodate it are actually working around its shortcomings.

I've also struggled with C++Builder in the past.


I was working some on getting protobuf to work in C++Builder as well.
I'm curious why you had to remove "using namespace std"?  I was able
to get it working without doing that.

IMHO, a "using namespace" statement in a header file is always a bad thing. I was running into weird namespace collision issues and was having trouble discerning between STL related ones and real problems. By removing that from common.h, it helped me zero in on what was causing some of the more seemingly obscure compile issues. Whether these issues were due to shortcomings of C++Builder's compiler I'm not comfortable/familiar enough with the C++ standard to be an authority on the matter.



I think that you should not have removed
MessageType_WorkAroundCppLookupDefect (in wire_format_lite_inl.h) for
all compilers.  My understanding is that its intent is to avoid the
overhead of virtual function dispatch, although I couldn't get it to
work in C++Builder.

Interesting, I will look closer at this as I'm unfamiliar as well.
 

I'll have to give your version a try later.  It looks like you solved
a couple of problems that were giving me trouble.

Thank you for your thoughts on these matters. Feedback is always welcomed! 
 
Scott

Pherl Liu

unread,
Nov 7, 2011, 4:10:37 AM11/7/11
to prot...@googlegroups.com
Thanks for the work! I've put a link in the 3rd party wiki page 

We'd like to accept patches. However, we don't have enough engineering resources to maintain another platform. Supporting a different platform requires much testing work in the release process (also user support). I'd suggest to keep the C++ builder branch. You can always merge when a new version of protobuf is released.
 

Thank you,
Scott Saad

--
You received this message because you are subscribed to the Google Groups "Protocol Buffers" group.
To view this discussion on the web visit https://groups.google.com/d/msg/protobuf/-/5L32fr5BM-QJ.

To post to this group, send email to prot...@googlegroups.com.
To unsubscribe from this group, send email to protobuf+u...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/protobuf?hl=en.

Scott Saad

unread,
Nov 9, 2011, 12:35:36 PM11/9/11
to prot...@googlegroups.com
Hello,
 
Thank you for adding a link! 

We'd like to accept patches. However, we don't have enough engineering resources to maintain another platform. Supporting a different platform requires much testing work in the release process (also user support). I'd suggest to keep the C++ builder branch. You can always merge when a new version of protobuf is released.

I can completely understand not wanting to support another platform and am happy to maintain the C++Builder branch. At the same time, if I were wanting to submit a patch, how do I go about that? Through this group?

Kind regards,
Scott 

Pherl Liu

unread,
Nov 9, 2011, 11:56:58 PM11/9/11
to prot...@googlegroups.com

Thanks for the understanding. You can create an issue on the project page(http://code.google.com/p/protobuf/issues/list) and put the diffs as attachments.
 

Kind regards,
Scott 

--
You received this message because you are subscribed to the Google Groups "Protocol Buffers" group.
To view this discussion on the web visit https://groups.google.com/d/msg/protobuf/-/8X7Wsx6yns4J.
Reply all
Reply to author
Forward
0 new messages