[GitHub] [incubator-kvrocks] torwig commented on a diff in pull request #1112: Fix modernize-loop-convert warning reported by clang-tidy

[GitBox <gi...@apache.org>]

torwig commented on code in PR #1112:
URL: https://github.com/apache/incubator-kvrocks/pull/1112#discussion_r1020873024


##########
src/types/redis_string.cc:
##########
@@ -137,8 +137,8 @@ std::vector<rocksdb::Status> String::MGet(const std::vector<Slice> &keys, std::v
   // don't use range-based for loop here, coz the slice member
   // would refer the address instead of copy the value, and use
   // range-based for loop may cause all members refer to the same addr
-  for (size_t i = 0; i < ns_keys.size(); i++) {
-    slice_keys.emplace_back(ns_keys[i]);
+  for (const auto &ns_key : ns_keys) {

Review Comment:
   @MaximSmolskiy This place is really tricky. The comment above it explains why we should NOT use a range-based loop here.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org