Make C++ compile and run under gcc-4.4 #6863

vjpai · 2016-06-10T06:00:20Z

Fixes #1928
Basic concepts are as follows

Do not allow range-based for or lambda
No explicit pointer comparisons to nullptr . All pointers have an implicit conversion to bool and that should be used in comparisons
Use GRPC_OVERRIDE and GRPC_FINAL, not override or final
unique_ptr must be used with extreme caution in any kind of collection and cannot be used in map (this change has not yet been added to this PR)

A separate PR adds relevant documentation for C++ development information (#6865)

Remove range-based fors

Do not make explicit comparisons against nullptr; only use implicit typecast to bool

vjpai · 2016-06-10T17:54:33Z

Dealing with this has raised another issue, documented in #6866

vjpai · 2016-06-10T19:44:07Z

Fixes #6866

vjpai · 2016-06-10T19:44:49Z

Remaining issue: build projects still has all the C++ boringssl tests included. We need to remove this for this case since we are bypassing boringssl for gcc-4.4

jtattermusch · 2016-06-10T23:23:07Z

Nit: clang format is needed.
https://grpc-testing.appspot.com/job/gRPC_pull_requests/9773/config=opt,language=sanity,platform=linux/testReport/junit/(root)/tests/tools_distrib_clang_format_code_sh/

vjpai · 2016-06-10T23:24:09Z

Yeah, about that. clang-format dockerfile is currently broken because it depends on http://llvm.org/apt , which is temporarily down .

ctiller · 2016-06-10T23:42:59Z

I'd offer to send you a PR against this, but I rebooted my machine: someone
else nearby might be able to do so...

On Fri, Jun 10, 2016 at 4:24 PM Vijay Pai notifications@github.com wrote:

Yeah, about that. clang-format dockerfile is currently broken because it
depends on http://llvm.org/apt , which is temporarily down .

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#6863 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AJpudaGJqJYSncElBoE6ArE6Wyueh0d-ks5qKfIqgaJpZM4IypSO
.

vjpai · 2016-06-13T17:36:44Z

The remaining issue here is that the C++ tests pull in the boringssl tests even if we're not (and can't) use boringssl. Any ideas how to deactivate those?

vjpai · 2016-06-13T19:17:45Z

Working on providing a mechanism to disable the tests we don't want.

vjpai · 2016-06-13T22:38:33Z

Fixes #6861

vjpai · 2016-06-14T19:33:04Z

Current status: need to separate out boringssl test libs from privatelibs_cxx in Makefile which is one of the dependences for building C++ tests . Will require changes to makefile generation or templates.

not true

vjpai · 2016-06-15T16:44:59Z

Now we've got a protobuf issue. Need to see how g++4.4 handles friend templates as that seems to be the problem

a map to 2 vectors. Additional minor changes needed (e.g., override->GRPC_OVERRIDE, nullptr->grpc::nullptr)

vjpai · 2016-06-16T23:25:46Z

My PR to protobuf (protocolbuffers/protobuf#1692) was merged, so this should build just fine if we pull down the newest protobuf. @nicolasnoble - would appreciate any advice on packaging, etc. implications.

dgquintas · 2016-06-17T05:40:14Z

heh, it seems you are going to have to fight clang-format on this one...

-    : public CommonStressTest< ::grpc::testing::EchoTestService::AsyncService> {
+    : public CommonStressTest<::grpc::testing::EchoTestService::AsyncService> {

vjpai · 2016-06-17T07:46:15Z

Just drop the initial paamayim nekudotayim; it's cleaner. Dropping the reference from global namespace and starting with grpc. Made other change requested by clang-format as well.

jtattermusch · 2016-06-17T20:34:17Z

Our third_party/protobuf has been bumped to the version with gcc4.4 fixes: #6952

dgquintas · 2016-06-17T22:32:56Z

include/grpc++/ext/proto_server_reflection_plugin.h

@@ -56,7 +56,7 @@ class ProtoServerReflectionPlugin : public ::grpc::ServerBuilderPlugin {
  bool has_sync_methods() const GRPC_OVERRIDE;

 private:
-  std::shared_ptr<::grpc::ProtoServerReflection> reflection_service_;
+  std::shared_ptr< ::grpc::ProtoServerReflection> reflection_service_;


very interesting that clang-format doesn't undo this change... would we still want to s/ ::grpc/grpc/?

probably for consistency, as they've been removed from the other places as well.

dgquintas · 2016-06-17T22:44:15Z

LGTM, just one nit comment regarding s/ ::grpc/grpc/

vjpai · 2016-06-17T23:03:15Z

Confirming that all tests pass when running gcc4.4 manually through docker

vjpai added 3 commits June 9, 2016 15:50

Add g++

203b20c

override->GRPC_OVERRIDE

ce350a5

Remove range-based fors

Remove another range-based for

b645a2d

Do not make explicit comparisons against nullptr; only use implicit typecast to bool

vjpai added lang/c++ disposition/DO NOT MERGE labels Jun 10, 2016

vjpai added this to the 2016/05/21 Milestone milestone Jun 10, 2016

googlebot added the cla: yes label Jun 10, 2016

WIP

eac07c3

Switch server builder plugin API to something that will work with gcc4.4

15855f3

vjpai mentioned this pull request Jun 11, 2016

Add support for clang3.5, clang3.7 and gcc4.6 in run_tests.py #6869

Merged

Manual clang-format

97da647

vjpai added 2 commits June 13, 2016 12:37

Add an option --regex_exclude to block certain tests from running

488fd0e

Set back to the default log function after using an override

8b48191

Exclude boringssl tests and libs from C++ build if EMBED_OPENSSL is

2041092

not true

vjpai added 5 commits June 15, 2016 10:23

Enforce gcc-4.4 compatibility on test_credentials_provider by changing

efa8430

a map to 2 vectors. Additional minor changes needed (e.g., override->GRPC_OVERRIDE, nullptr->grpc::nullptr)

Switch out a using with a typedef

9aa5f23

Provide nullptr->std::function cast

06ea541

Specify template parent explicitly

08e071a

Got carried away with grpc::

12bf380

vjpai added 5 commits June 16, 2016 00:24

Merge branch 'wheezy' of github.com:vjpai/grpc into wheezy

62eb1c7

Digraph

90460fa

Digraph

4e1a883

Remove range-based for, comparisons to nullptr

6b668cc

Include <cinttypes> for PRIuPTR. Not clear why this is needed.

b31a9a0

vjpai removed the disposition/DO NOT MERGE label Jun 16, 2016

Don't global initialize anything to nullptr

b8916aa

vjpai mentioned this pull request Jun 16, 2016

Implement C++ API for defining channel filters. #6915

Merged

clang-format changes

6ab4788

Merge branch 'master' into wheezy

ddf694c

dgquintas reviewed Jun 17, 2016
View reviewed changes

vjpai added 2 commits June 17, 2016 15:53

Merge branch 'wheezy' of github.com:vjpai/grpc into wheezy

f486881

Stop using global namespace on a template param

7b4dc53

jtattermusch added the disposition/ready to merge label Jun 18, 2016

jtattermusch merged commit 56f9862 into grpc:master Jun 18, 2016

This was referenced Jun 18, 2016

ServerBuilderOption::UpdatePlugins API is not compatible with gcc-4.4 #6866

Closed

Some tests consistently failing for C core with gcc4.4 #6861

Closed

vjpai deleted the wheezy branch August 18, 2016 20:22

lock bot locked as resolved and limited conversation to collaborators Jan 26, 2019

lock bot unassigned dgquintas Jan 26, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make C++ compile and run under gcc-4.4 #6863

Make C++ compile and run under gcc-4.4 #6863

vjpai commented Jun 10, 2016 •

edited

Loading

vjpai commented Jun 10, 2016

vjpai commented Jun 10, 2016

vjpai commented Jun 10, 2016

jtattermusch commented Jun 10, 2016

vjpai commented Jun 10, 2016

ctiller commented Jun 10, 2016

vjpai commented Jun 13, 2016

vjpai commented Jun 13, 2016

vjpai commented Jun 13, 2016

vjpai commented Jun 14, 2016

vjpai commented Jun 15, 2016

vjpai commented Jun 16, 2016

dgquintas commented Jun 17, 2016

vjpai commented Jun 17, 2016

jtattermusch commented Jun 17, 2016

dgquintas Jun 17, 2016

dgquintas Jun 17, 2016

vjpai Jun 17, 2016

dgquintas commented Jun 17, 2016

vjpai commented Jun 17, 2016

Make C++ compile and run under gcc-4.4 #6863

Make C++ compile and run under gcc-4.4 #6863

Conversation

vjpai commented Jun 10, 2016 • edited Loading

vjpai commented Jun 10, 2016

vjpai commented Jun 10, 2016

vjpai commented Jun 10, 2016

jtattermusch commented Jun 10, 2016

vjpai commented Jun 10, 2016

ctiller commented Jun 10, 2016

vjpai commented Jun 13, 2016

vjpai commented Jun 13, 2016

vjpai commented Jun 13, 2016

vjpai commented Jun 14, 2016

vjpai commented Jun 15, 2016

vjpai commented Jun 16, 2016

dgquintas commented Jun 17, 2016

vjpai commented Jun 17, 2016

jtattermusch commented Jun 17, 2016

dgquintas Jun 17, 2016

Choose a reason for hiding this comment

dgquintas Jun 17, 2016

Choose a reason for hiding this comment

vjpai Jun 17, 2016

Choose a reason for hiding this comment

dgquintas commented Jun 17, 2016

vjpai commented Jun 17, 2016

vjpai commented Jun 10, 2016 •

edited

Loading