From 2707fd0bd406488665776dfd89987d95e26e2d51 Mon Sep 17 00:00:00 2001 From: Esun Kim Date: Fri, 23 Aug 2019 15:47:40 -0700 Subject: [PATCH] Added GRPC_USE_CPP_STD_LIB flag --- BUILD | 5 +++++ bazel/grpc_build_system.bzl | 3 +++ include/grpc/impl/codegen/port_platform.h | 9 +++++++++ .../lb_policy/xds/xds_client_stats.cc | 8 ++++++++ .../lb_policy/xds/xds_load_balancer_api.cc | 8 ++++++++ src/core/lib/gprpp/abstract.h | 10 ++++++++++ src/core/lib/gprpp/map.h | 16 ++++++++++++++++ src/core/lib/security/credentials/credentials.h | 10 ++++++++-- test/core/gprpp/map_test.cc | 9 +++++++++ .../linux/grpc_bazel_build_in_docker.sh | 1 + 10 files changed, 77 insertions(+), 2 deletions(-) diff --git a/BUILD b/BUILD index c510757c35d..6855ad9602e 100644 --- a/BUILD +++ b/BUILD @@ -73,6 +73,11 @@ config_setting( values = {"cpu": "darwin"}, ) +config_setting( + name = "grpc_use_cpp_std_lib", + values = {"define": "GRPC_USE_CPP_STD_LIB=1"}, +) + # This should be updated along with build.yaml g_stands_for = "ganges" diff --git a/bazel/grpc_build_system.bzl b/bazel/grpc_build_system.bzl index f816fe14ff4..23f90d0dc80 100644 --- a/bazel/grpc_build_system.bzl +++ b/bazel/grpc_build_system.bzl @@ -98,6 +98,9 @@ def grpc_cc_library( "//:grpc_allow_exceptions": ["GRPC_ALLOW_EXCEPTIONS=1"], "//:grpc_disallow_exceptions": ["GRPC_ALLOW_EXCEPTIONS=0"], "//conditions:default": [], + }) + select({ + "//:grpc_use_cpp_std_lib": ["GRPC_USE_CPP_STD_LIB=1"], + "//conditions:default": [], }), hdrs = hdrs + public_hdrs, deps = deps + _get_external_deps(external_deps), diff --git a/include/grpc/impl/codegen/port_platform.h b/include/grpc/impl/codegen/port_platform.h index dd49a66fe25..bab57657a00 100644 --- a/include/grpc/impl/codegen/port_platform.h +++ b/include/grpc/impl/codegen/port_platform.h @@ -27,6 +27,15 @@ * - some syscalls to be made directly */ +/* + * Defines GRPC_USE_CPP_STD_LIB to use standard C++ library instead of + * in-house library if possible. (e.g. std::map) + */ +#ifndef GRPC_USE_CPP_STD_LIB +/* Default value will be 1 once all tests become green. */ +#define GRPC_USE_CPP_STD_LIB 0 +#endif + /* Get windows.h included everywhere (we need it) */ #if defined(_WIN64) || defined(WIN64) || defined(_WIN32) || defined(WIN32) #ifndef WIN32_LEAN_AND_MEAN diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/xds_client_stats.cc b/src/core/ext/filters/client_channel/lb_policy/xds/xds_client_stats.cc index 85f7d669ec0..a866d50b3a6 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/xds_client_stats.cc +++ b/src/core/ext/filters/client_channel/lb_policy/xds/xds_client_stats.cc @@ -143,7 +143,15 @@ XdsClientStats::Snapshot XdsClientStats::GetSnapshotAndReset() { } { MutexLock lock(&dropped_requests_mu_); +#if GRPC_USE_CPP_STD_LIB + // This is a workaround for the case where some compilers cannot build + // move-assignment of map with non-copyable but movable key. + // https://stackoverflow.com/questions/36475497 + std::swap(snapshot.dropped_requests, dropped_requests_); + dropped_requests_.clear(); +#else snapshot.dropped_requests = std::move(dropped_requests_); +#endif } return snapshot; } diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/xds_load_balancer_api.cc b/src/core/ext/filters/client_channel/lb_policy/xds/xds_load_balancer_api.cc index ac87053d47f..16d5f9a1391 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/xds_load_balancer_api.cc +++ b/src/core/ext/filters/client_channel/lb_policy/xds/xds_load_balancer_api.cc @@ -315,7 +315,15 @@ grpc_slice XdsLrsRequestCreateAndEncode(const char* server_name) { namespace { void LocalityStatsPopulate(envoy_api_v2_endpoint_UpstreamLocalityStats* output, +#if GRPC_USE_CPP_STD_LIB + // TODO(veblush): Clean up this + // This is to address the difference between + // std::map and Map. #else block will be gone + // once using stdlib is enabled by default. + Pair, +#else Pair, +#endif XdsClientStats::LocalityStats::Snapshot>& input, upb_arena* arena) { // Set sub_zone. diff --git a/src/core/lib/gprpp/abstract.h b/src/core/lib/gprpp/abstract.h index 5b7018e07e0..ea68e3ab041 100644 --- a/src/core/lib/gprpp/abstract.h +++ b/src/core/lib/gprpp/abstract.h @@ -19,6 +19,14 @@ #ifndef GRPC_CORE_LIB_GPRPP_ABSTRACT_H #define GRPC_CORE_LIB_GPRPP_ABSTRACT_H +#if GRPC_USE_CPP_STD_LIB + +#define GRPC_ABSTRACT_BASE_CLASS + +#define GRPC_ABSTRACT = 0 + +#else + // This is needed to support abstract base classes in the c core. Since gRPC // doesn't have a c++ runtime, it will hit a linker error on delete unless // we define a virtual operator delete. See this blog for more info: @@ -34,4 +42,6 @@ GPR_ASSERT(false); \ } +#endif // GRPC_USE_CPP_STD_LIB + #endif /* GRPC_CORE_LIB_GPRPP_ABSTRACT_H */ diff --git a/src/core/lib/gprpp/map.h b/src/core/lib/gprpp/map.h index 705b605e5b9..134625775cf 100644 --- a/src/core/lib/gprpp/map.h +++ b/src/core/lib/gprpp/map.h @@ -27,12 +27,17 @@ #include #include +#if GRPC_USE_CPP_STD_LIB +#include +#endif + #include "src/core/lib/gpr/useful.h" #include "src/core/lib/gprpp/memory.h" #include "src/core/lib/gprpp/pair.h" #include "src/core/lib/gprpp/ref_counted_ptr.h" namespace grpc_core { + struct StringLess { bool operator()(const char* a, const char* b) const { return strcmp(a, b) < 0; @@ -50,6 +55,13 @@ struct RefCountedPtrLess { } }; +#if GRPC_USE_CPP_STD_LIB + +template > +using Map = std::map; + +#else // GRPC_USE_CPP_STD_LIB + namespace testing { class MapTest; } @@ -537,5 +549,9 @@ int Map::CompareKeys(const key_type& lhs, } return left_comparison ? -1 : 1; } + +#endif // GRPC_USE_CPP_STD_LIB + } // namespace grpc_core + #endif /* GRPC_CORE_LIB_GPRPP_MAP_H */ diff --git a/src/core/lib/security/credentials/credentials.h b/src/core/lib/security/credentials/credentials.h index f3fbe881598..b43405b0861 100644 --- a/src/core/lib/security/credentials/credentials.h +++ b/src/core/lib/security/credentials/credentials.h @@ -110,11 +110,17 @@ struct grpc_channel_credentials create_security_connector( grpc_core::RefCountedPtr call_creds, const char* target, const grpc_channel_args* args, - grpc_channel_args** new_args) { + grpc_channel_args** new_args) +#if GRPC_USE_CPP_STD_LIB + = 0; +#else + { // Tell clang-tidy that call_creds cannot be passed as const-ref. call_creds.reset(); - GRPC_ABSTRACT; + gpr_log(GPR_ERROR, "Function marked GRPC_ABSTRACT was not implemented"); + GPR_ASSERT(false); } +#endif // Creates a version of the channel credentials without any attached call // credentials. This can be used in order to open a channel to a non-trusted diff --git a/test/core/gprpp/map_test.cc b/test/core/gprpp/map_test.cc index b0be7960b57..7dc1ba636f3 100644 --- a/test/core/gprpp/map_test.cc +++ b/test/core/gprpp/map_test.cc @@ -29,6 +29,13 @@ namespace grpc_core { namespace testing { + +#if GRPC_USE_CPP_STD_LIB + +TEST(MapTest, Nop) {} + +#else + class Payload { public: Payload() : data_(-1) {} @@ -495,6 +502,8 @@ TEST_F(MapTest, CopyAssignment) { EXPECT_EQ(test_map2.end(), test_map2.find("xxx")); } +#endif + } // namespace testing } // namespace grpc_core diff --git a/tools/internal_ci/linux/grpc_bazel_build_in_docker.sh b/tools/internal_ci/linux/grpc_bazel_build_in_docker.sh index 24598f43f02..8487ec49bbd 100755 --- a/tools/internal_ci/linux/grpc_bazel_build_in_docker.sh +++ b/tools/internal_ci/linux/grpc_bazel_build_in_docker.sh @@ -25,3 +25,4 @@ git clone /var/local/jenkins/grpc /var/local/git/grpc ${name}') cd /var/local/git/grpc bazel build --spawn_strategy=standalone --genrule_strategy=standalone :all test/... examples/... +bazel build --spawn_strategy=standalone --genrule_strategy=standalone --define=GRPC_USE_CPP_STD_LIB=1 :grpc