From a564519bb0d98b1aeb23cb6e46d43da8ecd47f21 Mon Sep 17 00:00:00 2001 From: zyfjeff Date: Mon, 5 Nov 2018 19:47:27 -0800 Subject: [PATCH 01/10] lint: fix readability-redundant-string-cstr Signed-off-by: zyfjeff --- .clang-tidy | 2 +- source/common/filesystem/filesystem_impl.cc | 2 +- source/common/router/config_impl.cc | 2 +- source/extensions/tracers/zipkin/zipkin_core_types.cc | 8 ++++---- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 55d1c2389161..d36ae32a2c04 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,7 +1,7 @@ Checks: 'clang-diagnostic-*,clang-analyzer-*,abseil-*,bugprone-*,modernize-*,performance-*,readability-redundant-*,readability-braces-around-statements' #TODO(lizan): grow this list, fix possible warnings and make more checks as error -WarningsAsErrors: 'bugprone-assert-side-effect,modernize-make-shared,modernize-make-unique,readability-redundant-smartptr-get,readability-braces-around-statements' +WarningsAsErrors: 'bugprone-assert-side-effect,modernize-make-shared,modernize-make-unique,readability-redundant-smartptr-get,readability-braces-around-statements,readability-redundant-string-cstr' CheckOptions: - key: bugprone-assert-side-effect.AssertMacros diff --git a/source/common/filesystem/filesystem_impl.cc b/source/common/filesystem/filesystem_impl.cc index d0e2dea40dae..dbccbddebf51 100644 --- a/source/common/filesystem/filesystem_impl.cc +++ b/source/common/filesystem/filesystem_impl.cc @@ -109,7 +109,7 @@ FileImpl::FileImpl(const std::string& path, Event::Dispatcher& dispatcher, } void FileImpl::open() { - Api::SysCallIntResult result = os_sys_calls_.open(path_.c_str(), O_RDWR | O_APPEND | O_CREAT, + Api::SysCallIntResult result = os_sys_calls_.open(path_, O_RDWR | O_APPEND | O_CREAT, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); fd_ = result.rc_; if (-1 == fd_) { diff --git a/source/common/router/config_impl.cc b/source/common/router/config_impl.cc index ce520bb3f95b..f730fe241579 100644 --- a/source/common/router/config_impl.cc +++ b/source/common/router/config_impl.cc @@ -743,7 +743,7 @@ RegexRouteEntryImpl::RegexRouteEntryImpl(const VirtualHostImpl& vhost, const envoy::api::v2::route::Route& route, Server::Configuration::FactoryContext& factory_context) : RouteEntryImplBase(vhost, route, factory_context), - regex_(RegexUtil::parseRegex(route.match().regex().c_str())), + regex_(RegexUtil::parseRegex(route.match().regex())), regex_str_(route.match().regex()) {} void RegexRouteEntryImpl::rewritePathHeader(Http::HeaderMap& headers, diff --git a/source/extensions/tracers/zipkin/zipkin_core_types.cc b/source/extensions/tracers/zipkin/zipkin_core_types.cc index 028ec2afada4..03a8a18d26f7 100644 --- a/source/extensions/tracers/zipkin/zipkin_core_types.cc +++ b/source/extensions/tracers/zipkin/zipkin_core_types.cc @@ -93,7 +93,7 @@ const std::string Annotation::toJson() { if (endpoint_) { Util::mergeJsons(json_string, static_cast(endpoint_.value()).toJson(), - ZipkinJsonFieldNames::get().ANNOTATION_ENDPOINT.c_str()); + ZipkinJsonFieldNames::get().ANNOTATION_ENDPOINT); } return json_string; @@ -133,7 +133,7 @@ const std::string BinaryAnnotation::toJson() { if (endpoint_) { Util::mergeJsons(json_string, static_cast(endpoint_.value()).toJson(), - ZipkinJsonFieldNames::get().BINARY_ANNOTATION_ENDPOINT.c_str()); + ZipkinJsonFieldNames::get().BINARY_ANNOTATION_ENDPOINT); } return json_string; @@ -207,14 +207,14 @@ const std::string Span::toJson() { annotation_json_vector.push_back(it->toJson()); } Util::addArrayToJson(json_string, annotation_json_vector, - ZipkinJsonFieldNames::get().SPAN_ANNOTATIONS.c_str()); + ZipkinJsonFieldNames::get().SPAN_ANNOTATIONS); std::vector binary_annotation_json_vector; for (auto it = binary_annotations_.begin(); it != binary_annotations_.end(); it++) { binary_annotation_json_vector.push_back(it->toJson()); } Util::addArrayToJson(json_string, binary_annotation_json_vector, - ZipkinJsonFieldNames::get().SPAN_BINARY_ANNOTATIONS.c_str()); + ZipkinJsonFieldNames::get().SPAN_BINARY_ANNOTATIONS); return json_string; } From abe050a8fa4aacf72095c52f22297b1f4669b344 Mon Sep 17 00:00:00 2001 From: "tianqian.zyf" Date: Tue, 6 Nov 2018 16:46:43 +0800 Subject: [PATCH 02/10] Add bugprone-use-after-move to WarningsAsErrors Signed-off-by: tianqian.zyf --- .clang-tidy | 2 +- source/common/grpc/async_client_impl.cc | 2 +- source/common/http/header_map_impl.cc | 14 +++++++------- source/common/stats/thread_local_store.cc | 2 ++ source/common/upstream/health_discovery_service.cc | 5 +++-- test/common/http/header_map_impl_test.cc | 6 +++--- .../tracers/zipkin/zipkin_core_types_test.cc | 2 +- 7 files changed, 18 insertions(+), 15 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index d36ae32a2c04..0794aa66661f 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,7 +1,7 @@ Checks: 'clang-diagnostic-*,clang-analyzer-*,abseil-*,bugprone-*,modernize-*,performance-*,readability-redundant-*,readability-braces-around-statements' #TODO(lizan): grow this list, fix possible warnings and make more checks as error -WarningsAsErrors: 'bugprone-assert-side-effect,modernize-make-shared,modernize-make-unique,readability-redundant-smartptr-get,readability-braces-around-statements,readability-redundant-string-cstr' +WarningsAsErrors: 'bugprone-assert-side-effect,modernize-make-shared,modernize-make-unique,readability-redundant-smartptr-get,readability-braces-around-statements,readability-redundant-string-cstr,bugprone-use-after-move' CheckOptions: - key: bugprone-assert-side-effect.AssertMacros diff --git a/source/common/grpc/async_client_impl.cc b/source/common/grpc/async_client_impl.cc index 278eca02bba2..8b5154dd2254 100644 --- a/source/common/grpc/async_client_impl.cc +++ b/source/common/grpc/async_client_impl.cc @@ -103,7 +103,7 @@ void AsyncStreamImpl::onHeaders(Http::HeaderMapPtr&& headers, bool end_stream) { // https://github.com/grpc/grpc/blob/master/doc/http-grpc-status-mapping.md requires that // grpc-status be used if available. if (end_stream && grpc_status) { - onTrailers(std::move(headers)); + onTrailers(std::move(headers)); // NOLINT(bugprone-use-after-move) return; } // Technically this should be diff --git a/source/common/http/header_map_impl.cc b/source/common/http/header_map_impl.cc index 47f7c137cdd8..566e479ba849 100644 --- a/source/common/http/header_map_impl.cc +++ b/source/common/http/header_map_impl.cc @@ -404,7 +404,7 @@ void HeaderMapImpl::addReferenceKey(const LowerCaseString& key, uint64_t value) HeaderString new_value; new_value.setInteger(value); insertByKey(std::move(ref_key), std::move(new_value)); - ASSERT(new_value.empty()); + ASSERT(new_value.empty()); // NOLINT(bugprone-use-after-move) } void HeaderMapImpl::addReferenceKey(const LowerCaseString& key, const std::string& value) { @@ -412,7 +412,7 @@ void HeaderMapImpl::addReferenceKey(const LowerCaseString& key, const std::strin HeaderString new_value; new_value.setCopy(value.c_str(), value.size()); insertByKey(std::move(ref_key), std::move(new_value)); - ASSERT(new_value.empty()); + ASSERT(new_value.empty()); // NOLINT(bugprone-use-after-move) } void HeaderMapImpl::addCopy(const LowerCaseString& key, uint64_t value) { @@ -428,8 +428,8 @@ void HeaderMapImpl::addCopy(const LowerCaseString& key, uint64_t value) { HeaderString new_value; new_value.setInteger(value); insertByKey(std::move(new_key), std::move(new_value)); - ASSERT(new_key.empty()); - ASSERT(new_value.empty()); + ASSERT(new_key.empty()); // NOLINT(bugprone-use-after-move) + ASSERT(new_value.empty()); // NOLINT(bugprone-use-after-move) } void HeaderMapImpl::addCopy(const LowerCaseString& key, const std::string& value) { @@ -443,8 +443,8 @@ void HeaderMapImpl::addCopy(const LowerCaseString& key, const std::string& value HeaderString new_value; new_value.setCopy(value.c_str(), value.size()); insertByKey(std::move(new_key), std::move(new_value)); - ASSERT(new_key.empty()); - ASSERT(new_value.empty()); + ASSERT(new_key.empty()); // NOLINT(bugprone-use-after-move) + ASSERT(new_value.empty()); // NOLINT(bugprone-use-after-move) } void HeaderMapImpl::setReference(const LowerCaseString& key, const std::string& value) { @@ -460,7 +460,7 @@ void HeaderMapImpl::setReferenceKey(const LowerCaseString& key, const std::strin new_value.setCopy(value.c_str(), value.size()); remove(key); insertByKey(std::move(ref_key), std::move(new_value)); - ASSERT(new_value.empty()); + ASSERT(new_value.empty()); // NOLINT(bugprone-use-after-move) } uint64_t HeaderMapImpl::byteSize() const { diff --git a/source/common/stats/thread_local_store.cc b/source/common/stats/thread_local_store.cc index 000409d92c6e..d682066a81ed 100644 --- a/source/common/stats/thread_local_store.cc +++ b/source/common/stats/thread_local_store.cc @@ -219,6 +219,8 @@ StatType& ThreadLocalStoreImpl::ScopeImpl::safeMakeStat( make_stat(parent_.alloc_, truncated_name, std::move(tag_extracted_name), std::move(tags)); if (stat == nullptr) { parent_.num_last_resort_stats_.inc(); + std::vector tags; + std::string tag_extracted_name = parent_.getTagsForName(name, tags); stat = make_stat(parent_.heap_allocator_, truncated_name, std::move(tag_extracted_name), std::move(tags)); ASSERT(stat != nullptr); diff --git a/source/common/upstream/health_discovery_service.cc b/source/common/upstream/health_discovery_service.cc index fcfc9238bb1b..cb0e2363c496 100644 --- a/source/common/upstream/health_discovery_service.cc +++ b/source/common/upstream/health_discovery_service.cc @@ -160,11 +160,12 @@ void HdsDelegate::onReceiveMessage( // Reset hds_clusters_.clear(); + // Set response + server_response_ms_ = PROTOBUF_GET_MS_REQUIRED(*message, interval); + // Process the HealthCheckSpecifier message processMessage(std::move(message)); - // Set response - server_response_ms_ = PROTOBUF_GET_MS_REQUIRED(*message, interval); setHdsStreamResponseTimer(); } diff --git a/test/common/http/header_map_impl_test.cc b/test/common/http/header_map_impl_test.cc index 5b69e165ed0d..d82815c3950b 100644 --- a/test/common/http/header_map_impl_test.cc +++ b/test/common/http/header_map_impl_test.cc @@ -38,7 +38,7 @@ TEST(HeaderStringTest, All) { HeaderString string1(static_string); HeaderString string2(std::move(string1)); EXPECT_STREQ("HELLO", string2.c_str()); - EXPECT_EQ(static_string.c_str(), string1.c_str()); + EXPECT_EQ(static_string.c_str(), string1.c_str()); // NOLINT(bugprone-use-after-move) EXPECT_EQ(static_string.c_str(), string2.c_str()); EXPECT_EQ(5U, string1.size()); EXPECT_EQ(5U, string2.size()); @@ -50,7 +50,7 @@ TEST(HeaderStringTest, All) { string.setCopy("hello", 5); EXPECT_EQ(HeaderString::Type::Inline, string.type()); HeaderString string2(std::move(string)); - EXPECT_TRUE(string.empty()); + EXPECT_TRUE(string.empty()); // NOLINT(bugprone-use-after-move) EXPECT_EQ(HeaderString::Type::Inline, string.type()); EXPECT_EQ(HeaderString::Type::Inline, string2.type()); string.append("world", 5); @@ -67,7 +67,7 @@ TEST(HeaderStringTest, All) { string.setCopy(large.c_str(), large.size()); EXPECT_EQ(HeaderString::Type::Dynamic, string.type()); HeaderString string2(std::move(string)); - EXPECT_TRUE(string.empty()); + EXPECT_TRUE(string.empty()); // NOLINT(bugprone-use-after-move) EXPECT_EQ(HeaderString::Type::Inline, string.type()); EXPECT_EQ(HeaderString::Type::Dynamic, string2.type()); string.append("b", 1); diff --git a/test/extensions/tracers/zipkin/zipkin_core_types_test.cc b/test/extensions/tracers/zipkin/zipkin_core_types_test.cc index 239e5f28738c..bcc25f14523b 100644 --- a/test/extensions/tracers/zipkin/zipkin_core_types_test.cc +++ b/test/extensions/tracers/zipkin/zipkin_core_types_test.cc @@ -449,7 +449,7 @@ TEST(ZipkinCoreTypesSpanTest, defaultConstructor) { // Test setSourceServiceName and setDestinationServiceName - ann.setValue(Zipkin::ZipkinCoreConstants::get().CLIENT_RECV); + ann.setValue(Zipkin::ZipkinCoreConstants::get().CLIENT_RECV); // NOLINT(bugprone-use-after-move) span.addAnnotation(ann); span.setServiceName("NEW_SERVICE_NAME"); EXPECT_EQ(R"({"traceId":")" + span.traceIdAsHexString() + R"(","name":"span_name","id":")" + From e2746b0e9f9b82c017729cb0d905e989884ae898 Mon Sep 17 00:00:00 2001 From: zyfjeff Date: Tue, 6 Nov 2018 01:44:24 -0800 Subject: [PATCH 03/10] Fix format Signed-off-by: zyfjeff --- source/common/filesystem/filesystem_impl.cc | 4 ++-- source/common/http/header_map_impl.cc | 14 +++++++------- source/common/router/config_impl.cc | 3 +-- test/common/http/header_map_impl_test.cc | 6 +++--- .../tracers/zipkin/zipkin_core_types_test.cc | 2 +- 5 files changed, 14 insertions(+), 15 deletions(-) diff --git a/source/common/filesystem/filesystem_impl.cc b/source/common/filesystem/filesystem_impl.cc index dbccbddebf51..90d44982026e 100644 --- a/source/common/filesystem/filesystem_impl.cc +++ b/source/common/filesystem/filesystem_impl.cc @@ -109,8 +109,8 @@ FileImpl::FileImpl(const std::string& path, Event::Dispatcher& dispatcher, } void FileImpl::open() { - Api::SysCallIntResult result = os_sys_calls_.open(path_, O_RDWR | O_APPEND | O_CREAT, - S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); + Api::SysCallIntResult result = + os_sys_calls_.open(path_, O_RDWR | O_APPEND | O_CREAT, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); fd_ = result.rc_; if (-1 == fd_) { throw EnvoyException( diff --git a/source/common/http/header_map_impl.cc b/source/common/http/header_map_impl.cc index 566e479ba849..281255c0c0e1 100644 --- a/source/common/http/header_map_impl.cc +++ b/source/common/http/header_map_impl.cc @@ -404,7 +404,7 @@ void HeaderMapImpl::addReferenceKey(const LowerCaseString& key, uint64_t value) HeaderString new_value; new_value.setInteger(value); insertByKey(std::move(ref_key), std::move(new_value)); - ASSERT(new_value.empty()); // NOLINT(bugprone-use-after-move) + ASSERT(new_value.empty()); // NOLINT(bugprone-use-after-move) } void HeaderMapImpl::addReferenceKey(const LowerCaseString& key, const std::string& value) { @@ -412,7 +412,7 @@ void HeaderMapImpl::addReferenceKey(const LowerCaseString& key, const std::strin HeaderString new_value; new_value.setCopy(value.c_str(), value.size()); insertByKey(std::move(ref_key), std::move(new_value)); - ASSERT(new_value.empty()); // NOLINT(bugprone-use-after-move) + ASSERT(new_value.empty()); // NOLINT(bugprone-use-after-move) } void HeaderMapImpl::addCopy(const LowerCaseString& key, uint64_t value) { @@ -428,8 +428,8 @@ void HeaderMapImpl::addCopy(const LowerCaseString& key, uint64_t value) { HeaderString new_value; new_value.setInteger(value); insertByKey(std::move(new_key), std::move(new_value)); - ASSERT(new_key.empty()); // NOLINT(bugprone-use-after-move) - ASSERT(new_value.empty()); // NOLINT(bugprone-use-after-move) + ASSERT(new_key.empty()); // NOLINT(bugprone-use-after-move) + ASSERT(new_value.empty()); // NOLINT(bugprone-use-after-move) } void HeaderMapImpl::addCopy(const LowerCaseString& key, const std::string& value) { @@ -443,8 +443,8 @@ void HeaderMapImpl::addCopy(const LowerCaseString& key, const std::string& value HeaderString new_value; new_value.setCopy(value.c_str(), value.size()); insertByKey(std::move(new_key), std::move(new_value)); - ASSERT(new_key.empty()); // NOLINT(bugprone-use-after-move) - ASSERT(new_value.empty()); // NOLINT(bugprone-use-after-move) + ASSERT(new_key.empty()); // NOLINT(bugprone-use-after-move) + ASSERT(new_value.empty()); // NOLINT(bugprone-use-after-move) } void HeaderMapImpl::setReference(const LowerCaseString& key, const std::string& value) { @@ -460,7 +460,7 @@ void HeaderMapImpl::setReferenceKey(const LowerCaseString& key, const std::strin new_value.setCopy(value.c_str(), value.size()); remove(key); insertByKey(std::move(ref_key), std::move(new_value)); - ASSERT(new_value.empty()); // NOLINT(bugprone-use-after-move) + ASSERT(new_value.empty()); // NOLINT(bugprone-use-after-move) } uint64_t HeaderMapImpl::byteSize() const { diff --git a/source/common/router/config_impl.cc b/source/common/router/config_impl.cc index f730fe241579..d0af5bf0829b 100644 --- a/source/common/router/config_impl.cc +++ b/source/common/router/config_impl.cc @@ -743,8 +743,7 @@ RegexRouteEntryImpl::RegexRouteEntryImpl(const VirtualHostImpl& vhost, const envoy::api::v2::route::Route& route, Server::Configuration::FactoryContext& factory_context) : RouteEntryImplBase(vhost, route, factory_context), - regex_(RegexUtil::parseRegex(route.match().regex())), - regex_str_(route.match().regex()) {} + regex_(RegexUtil::parseRegex(route.match().regex())), regex_str_(route.match().regex()) {} void RegexRouteEntryImpl::rewritePathHeader(Http::HeaderMap& headers, bool insert_envoy_original_path) const { diff --git a/test/common/http/header_map_impl_test.cc b/test/common/http/header_map_impl_test.cc index d82815c3950b..19d6cec7f27f 100644 --- a/test/common/http/header_map_impl_test.cc +++ b/test/common/http/header_map_impl_test.cc @@ -38,7 +38,7 @@ TEST(HeaderStringTest, All) { HeaderString string1(static_string); HeaderString string2(std::move(string1)); EXPECT_STREQ("HELLO", string2.c_str()); - EXPECT_EQ(static_string.c_str(), string1.c_str()); // NOLINT(bugprone-use-after-move) + EXPECT_EQ(static_string.c_str(), string1.c_str()); // NOLINT(bugprone-use-after-move) EXPECT_EQ(static_string.c_str(), string2.c_str()); EXPECT_EQ(5U, string1.size()); EXPECT_EQ(5U, string2.size()); @@ -50,7 +50,7 @@ TEST(HeaderStringTest, All) { string.setCopy("hello", 5); EXPECT_EQ(HeaderString::Type::Inline, string.type()); HeaderString string2(std::move(string)); - EXPECT_TRUE(string.empty()); // NOLINT(bugprone-use-after-move) + EXPECT_TRUE(string.empty()); // NOLINT(bugprone-use-after-move) EXPECT_EQ(HeaderString::Type::Inline, string.type()); EXPECT_EQ(HeaderString::Type::Inline, string2.type()); string.append("world", 5); @@ -67,7 +67,7 @@ TEST(HeaderStringTest, All) { string.setCopy(large.c_str(), large.size()); EXPECT_EQ(HeaderString::Type::Dynamic, string.type()); HeaderString string2(std::move(string)); - EXPECT_TRUE(string.empty()); // NOLINT(bugprone-use-after-move) + EXPECT_TRUE(string.empty()); // NOLINT(bugprone-use-after-move) EXPECT_EQ(HeaderString::Type::Inline, string.type()); EXPECT_EQ(HeaderString::Type::Dynamic, string2.type()); string.append("b", 1); diff --git a/test/extensions/tracers/zipkin/zipkin_core_types_test.cc b/test/extensions/tracers/zipkin/zipkin_core_types_test.cc index bcc25f14523b..a534e6b6e9c0 100644 --- a/test/extensions/tracers/zipkin/zipkin_core_types_test.cc +++ b/test/extensions/tracers/zipkin/zipkin_core_types_test.cc @@ -449,7 +449,7 @@ TEST(ZipkinCoreTypesSpanTest, defaultConstructor) { // Test setSourceServiceName and setDestinationServiceName - ann.setValue(Zipkin::ZipkinCoreConstants::get().CLIENT_RECV); // NOLINT(bugprone-use-after-move) + ann.setValue(Zipkin::ZipkinCoreConstants::get().CLIENT_RECV); // NOLINT(bugprone-use-after-move) span.addAnnotation(ann); span.setServiceName("NEW_SERVICE_NAME"); EXPECT_EQ(R"({"traceId":")" + span.traceIdAsHexString() + R"(","name":"span_name","id":")" + From 2ce87642946d1158ff1d5df212a80a0429b246f5 Mon Sep 17 00:00:00 2001 From: "tianqian.zyf" Date: Sat, 10 Nov 2018 21:04:46 +0800 Subject: [PATCH 04/10] Address code review Signed-off-by: tianqian.zyf --- source/common/grpc/async_client_impl.cc | 2 ++ source/common/stats/thread_local_store.cc | 8 +++++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/source/common/grpc/async_client_impl.cc b/source/common/grpc/async_client_impl.cc index 8b5154dd2254..8e306f7b6e19 100644 --- a/source/common/grpc/async_client_impl.cc +++ b/source/common/grpc/async_client_impl.cc @@ -103,6 +103,8 @@ void AsyncStreamImpl::onHeaders(Http::HeaderMapPtr&& headers, bool end_stream) { // https://github.com/grpc/grpc/blob/master/doc/http-grpc-status-mapping.md requires that // grpc-status be used if available. if (end_stream && grpc_status) { + // There is actually no use-after-move problem here, + // because it will only be executed when end_stream is equal to true. onTrailers(std::move(headers)); // NOLINT(bugprone-use-after-move) return; } diff --git a/source/common/stats/thread_local_store.cc b/source/common/stats/thread_local_store.cc index d682066a81ed..7f751f7c8152 100644 --- a/source/common/stats/thread_local_store.cc +++ b/source/common/stats/thread_local_store.cc @@ -216,11 +216,13 @@ StatType& ThreadLocalStoreImpl::ScopeImpl::safeMakeStat( std::string tag_extracted_name = parent_.getTagsForName(name, tags); absl::string_view truncated_name = parent_.truncateStatNameIfNeeded(name); std::shared_ptr stat = - make_stat(parent_.alloc_, truncated_name, std::move(tag_extracted_name), std::move(tags)); + make_stat(parent_.alloc_, truncated_name, std::move(tag_extracted_name), + std::move(tags)); // NOLINT(bugprone-use-after-move) if (stat == nullptr) { + // TODO(jmarantz): If make_stat fails, the actual move does not actually occur + // for tag_extracted_name and tags, so there is no use-after-move problem. + // In order to increase the readability of the code, refactoring is done here. parent_.num_last_resort_stats_.inc(); - std::vector tags; - std::string tag_extracted_name = parent_.getTagsForName(name, tags); stat = make_stat(parent_.heap_allocator_, truncated_name, std::move(tag_extracted_name), std::move(tags)); ASSERT(stat != nullptr); From 3293c27911d8ca94a7918ee949895a579896ba71 Mon Sep 17 00:00:00 2001 From: "tianqian.zyf" Date: Sat, 10 Nov 2018 22:57:22 +0800 Subject: [PATCH 05/10] Fix clang-tidy error Signed-off-by: tianqian.zyf --- source/common/stats/thread_local_store.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/source/common/stats/thread_local_store.cc b/source/common/stats/thread_local_store.cc index 7f751f7c8152..a8bd46db8875 100644 --- a/source/common/stats/thread_local_store.cc +++ b/source/common/stats/thread_local_store.cc @@ -217,14 +217,14 @@ StatType& ThreadLocalStoreImpl::ScopeImpl::safeMakeStat( absl::string_view truncated_name = parent_.truncateStatNameIfNeeded(name); std::shared_ptr stat = make_stat(parent_.alloc_, truncated_name, std::move(tag_extracted_name), - std::move(tags)); // NOLINT(bugprone-use-after-move) + std::move(tags)); if (stat == nullptr) { // TODO(jmarantz): If make_stat fails, the actual move does not actually occur // for tag_extracted_name and tags, so there is no use-after-move problem. // In order to increase the readability of the code, refactoring is done here. parent_.num_last_resort_stats_.inc(); - stat = make_stat(parent_.heap_allocator_, truncated_name, std::move(tag_extracted_name), - std::move(tags)); + stat = make_stat(parent_.heap_allocator_, truncated_name, + std::move(tag_extracted_name), std::move(tags)); // NOLINT(bugprone-use-after-move) ASSERT(stat != nullptr); } central_ref = ¢ral_cache_map[stat->nameCStr()]; From e5dd48a5d50eb4d01a52609fbaee2fc1bb8ef9e1 Mon Sep 17 00:00:00 2001 From: "tianqian.zyf" Date: Sat, 10 Nov 2018 23:10:40 +0800 Subject: [PATCH 06/10] Kick CI Signed-off-by: tianqian.zyf From 6214424641e6c5523919e6aad437d05966f3eeb6 Mon Sep 17 00:00:00 2001 From: "tianqian.zyf" Date: Sat, 10 Nov 2018 23:41:06 +0800 Subject: [PATCH 07/10] Kick CI Signed-off-by: tianqian.zyf From 353bc0a832240b4e1fb21a050e0b35ce6d36ac31 Mon Sep 17 00:00:00 2001 From: zyfjeff Date: Sat, 10 Nov 2018 07:45:42 -0800 Subject: [PATCH 08/10] Fix format Signed-off-by: zyfjeff --- source/common/stats/thread_local_store.cc | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/source/common/stats/thread_local_store.cc b/source/common/stats/thread_local_store.cc index a8bd46db8875..e48885e456a3 100644 --- a/source/common/stats/thread_local_store.cc +++ b/source/common/stats/thread_local_store.cc @@ -216,15 +216,14 @@ StatType& ThreadLocalStoreImpl::ScopeImpl::safeMakeStat( std::string tag_extracted_name = parent_.getTagsForName(name, tags); absl::string_view truncated_name = parent_.truncateStatNameIfNeeded(name); std::shared_ptr stat = - make_stat(parent_.alloc_, truncated_name, std::move(tag_extracted_name), - std::move(tags)); + make_stat(parent_.alloc_, truncated_name, std::move(tag_extracted_name), std::move(tags)); if (stat == nullptr) { // TODO(jmarantz): If make_stat fails, the actual move does not actually occur // for tag_extracted_name and tags, so there is no use-after-move problem. // In order to increase the readability of the code, refactoring is done here. parent_.num_last_resort_stats_.inc(); - stat = make_stat(parent_.heap_allocator_, truncated_name, - std::move(tag_extracted_name), std::move(tags)); // NOLINT(bugprone-use-after-move) + stat = make_stat(parent_.heap_allocator_, truncated_name, std::move(tag_extracted_name), + std::move(tags)); // NOLINT(bugprone-use-after-move) ASSERT(stat != nullptr); } central_ref = ¢ral_cache_map[stat->nameCStr()]; From 6a6313166490e763dabcafbaa67c2a13cd61c463 Mon Sep 17 00:00:00 2001 From: "tianqian.zyf" Date: Sun, 11 Nov 2018 10:13:32 +0800 Subject: [PATCH 09/10] Kick CI Signed-off-by: tianqian.zyf --- source/common/stats/thread_local_store.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/common/stats/thread_local_store.cc b/source/common/stats/thread_local_store.cc index e48885e456a3..568338aa7e43 100644 --- a/source/common/stats/thread_local_store.cc +++ b/source/common/stats/thread_local_store.cc @@ -222,7 +222,7 @@ StatType& ThreadLocalStoreImpl::ScopeImpl::safeMakeStat( // for tag_extracted_name and tags, so there is no use-after-move problem. // In order to increase the readability of the code, refactoring is done here. parent_.num_last_resort_stats_.inc(); - stat = make_stat(parent_.heap_allocator_, truncated_name, std::move(tag_extracted_name), + stat = make_stat(parent_.heap_allocator_, truncated_name, std::move(tag_extracted_name), // NOLINT(bugprone-use-after-move) std::move(tags)); // NOLINT(bugprone-use-after-move) ASSERT(stat != nullptr); } From 221ca4a2fbe2345b2d6c13696061740c82841c0b Mon Sep 17 00:00:00 2001 From: zyfjeff Date: Sun, 11 Nov 2018 00:38:25 -0800 Subject: [PATCH 10/10] Fix format Signed-off-by: zyfjeff --- source/common/stats/thread_local_store.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/source/common/stats/thread_local_store.cc b/source/common/stats/thread_local_store.cc index 568338aa7e43..672662261689 100644 --- a/source/common/stats/thread_local_store.cc +++ b/source/common/stats/thread_local_store.cc @@ -222,8 +222,9 @@ StatType& ThreadLocalStoreImpl::ScopeImpl::safeMakeStat( // for tag_extracted_name and tags, so there is no use-after-move problem. // In order to increase the readability of the code, refactoring is done here. parent_.num_last_resort_stats_.inc(); - stat = make_stat(parent_.heap_allocator_, truncated_name, std::move(tag_extracted_name), // NOLINT(bugprone-use-after-move) - std::move(tags)); // NOLINT(bugprone-use-after-move) + stat = make_stat(parent_.heap_allocator_, truncated_name, + std::move(tag_extracted_name), // NOLINT(bugprone-use-after-move) + std::move(tags)); // NOLINT(bugprone-use-after-move) ASSERT(stat != nullptr); } central_ref = ¢ral_cache_map[stat->nameCStr()];