Move Optional to gprpp, and reviewer comments

pull/17757/head
Yash Tibrewal 6 years ago
parent 862faf55ba
commit 9df6023dca
  1. 12
      BUILD
  2. 40
      CMakeLists.txt
  3. 48
      Makefile
  4. 14
      build.yaml
  5. 2
      gRPC-C++.podspec
  6. 2
      gRPC-Core.podspec
  7. 1
      grpc.gemspec
  8. 1
      package.xml
  9. 45
      src/core/lib/gprpp/optional.h
  10. 6
      src/core/lib/iomgr/buffer_list.cc
  11. 22
      src/core/lib/iomgr/buffer_list.h
  12. 13
      test/core/gprpp/BUILD
  13. 50
      test/core/gprpp/optional_test.cc
  14. 15
      test/core/iomgr/buffer_list_test.cc
  15. 1
      tools/doxygen/Doxyfile.c++.internal
  16. 1
      tools/doxygen/Doxyfile.core.internal
  17. 20
      tools/run_tests/generated/sources_and_headers.json
  18. 24
      tools/run_tests/generated/tests.json

12
BUILD

@ -643,6 +643,17 @@ grpc_cc_library(
public_hdrs = ["src/core/lib/gprpp/debug_location.h"],
)
grpc_cc_library(
name = "optional",
language = "c++",
public_hdrs = [
"src/core/lib/gprpp/optional.h",
],
deps = [
"gpr_base",
],
)
grpc_cc_library(
name = "orphanable",
language = "c++",
@ -976,6 +987,7 @@ grpc_cc_library(
"grpc_codegen",
"grpc_trace",
"inlined_vector",
"optional",
"orphanable",
"ref_counted",
"ref_counted_ptr",

@ -652,6 +652,7 @@ add_dependencies(buildtests_cxx metrics_client)
add_dependencies(buildtests_cxx mock_test)
add_dependencies(buildtests_cxx nonblocking_test)
add_dependencies(buildtests_cxx noop-benchmark)
add_dependencies(buildtests_cxx optional_test)
add_dependencies(buildtests_cxx orphanable_test)
add_dependencies(buildtests_cxx proto_server_reflection_test)
add_dependencies(buildtests_cxx proto_utils_test)
@ -14443,6 +14444,45 @@ target_link_libraries(noop-benchmark
)
endif (gRPC_BUILD_TESTS)
if (gRPC_BUILD_TESTS)
add_executable(optional_test
test/core/gprpp/optional_test.cc
third_party/googletest/googletest/src/gtest-all.cc
third_party/googletest/googlemock/src/gmock-all.cc
)
target_include_directories(optional_test
PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}
PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include
PRIVATE ${_gRPC_SSL_INCLUDE_DIR}
PRIVATE ${_gRPC_PROTOBUF_INCLUDE_DIR}
PRIVATE ${_gRPC_ZLIB_INCLUDE_DIR}
PRIVATE ${_gRPC_BENCHMARK_INCLUDE_DIR}
PRIVATE ${_gRPC_CARES_INCLUDE_DIR}
PRIVATE ${_gRPC_GFLAGS_INCLUDE_DIR}
PRIVATE ${_gRPC_ADDRESS_SORTING_INCLUDE_DIR}
PRIVATE ${_gRPC_NANOPB_INCLUDE_DIR}
PRIVATE third_party/googletest/googletest/include
PRIVATE third_party/googletest/googletest
PRIVATE third_party/googletest/googlemock/include
PRIVATE third_party/googletest/googlemock
PRIVATE ${_gRPC_PROTO_GENS_DIR}
)
target_link_libraries(optional_test
${_gRPC_PROTOBUF_LIBRARIES}
${_gRPC_ALLTARGETS_LIBRARIES}
grpc_test_util
grpc++
grpc
gpr
${_gRPC_GFLAGS_LIBRARIES}
)
endif (gRPC_BUILD_TESTS)
if (gRPC_BUILD_TESTS)

@ -1213,6 +1213,7 @@ metrics_client: $(BINDIR)/$(CONFIG)/metrics_client
mock_test: $(BINDIR)/$(CONFIG)/mock_test
nonblocking_test: $(BINDIR)/$(CONFIG)/nonblocking_test
noop-benchmark: $(BINDIR)/$(CONFIG)/noop-benchmark
optional_test: $(BINDIR)/$(CONFIG)/optional_test
orphanable_test: $(BINDIR)/$(CONFIG)/orphanable_test
proto_server_reflection_test: $(BINDIR)/$(CONFIG)/proto_server_reflection_test
proto_utils_test: $(BINDIR)/$(CONFIG)/proto_utils_test
@ -1720,6 +1721,7 @@ buildtests_cxx: privatelibs_cxx \
$(BINDIR)/$(CONFIG)/mock_test \
$(BINDIR)/$(CONFIG)/nonblocking_test \
$(BINDIR)/$(CONFIG)/noop-benchmark \
$(BINDIR)/$(CONFIG)/optional_test \
$(BINDIR)/$(CONFIG)/orphanable_test \
$(BINDIR)/$(CONFIG)/proto_server_reflection_test \
$(BINDIR)/$(CONFIG)/proto_utils_test \
@ -1906,6 +1908,7 @@ buildtests_cxx: privatelibs_cxx \
$(BINDIR)/$(CONFIG)/mock_test \
$(BINDIR)/$(CONFIG)/nonblocking_test \
$(BINDIR)/$(CONFIG)/noop-benchmark \
$(BINDIR)/$(CONFIG)/optional_test \
$(BINDIR)/$(CONFIG)/orphanable_test \
$(BINDIR)/$(CONFIG)/proto_server_reflection_test \
$(BINDIR)/$(CONFIG)/proto_utils_test \
@ -2399,6 +2402,8 @@ test_cxx: buildtests_cxx
$(Q) $(BINDIR)/$(CONFIG)/nonblocking_test || ( echo test nonblocking_test failed ; exit 1 )
$(E) "[RUN] Testing noop-benchmark"
$(Q) $(BINDIR)/$(CONFIG)/noop-benchmark || ( echo test noop-benchmark failed ; exit 1 )
$(E) "[RUN] Testing optional_test"
$(Q) $(BINDIR)/$(CONFIG)/optional_test || ( echo test optional_test failed ; exit 1 )
$(E) "[RUN] Testing orphanable_test"
$(Q) $(BINDIR)/$(CONFIG)/orphanable_test || ( echo test orphanable_test failed ; exit 1 )
$(E) "[RUN] Testing proto_server_reflection_test"
@ -19442,6 +19447,49 @@ endif
endif
OPTIONAL_TEST_SRC = \
test/core/gprpp/optional_test.cc \
OPTIONAL_TEST_OBJS = $(addprefix $(OBJDIR)/$(CONFIG)/, $(addsuffix .o, $(basename $(OPTIONAL_TEST_SRC))))
ifeq ($(NO_SECURE),true)
# You can't build secure targets if you don't have OpenSSL.
$(BINDIR)/$(CONFIG)/optional_test: openssl_dep_error
else
ifeq ($(NO_PROTOBUF),true)
# You can't build the protoc plugins or protobuf-enabled targets if you don't have protobuf 3.5.0+.
$(BINDIR)/$(CONFIG)/optional_test: protobuf_dep_error
else
$(BINDIR)/$(CONFIG)/optional_test: $(PROTOBUF_DEP) $(OPTIONAL_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc++.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr.a
$(E) "[LD] Linking $@"
$(Q) mkdir -p `dirname $@`
$(Q) $(LDXX) $(LDFLAGS) $(OPTIONAL_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc++.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr.a $(LDLIBSXX) $(LDLIBS_PROTOBUF) $(LDLIBS) $(LDLIBS_SECURE) $(GTEST_LIB) -o $(BINDIR)/$(CONFIG)/optional_test
endif
endif
$(OBJDIR)/$(CONFIG)/test/core/gprpp/optional_test.o: $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc++.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr.a
deps_optional_test: $(OPTIONAL_TEST_OBJS:.o=.dep)
ifneq ($(NO_SECURE),true)
ifneq ($(NO_DEPS),true)
-include $(OPTIONAL_TEST_OBJS:.o=.dep)
endif
endif
ORPHANABLE_TEST_SRC = \
test/core/gprpp/orphanable_test.cc \

@ -430,6 +430,7 @@ filegroups:
- src/core/lib/debug/stats_data.h
- src/core/lib/gprpp/debug_location.h
- src/core/lib/gprpp/inlined_vector.h
- src/core/lib/gprpp/optional.h
- src/core/lib/gprpp/orphanable.h
- src/core/lib/gprpp/ref_counted.h
- src/core/lib/gprpp/ref_counted_ptr.h
@ -5057,6 +5058,19 @@ targets:
deps:
- benchmark
defaults: benchmark
- name: optional_test
gtest: true
build: test
language: c++
src:
- test/core/gprpp/optional_test.cc
deps:
- grpc_test_util
- grpc++
- grpc
- gpr
uses:
- grpc++_test
- name: orphanable_test
gtest: true
build: test

@ -402,6 +402,7 @@ Pod::Spec.new do |s|
'src/core/lib/debug/stats_data.h',
'src/core/lib/gprpp/debug_location.h',
'src/core/lib/gprpp/inlined_vector.h',
'src/core/lib/gprpp/optional.h',
'src/core/lib/gprpp/orphanable.h',
'src/core/lib/gprpp/ref_counted.h',
'src/core/lib/gprpp/ref_counted_ptr.h',
@ -595,6 +596,7 @@ Pod::Spec.new do |s|
'src/core/lib/debug/stats_data.h',
'src/core/lib/gprpp/debug_location.h',
'src/core/lib/gprpp/inlined_vector.h',
'src/core/lib/gprpp/optional.h',
'src/core/lib/gprpp/orphanable.h',
'src/core/lib/gprpp/ref_counted.h',
'src/core/lib/gprpp/ref_counted_ptr.h',

@ -396,6 +396,7 @@ Pod::Spec.new do |s|
'src/core/lib/debug/stats_data.h',
'src/core/lib/gprpp/debug_location.h',
'src/core/lib/gprpp/inlined_vector.h',
'src/core/lib/gprpp/optional.h',
'src/core/lib/gprpp/orphanable.h',
'src/core/lib/gprpp/ref_counted.h',
'src/core/lib/gprpp/ref_counted_ptr.h',
@ -1024,6 +1025,7 @@ Pod::Spec.new do |s|
'src/core/lib/debug/stats_data.h',
'src/core/lib/gprpp/debug_location.h',
'src/core/lib/gprpp/inlined_vector.h',
'src/core/lib/gprpp/optional.h',
'src/core/lib/gprpp/orphanable.h',
'src/core/lib/gprpp/ref_counted.h',
'src/core/lib/gprpp/ref_counted_ptr.h',

@ -332,6 +332,7 @@ Gem::Specification.new do |s|
s.files += %w( src/core/lib/debug/stats_data.h )
s.files += %w( src/core/lib/gprpp/debug_location.h )
s.files += %w( src/core/lib/gprpp/inlined_vector.h )
s.files += %w( src/core/lib/gprpp/optional.h )
s.files += %w( src/core/lib/gprpp/orphanable.h )
s.files += %w( src/core/lib/gprpp/ref_counted.h )
s.files += %w( src/core/lib/gprpp/ref_counted_ptr.h )

@ -337,6 +337,7 @@
<file baseinstalldir="/" name="src/core/lib/debug/stats_data.h" role="src" />
<file baseinstalldir="/" name="src/core/lib/gprpp/debug_location.h" role="src" />
<file baseinstalldir="/" name="src/core/lib/gprpp/inlined_vector.h" role="src" />
<file baseinstalldir="/" name="src/core/lib/gprpp/optional.h" role="src" />
<file baseinstalldir="/" name="src/core/lib/gprpp/orphanable.h" role="src" />
<file baseinstalldir="/" name="src/core/lib/gprpp/ref_counted.h" role="src" />
<file baseinstalldir="/" name="src/core/lib/gprpp/ref_counted_ptr.h" role="src" />

@ -0,0 +1,45 @@
/*
*
* Copyright 2019 gRPC authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/
#ifndef GRPC_CORE_LIB_GPRPP_OPTIONAL_H
#define GRPC_CORE_LIB_GPRPP_OPTIONAL_H
namespace grpc_core {
/* A make-shift alternative for absl::Optional. This can be removed in favor of
* that once absl dependencies can be introduced. */
template <typename T>
class Optional {
public:
void set(const T& val) {
value_ = val;
set_ = true;
}
bool has_value() { return set_; }
void reset() { set_ = false; }
T value() { return value_; }
T value_;
bool set_ = false;
};
} /* namespace grpc_core */
#endif /* GRPC_CORE_LIB_GPRPP_OPTIONAL_H */

@ -66,8 +66,8 @@ void extract_opt_stats_from_tcp_info(ConnectionMetrics* metrics,
}
if (info->length > offsetof(grpc_core::tcp_info, tcpi_sndbuf_limited)) {
metrics->recurring_retrans.set(info->tcpi_retransmits);
metrics->is_delivery_rate_app_limited =
info->tcpi_delivery_rate_app_limited;
metrics->is_delivery_rate_app_limited.set(
info->tcpi_delivery_rate_app_limited);
metrics->congestion_window.set(info->tcpi_snd_cwnd);
metrics->reordering.set(info->tcpi_reordering);
metrics->packet_retx.set(info->tcpi_total_retrans);
@ -126,7 +126,7 @@ void extract_opt_stats_from_cmsg(ConnectionMetrics* metrics,
break;
}
case TCP_NLA_DELIVERY_RATE_APP_LMT: {
metrics->is_delivery_rate_app_limited = read_unaligned<uint8_t>(val);
metrics->is_delivery_rate_app_limited.set(read_unaligned<uint8_t>(val));
break;
}
case TCP_NLA_SND_CWND: {

@ -26,35 +26,17 @@
#include <grpc/support/time.h>
#include "src/core/lib/gprpp/memory.h"
#include "src/core/lib/gprpp/optional.h"
#include "src/core/lib/iomgr/error.h"
#include "src/core/lib/iomgr/internal_errqueue.h"
namespace grpc_core {
/* A make-shift alternative for absl::Optional. This can be removed in favor of
* that once is absl dependencies can be introduced. */
template <typename T>
class Optional {
public:
void set(const T& val) {
value_ = val;
set_ = true;
}
bool has_value() { return set_; }
void reset() { set_ = false; }
T value() { return value_; }
T value_;
bool set_ = false;
};
struct ConnectionMetrics {
/* Delivery rate in Bps. */
Optional<uint64_t> delivery_rate;
/* If the delivery rate is limited by the application, this is set to true. */
bool is_delivery_rate_app_limited = true;
Optional<uint64_t> is_delivery_rate_app_limited;
/* Total packets retransmitted. */
Optional<uint32_t> packet_retx;
/* Total packets retransmitted spuriously. This metric is smaller than or

@ -64,6 +64,19 @@ grpc_cc_test(
],
)
grpc_cc_test(
name = "optional_test",
srcs = ["optional_test.cc"],
external_deps = [
"gtest",
],
language = "C++",
deps = [
"//:optional",
"//test/core/util:grpc_test_util",
],
)
grpc_cc_test(
name = "orphanable_test",
srcs = ["orphanable_test.cc"],

@ -0,0 +1,50 @@
/*
*
* Copyright 2019 gRPC authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/
#include "src/core/lib/gprpp/optional.h"
#include <grpc/support/log.h>
#include <gtest/gtest.h>
#include "src/core/lib/gprpp/memory.h"
#include "test/core/util/test_config.h"
namespace grpc_core {
namespace testing {
namespace {
TEST(OptionalTest, BasicTest) {
grpc_core::Optional<int> opt_val;
EXPECT_FALSE(opt_val.has_value());
const int kTestVal = 123;
opt_val.set(kTestVal);
EXPECT_TRUE(opt_val.has_value());
EXPECT_EQ(opt_val.value(), kTestVal);
opt_val.reset();
EXPECT_EQ(opt_val.has_value(), false);
}
} // namespace
} // namespace testing
} // namespace grpc_core
int main(int argc, char** argv) {
grpc::testing::TestEnvironment env(argc, argv);
::testing::InitGoogleTest(&argc, argv);
return RUN_ALL_TESTS();
}

@ -96,25 +96,10 @@ static void TestTcpBufferList() {
TestShutdownFlushesList();
}
/* Tests grpc_core::Optional */
static void TestOptional() {
grpc_core::Optional<int> opt_val;
GPR_ASSERT(opt_val.has_value() == false);
const int kTestVal = 123;
opt_val.set(kTestVal);
GPR_ASSERT(opt_val.has_value());
GPR_ASSERT(opt_val.value() == 123);
opt_val.reset();
GPR_ASSERT(opt_val.has_value() == false);
}
int main(int argc, char** argv) {
grpc::testing::TestEnvironment env(argc, argv);
grpc_init();
TestTcpBufferList();
TestOptional();
grpc_shutdown();
return 0;
}

@ -1076,6 +1076,7 @@ src/core/lib/gprpp/inlined_vector.h \
src/core/lib/gprpp/manual_constructor.h \
src/core/lib/gprpp/memory.h \
src/core/lib/gprpp/mutex_lock.h \
src/core/lib/gprpp/optional.h \
src/core/lib/gprpp/orphanable.h \
src/core/lib/gprpp/ref_counted.h \
src/core/lib/gprpp/ref_counted_ptr.h \

@ -1167,6 +1167,7 @@ src/core/lib/gprpp/inlined_vector.h \
src/core/lib/gprpp/manual_constructor.h \
src/core/lib/gprpp/memory.h \
src/core/lib/gprpp/mutex_lock.h \
src/core/lib/gprpp/optional.h \
src/core/lib/gprpp/orphanable.h \
src/core/lib/gprpp/ref_counted.h \
src/core/lib/gprpp/ref_counted_ptr.h \

@ -4184,6 +4184,24 @@
"third_party": false,
"type": "target"
},
{
"deps": [
"gpr",
"grpc",
"grpc++",
"grpc++_test",
"grpc_test_util"
],
"headers": [],
"is_filegroup": false,
"language": "c++",
"name": "optional_test",
"src": [
"test/core/gprpp/optional_test.cc"
],
"third_party": false,
"type": "target"
},
{
"deps": [
"gpr",
@ -9556,6 +9574,7 @@
"src/core/lib/debug/stats_data.h",
"src/core/lib/gprpp/debug_location.h",
"src/core/lib/gprpp/inlined_vector.h",
"src/core/lib/gprpp/optional.h",
"src/core/lib/gprpp/orphanable.h",
"src/core/lib/gprpp/ref_counted.h",
"src/core/lib/gprpp/ref_counted_ptr.h",
@ -9709,6 +9728,7 @@
"src/core/lib/debug/stats_data.h",
"src/core/lib/gprpp/debug_location.h",
"src/core/lib/gprpp/inlined_vector.h",
"src/core/lib/gprpp/optional.h",
"src/core/lib/gprpp/orphanable.h",
"src/core/lib/gprpp/ref_counted.h",
"src/core/lib/gprpp/ref_counted_ptr.h",

@ -4887,6 +4887,30 @@
],
"uses_polling": true
},
{
"args": [],
"benchmark": false,
"ci_platforms": [
"linux",
"mac",
"posix",
"windows"
],
"cpu_cost": 1.0,
"exclude_configs": [],
"exclude_iomgrs": [],
"flaky": false,
"gtest": true,
"language": "c++",
"name": "optional_test",
"platforms": [
"linux",
"mac",
"posix",
"windows"
],
"uses_polling": true
},
{
"args": [],
"benchmark": false,

Loading…
Cancel
Save