clang-tidy: performance-for-range-copy (#7349)

Signed-off-by: Derek Argueta <dereka@pinterest.com>

.clang-tidy

@@ -1,7 +1,7 @@
 Checks:          'clang-diagnostic-*,clang-analyzer-*,abseil-*,bugprone-*,modernize-*,performance-*,readability-redundant-*,readability-braces-around-statements,readability-container-size-empty'
 
 #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,bugprone-use-after-move,readability-container-size-empty,performance-faster-string-find'
+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,readability-container-size-empty,performance-faster-string-find,performance-for-range-copy'
 
 CheckOptions:
   - key:             bugprone-assert-side-effect.AssertMacros

source/common/config/filter_json.cc

@@ -284,14 +284,14 @@ void FilterJson::translateFaultFilter(
     JSON_UTIL_SET_DURATION_FROM_FIELD(*json_config_delay, *delay, fixed_delay, fixed_duration);
   }
 
-  for (const auto json_header_matcher : json_config.getObjectArray("headers", true)) {
+  for (const auto& json_header_matcher : json_config.getObjectArray("headers", true)) {
     auto* header_matcher = proto_config.mutable_headers()->Add();
     RdsJson::translateHeaderMatcher(*json_header_matcher, *header_matcher);
   }
 
   JSON_UTIL_SET_STRING(json_config, proto_config, upstream_cluster);
 
-  for (auto json_downstream_node : json_config.getStringArray("downstream_nodes", true)) {
+  for (const auto& json_downstream_node : json_config.getStringArray("downstream_nodes", true)) {
     auto* downstream_node = proto_config.mutable_downstream_nodes()->Add();
     *downstream_node = json_downstream_node;
   }

source/common/config/grpc_mux_impl.cc

@@ -175,7 +175,7 @@ void GrpcMuxImpl::onDiscoveryResponse(
         continue;
       }
       Protobuf::RepeatedPtrField<ProtobufWkt::Any> found_resources;
-      for (auto watched_resource_name : watch->resources_) {
+      for (const auto& watched_resource_name : watch->resources_) {
         auto it = resources.find(watched_resource_name);
         if (it != resources.end()) {
           found_resources.Add()->MergeFrom(it->second);
@@ -205,7 +205,7 @@ void GrpcMuxImpl::onDiscoveryResponse(
 void GrpcMuxImpl::onWriteable() { drainRequests(); }
 
 void GrpcMuxImpl::onStreamEstablished() {
-  for (const auto type_url : subscriptions_) {
+  for (const auto& type_url : subscriptions_) {
     queueDiscoveryRequest(type_url);
   }
 }

source/common/config/rds_json.cc

@@ -56,7 +56,7 @@ void RdsJson::translateRateLimit(const Json::Object& json_rate_limit,
   JSON_UTIL_SET_INTEGER(json_rate_limit, rate_limit, stage);
   JSON_UTIL_SET_STRING(json_rate_limit, rate_limit, disable_key);
   const auto actions = json_rate_limit.getObjectArray("actions");
-  for (const auto json_action : actions) {
+  for (const auto& json_action : actions) {
     auto* action = rate_limit.mutable_actions()->Add();
     const std::string type = json_action->getString("type");
     if (type == "source_cluster") {
@@ -119,7 +119,7 @@ void RdsJson::translateRouteConfiguration(const Json::Object& json_route_config,
                                           envoy::api::v2::RouteConfiguration& route_config) {
   json_route_config.validateSchema(Json::Schema::ROUTE_CONFIGURATION_SCHEMA);
 
-  for (const auto json_virtual_host : json_route_config.getObjectArray("virtual_hosts", true)) {
+  for (const auto& json_virtual_host : json_route_config.getObjectArray("virtual_hosts", true)) {
     auto* virtual_host = route_config.mutable_virtual_hosts()->Add();
     translateVirtualHost(*json_virtual_host, *virtual_host);
   }
@@ -129,7 +129,7 @@ void RdsJson::translateRouteConfiguration(const Json::Object& json_route_config,
     route_config.add_internal_only_headers(header);
   }
 
-  for (const auto header_value :
+  for (const auto& header_value :
        json_route_config.getObjectArray("response_headers_to_add", true)) {
     auto* header_value_option = route_config.mutable_response_headers_to_add()->Add();
     BaseJson::translateHeaderValueOption(*header_value, *header_value_option);
@@ -140,7 +140,8 @@ void RdsJson::translateRouteConfiguration(const Json::Object& json_route_config,
     route_config.add_response_headers_to_remove(header);
   }
 
-  for (const auto header_value : json_route_config.getObjectArray("request_headers_to_add", true)) {
+  for (const auto& header_value :
+       json_route_config.getObjectArray("request_headers_to_add", true)) {
     auto* header_value_option = route_config.mutable_request_headers_to_add()->Add();
     BaseJson::translateHeaderValueOption(*header_value, *header_value_option);
   }
@@ -159,7 +160,7 @@ void RdsJson::translateVirtualHost(const Json::Object& json_virtual_host,
     virtual_host.add_domains(domain);
   }
 
-  for (const auto json_route : json_virtual_host.getObjectArray("routes", true)) {
+  for (const auto& json_route : json_virtual_host.getObjectArray("routes", true)) {
     auto* route = virtual_host.mutable_routes()->Add();
     translateRoute(*json_route, *route);
   }
@@ -169,18 +170,19 @@ void RdsJson::translateVirtualHost(const Json::Object& json_virtual_host,
       StringUtil::toUpper(json_virtual_host.getString("require_ssl", "")), &tls_requirement);
   virtual_host.set_require_tls(tls_requirement);
 
-  for (const auto json_virtual_cluster :
+  for (const auto& json_virtual_cluster :
        json_virtual_host.getObjectArray("virtual_clusters", true)) {
     auto* virtual_cluster = virtual_host.mutable_virtual_clusters()->Add();
     translateVirtualCluster(*json_virtual_cluster, *virtual_cluster);
   }
 
-  for (const auto json_rate_limit : json_virtual_host.getObjectArray("rate_limits", true)) {
+  for (const auto& json_rate_limit : json_virtual_host.getObjectArray("rate_limits", true)) {
     auto* rate_limit = virtual_host.mutable_rate_limits()->Add();
     translateRateLimit(*json_rate_limit, *rate_limit);
   }
 
-  for (const auto header_value : json_virtual_host.getObjectArray("request_headers_to_add", true)) {
+  for (const auto& header_value :
+       json_virtual_host.getObjectArray("request_headers_to_add", true)) {
     auto* header_value_option = virtual_host.mutable_request_headers_to_add()->Add();
     BaseJson::translateHeaderValueOption(*header_value, *header_value_option);
   }
@@ -226,12 +228,12 @@ void RdsJson::translateRoute(const Json::Object& json_route, envoy::api::v2::rou
                                        *match->mutable_runtime_fraction());
   }
 
-  for (const auto json_header_matcher : json_route.getObjectArray("headers", true)) {
+  for (const auto& json_header_matcher : json_route.getObjectArray("headers", true)) {
     auto* header_matcher = match->mutable_headers()->Add();
     translateHeaderMatcher(*json_header_matcher, *header_matcher);
   }
 
-  for (const auto json_query_parameter_matcher :
+  for (const auto& json_query_parameter_matcher :
        json_route.getObjectArray("query_parameters", true)) {
     auto* query_parameter_matcher = match->mutable_query_parameters()->Add();
     translateQueryParameterMatcher(*json_query_parameter_matcher, *query_parameter_matcher);
@@ -308,12 +310,12 @@ void RdsJson::translateRoute(const Json::Object& json_route, envoy::api::v2::rou
                           &priority);
     action->set_priority(priority);
 
-    for (const auto header_value : json_route.getObjectArray("request_headers_to_add", true)) {
+    for (const auto& header_value : json_route.getObjectArray("request_headers_to_add", true)) {
       auto* header_value_option = route.mutable_request_headers_to_add()->Add();
       BaseJson::translateHeaderValueOption(*header_value, *header_value_option);
     }
 
-    for (const auto json_rate_limit : json_route.getObjectArray("rate_limits", true)) {
+    for (const auto& json_rate_limit : json_route.getObjectArray("rate_limits", true)) {
       auto* rate_limit = action->mutable_rate_limits()->Add();
       translateRateLimit(*json_rate_limit, *rate_limit);
     }

source/common/grpc/google_async_client_impl.cc

@@ -348,7 +348,7 @@ void GoogleAsyncStreamImpl::metadataTranslate(
     Http::HeaderMap& header_map) {
   // More painful copying, this time due to the mismatch in header
   // representation data structures in Envoy and Google gRPC.
-  for (auto it : grpc_metadata) {
+  for (const auto& it : grpc_metadata) {
     header_map.addCopy(Http::LowerCaseString(std::string(it.first.data(), it.first.size())),
                        std::string(it.second.data(), it.second.size()));
   }

source/common/http/utility.cc

@@ -422,7 +422,7 @@ MessagePtr Utility::prepareHeaders(const ::envoy::api::v2::core::HttpUri& http_u
 std::string Utility::queryParamsToString(const QueryParams& params) {
   std::string out;
   std::string delim = "?";
-  for (auto p : params) {
+  for (const auto& p : params) {
     absl::StrAppend(&out, delim, p.first, "=", p.second);
     delim = "&";
   }

source/common/router/config_impl.cc

@@ -462,7 +462,7 @@ RouteEntryImplBase::RouteEntryImplBase(const VirtualHostImpl& vhost,
     cors_policy_ =
         std::make_unique<CorsPolicyImpl>(route.route().cors(), factory_context.runtime());
   }
-  for (const auto upgrade_config : route.route().upgrade_configs()) {
+  for (const auto& upgrade_config : route.route().upgrade_configs()) {
     const bool enabled = upgrade_config.has_enabled() ? upgrade_config.enabled().value() : true;
     const bool success =
         upgrade_map_

source/common/router/metadatamatchcriteria_impl.cc

@@ -20,7 +20,7 @@ MetadataMatchCriteriaImpl::extractMetadataMatchCriteria(const MetadataMatchCrite
   }
 
   // Add values from matches, replacing name/values copied from parent.
-  for (const auto it : matches.fields()) {
+  for (const auto& it : matches.fields()) {
     const auto index_it = existing.find(it.first);
     if (index_it != existing.end()) {
       v[index_it->second] = std::make_shared<MetadataMatchCriterionImpl>(it.first, it.second);

source/common/upstream/cds_api_impl.cc

@@ -81,7 +81,7 @@ void CdsApiImpl::onConfigUpdate(
       exception_msgs.push_back(fmt::format("{}: {}", cluster.name(), e.what()));
     }
   }
-  for (auto resource_name : removed_resources) {
+  for (const auto& resource_name : removed_resources) {
     if (cm_.removeCluster(resource_name)) {
       any_applied = true;
       ENVOY_LOG(debug, "cds: remove cluster '{}'", resource_name);

source/common/upstream/load_stats_reporter.cc

@@ -64,7 +64,7 @@ void LoadStatsReporter::sendLoadStatsRequest() {
         uint64_t rq_error = 0;
         uint64_t rq_active = 0;
         uint64_t rq_issued = 0;
-        for (auto host : hosts) {
+        for (const auto& host : hosts) {
           rq_success += host->stats().rq_success_.latch();
           rq_error += host->stats().rq_error_.latch();
           rq_active += host->stats().rq_active_.value();
@@ -154,7 +154,7 @@ void LoadStatsReporter::startLoadReportPeriod() {
     }
     auto& cluster = it->second.get();
     for (auto& host_set : cluster.prioritySet().hostSetsPerPriority()) {
-      for (auto host : host_set->hosts()) {
+      for (const auto& host : host_set->hosts()) {
         host->stats().rq_success_.latch();
         host->stats().rq_error_.latch();
         host->stats().rq_total_.latch();

source/common/upstream/outlier_detection_impl.cc

@@ -140,7 +140,7 @@ DetectorImpl::DetectorImpl(const Cluster& cluster,
 }
 
 DetectorImpl::~DetectorImpl() {
-  for (auto host : host_monitors_) {
+  for (const auto& host : host_monitors_) {
     if (host.first->healthFlagGet(Host::HealthFlag::FAILED_OUTLIER_CHECK)) {
       ASSERT(stats_.ejections_active_.value() > 0);
       stats_.ejections_active_.dec();

source/common/upstream/subset_lb.cc

@@ -472,7 +472,7 @@ SubsetLoadBalancer::extractSubsetMetadata(const std::set<std::string>& subset_ke
   }
 
   const auto& fields = filter_it->second.fields();
-  for (const auto key : subset_keys) {
+  for (const auto& key : subset_keys) {
     const auto it = fields.find(key);
     if (it == fields.end()) {
       break;

source/extensions/filters/network/http_connection_manager/config.cc

@@ -311,7 +311,7 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig(
     processFilter(filters[i], i, "http", filter_factories_);
   }
 
-  for (auto upgrade_config : config.upgrade_configs()) {
+  for (const auto& upgrade_config : config.upgrade_configs()) {
     const std::string& name = upgrade_config.upgrade_type();
     const bool enabled = upgrade_config.has_enabled() ? upgrade_config.enabled().value() : true;
     if (findUpgradeCaseInsensitive(upgrade_filter_factories_, name) !=

source/extensions/transport_sockets/tls/context_impl.cc

@@ -191,7 +191,7 @@ ContextImpl::ContextImpl(Stats::Scope& scope, const Envoy::Ssl::ContextConfig& c
 
   if (config.certificateValidationContext() != nullptr &&
       !config.certificateValidationContext()->verifyCertificateSpkiList().empty()) {
-    for (auto hash : config.certificateValidationContext()->verifyCertificateSpkiList()) {
+    for (const auto& hash : config.certificateValidationContext()->verifyCertificateSpkiList()) {
       const auto decoded = Base64::decode(hash);
       if (decoded.size() != SHA256_DIGEST_LENGTH) {
         throw EnvoyException(fmt::format("Invalid base64-encoded SHA-256 {}", hash));

source/server/http/admin.cc

@@ -433,7 +433,7 @@ void AdminImpl::writeClustersAsText(Buffer::Instance& response) {
           all_stats[gauge->name()] = gauge->value();
         }
 
-        for (auto stat : all_stats) {
+        for (const auto& stat : all_stats) {
           response.add(fmt::format("{}::{}::{}::{}\n", cluster.second.get().info()->name(),
                                    host->address()->asString(), stat.first, stat.second));
         }
@@ -757,7 +757,7 @@ Http::Code AdminImpl::handlerStats(absl::string_view url, Http::HeaderMap& respo
       rc = Http::Code::NotFound;
     }
   } else { // Display plain stats if format query param is not there.
-    for (auto stat : all_stats) {
+    for (const auto& stat : all_stats) {
       response.add(fmt::format("{}: {}\n", stat.first, stat.second));
     }
     // TODO(ramaraochavali): See the comment in ThreadLocalStoreImpl::histograms() for why we use a
@@ -769,7 +769,7 @@ Http::Code AdminImpl::handlerStats(absl::string_view url, Http::HeaderMap& respo
         all_histograms.emplace(histogram->name(), histogram->quantileSummary());
       }
     }
-    for (auto histogram : all_histograms) {
+    for (const auto& histogram : all_histograms) {
       response.add(fmt::format("{}: {}\n", histogram.first, histogram.second));
     }
   }
@@ -894,7 +894,7 @@ AdminImpl::statsAsJson(const std::map<std::string, uint64_t>& all_stats,
   document.SetObject();
   rapidjson::Value stats_array(rapidjson::kArrayType);
   rapidjson::Document::AllocatorType& allocator = document.GetAllocator();
-  for (auto stat : all_stats) {
+  for (const auto& stat : all_stats) {
     Value stat_obj;
     stat_obj.SetObject();
     Value stat_name;

test/common/filesystem/directory_test.cc

@@ -75,13 +75,13 @@ using EntrySet = std::unordered_set<DirectoryEntry, EntryHash>;
 EntrySet getDirectoryContents(const std::string& dir_path, bool recursive) {
   Directory directory(dir_path);
   EntrySet ret;
-  for (const DirectoryEntry entry : directory) {
+  for (const DirectoryEntry& entry : directory) {
     ret.insert(entry);
     if (entry.type_ == FileType::Directory && entry.name_ != "." && entry.name_ != ".." &&
         recursive) {
       std::string subdir_name = entry.name_;
       EntrySet subdir = getDirectoryContents(dir_path + "/" + subdir_name, recursive);
-      for (const DirectoryEntry entry : subdir) {
+      for (const DirectoryEntry& entry : subdir) {
         ret.insert({subdir_name + "/" + entry.name_, entry.type_});
       }
     }

test/common/upstream/upstream_impl_test.cc

@@ -686,7 +686,7 @@ TEST_F(StrictDnsClusterImplTest, LoadAssignmentBasic) {
             cluster.prioritySet().hostSetsPerPriority()[0]->healthyHostsPerLocality().get().size());
 
   // Ensure that all host objects in the host list are unique.
-  for (const auto host : hosts) {
+  for (const auto& host : hosts) {
     EXPECT_EQ(1, std::count(hosts.begin(), hosts.end(), host));
   }
 

test/config/utility.cc

@@ -267,7 +267,7 @@ ConfigHelper::ConfigHelper(const Network::Address::IpVersion version, Api::Api&
 }
 
 void ConfigHelper::applyConfigModifiers() {
-  for (auto config_modifier : config_modifiers_) {
+  for (const auto& config_modifier : config_modifiers_) {
     config_modifier(bootstrap_);
   }
   config_modifiers_.clear();

test/extensions/filters/common/ext_authz/test_common.cc

@@ -71,7 +71,7 @@ Response TestCommon::makeAuthzResponse(CheckStatus status, Http::Code status_cod
 
 HeaderValueOptionVector TestCommon::makeHeaderValueOption(KeyValueOptionVector&& headers) {
   HeaderValueOptionVector header_option_vector{};
-  for (auto header : headers) {
+  for (const auto& header : headers) {
     envoy::api::v2::core::HeaderValueOption header_value_option;
     auto* mutable_header = header_value_option.mutable_header();
     mutable_header->set_key(header.key);

test/extensions/filters/network/thrift_proxy/conn_manager_test.cc

@@ -83,7 +83,7 @@ class ThriftConnectionManagerTest : public testing::Test {
     // Destroy any existing filter first.
     filter_ = nullptr;
 
-    for (auto counter : store_.counters()) {
+    for (const auto& counter : store_.counters()) {
       counter->reset();
     }
 

test/extensions/stats_sinks/metrics_service/metrics_service_integration_test.cc

@@ -85,7 +85,7 @@ class MetricsServiceIntegrationTest : public Grpc::GrpcClientIntegrationParamTes
       const Protobuf::RepeatedPtrField<::io::prometheus::client::MetricFamily>& envoy_metrics =
           request_msg.envoy_metrics();
 
-      for (::io::prometheus::client::MetricFamily metrics_family : envoy_metrics) {
+      for (const ::io::prometheus::client::MetricFamily& metrics_family : envoy_metrics) {
         if (metrics_family.name() == "cluster.cluster_0.membership_change" &&
             metrics_family.type() == ::io::prometheus::client::MetricType::COUNTER) {
           known_counter_exists = true;

test/integration/http2_integration_test.cc

@@ -257,7 +257,7 @@ TEST_P(Http2MetadataIntegrationTest, ProxyInvalidMetadata) {
 }
 
 void verifyExpectedMetadata(Http::MetadataMap metadata_map, std::set<std::string> keys) {
-  for (const auto key : keys) {
+  for (const auto& key : keys) {
     // keys are the same as their corresponding values.
     EXPECT_EQ(metadata_map.find(key)->second, key);
   }

test/integration/http_integration.cc

@@ -328,7 +328,7 @@ void HttpIntegrationTest::waitForNextUpstreamRequest(uint64_t upstream_index) {
 }
 
 void HttpIntegrationTest::addFilters(std::vector<std::string> filters) {
-  for (const auto filter : filters) {
+  for (const auto& filter : filters) {
     config_helper_.addFilter(filter);
   }
 }

test/integration/integration_admin_test.h

@@ -30,7 +30,7 @@ class IntegrationAdminTest : public HttpProtocolIntegrationTest {
     Json::ObjectSharedPtr statsjson = Json::Factory::loadFromString(stats_json);
     EXPECT_TRUE(statsjson->hasObject("stats"));
     uint64_t histogram_count = 0;
-    for (Json::ObjectSharedPtr obj_ptr : statsjson->getObjectArray("stats")) {
+    for (const Json::ObjectSharedPtr& obj_ptr : statsjson->getObjectArray("stats")) {
       if (obj_ptr->hasObject("histograms")) {
         histogram_count++;
         const Json::ObjectSharedPtr& histograms_ptr = obj_ptr->getObject("histograms");

test/server/http/admin_test.cc

@@ -1033,7 +1033,7 @@ TEST_P(AdminInstanceTest, RuntimeModifyNoArguments) {
 
 TEST_P(AdminInstanceTest, TracingStatsDisabled) {
   const std::string& name = admin_.tracingStats().service_forced_.name();
-  for (Stats::CounterSharedPtr counter : server_.stats().counters()) {
+  for (const Stats::CounterSharedPtr& counter : server_.stats().counters()) {
     EXPECT_NE(counter->name(), name) << "Unexpected tracing stat found in server stats: " << name;
   }
 }