This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push:
new 88eea89cc [compile] Fix compiler warnings about -Wrange-loop-construct
88eea89cc is described below
commit 88eea89cc2e8f019c5f80e4ba7eb2947dc646f64
Author: kedeng <[email protected]>
AuthorDate: Fri Jun 20 16:15:07 2025 +0800
[compile] Fix compiler warnings about -Wrange-loop-construct
This commit resolves multiple instances of the
`-Wrange-loop-construct` warning across various tests.
These warnings occur when using `const std::string&` as
a loop variable to iterate over a brace-enclosed initializer
list of string literals (`const char*`).
The warnings were of the form:
warning: loop variable '...' of type 'const string&' binds
to a temporary constructed from type 'const char* const'
[-Wrange-loop-construct]
To address this:
- Use of `std::string_view`.
- Use of `const std::string` directly in loop.
- Support for `std::string_view` in `SubstituteArg`.
This change eliminates warnings while maintaining safe and
readable code, and avoids unnecessary string copies when not
needed.
Change-Id: I32a9a043e7aade2c2c879472c13cbf2641daa7f3
Reviewed-on: http://gerrit.cloudera.org:8080/23057
Reviewed-by: Alexey Serbin <[email protected]>
Tested-by: Alexey Serbin <[email protected]>
---
src/kudu/consensus/quorum_util-test.cc | 4 ++--
src/kudu/gutil/strings/substitute.h | 4 ++++
src/kudu/tools/kudu-tool-test.cc | 8 +++++---
src/kudu/tserver/tablet_server-test.cc | 4 ++--
4 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/src/kudu/consensus/quorum_util-test.cc
b/src/kudu/consensus/quorum_util-test.cc
index a23acfdce..4422feea3 100644
--- a/src/kudu/consensus/quorum_util-test.cc
+++ b/src/kudu/consensus/quorum_util-test.cc
@@ -1520,7 +1520,7 @@ TEST(QuorumUtilTest,
MultipleReplicasWithReplaceAttribute) {
AddPeer(&config, "E", V, '+');
AddPeer(&config, "F", V, '+');
- for (const string& leader_replica : { "A", "B", "C", "D", "E", "F" }) {
+ for (const string leader_replica : { "A", "B", "C", "D", "E", "F" }) {
string to_evict;
ASSERT_TRUE(ShouldEvictReplica(config, leader_replica, 3, &to_evict));
EXPECT_TRUE(to_evict == "A" || to_evict == "B" || to_evict == "C");
@@ -1539,7 +1539,7 @@ TEST(QuorumUtilTest,
MultipleReplicasWithReplaceAttribute) {
AddPeer(&config, "E", N, '+', {{"PROMOTE", true}});
AddPeer(&config, "F", N, '+', {{"PROMOTE", true}});
- for (const string& leader_replica : { "A", "B", "C" }) {
+ for (const auto* const leader_replica : { "A", "B", "C" }) {
// All non-voters are in good shape and not a single one has been
// promoted yet.
ASSERT_FALSE(ShouldEvictReplica(config, leader_replica, 3));
diff --git a/src/kudu/gutil/strings/substitute.h
b/src/kudu/gutil/strings/substitute.h
index cb2c81864..303f54b29 100644
--- a/src/kudu/gutil/strings/substitute.h
+++ b/src/kudu/gutil/strings/substitute.h
@@ -2,6 +2,7 @@
#include <cstring>
#include <string>
+#include <string_view>
#include "kudu/gutil/strings/numbers.h"
#include "kudu/gutil/strings/stringpiece.h"
@@ -77,6 +78,9 @@ class SubstituteArg {
inline SubstituteArg(const StringPiece& value) // NOLINT(runtime/explicit)
: text_(value.data()), size_(value.size()) {}
+ inline SubstituteArg(const std::string_view& value) //
NOLINT(runtime/explicit)
+ : text_(value.data()), size_(value.size()) {}
+
// Primitives
// We don't overload for signed and unsigned char because if people are
// explicitly declaring their chars as signed or unsigned then they are
diff --git a/src/kudu/tools/kudu-tool-test.cc b/src/kudu/tools/kudu-tool-test.cc
index eecff6aa0..9787908df 100644
--- a/src/kudu/tools/kudu-tool-test.cc
+++ b/src/kudu/tools/kudu-tool-test.cc
@@ -33,6 +33,7 @@
#include <optional>
#include <set>
#include <string>
+#include <string_view>
#include <tuple> // IWYU pragma: keep
#include <type_traits>
#include <unordered_map>
@@ -242,6 +243,7 @@ using std::ostringstream;
using std::pair;
using std::set;
using std::string;
+using std::string_view;
using std::to_string;
using std::unique_ptr;
using std::unordered_map;
@@ -7502,7 +7504,7 @@ TEST_P(ToolTestKerberosParameterized,
TestCheckAndAutomaticFixHmsMetadata) {
ASSERT_TRUE(inconsistent_tables.empty());
- for (const string& table : {
+ for (const auto* const table : {
"control",
"control_external",
"uppercase",
@@ -7816,7 +7818,7 @@ TEST_F(ToolTest, TestHmsPrecheck) {
ASSERT_OK(cluster_->CreateClient(nullptr, &client));
// Create test tables.
- for (const string& table_name : {
+ for (const auto* const table_name : {
"a.b",
"foo.bar",
"FOO.bar",
@@ -9169,7 +9171,7 @@ TEST_P(GetFlagsTest, TestGetFlags) {
// CSV formatting is easiest to match against.
// It seems safe to assume that -help and -logemaillevel will not be
// set, and that fs_wal_dir will be set to a non-default value.
- for (const string& daemon_type : { "master", "tserver" }) {
+ for (const string_view daemon_type : { "master", "tserver" }) {
const string& daemon_addr = daemon_type == "master" ?
cluster_->master()->bound_rpc_addr().ToString() :
cluster_->tablet_server(0)->bound_rpc_addr().ToString();
diff --git a/src/kudu/tserver/tablet_server-test.cc
b/src/kudu/tserver/tablet_server-test.cc
index cddccf195..267a1a55a 100644
--- a/src/kudu/tserver/tablet_server-test.cc
+++ b/src/kudu/tserver/tablet_server-test.cc
@@ -5008,7 +5008,7 @@ TEST_F(TabletServerTest, TestScannerCheckMatchingUser) {
ASSERT_STR_CONTAINS(s.ToString(), "Not authorized");
};
- for (const string& other : { "", "bad-guy" }) {
+ for (const string other : { "", "bad-guy" }) {
TabletServerServiceProxy bad_proxy(
client_messenger_, mini_server_->bound_rpc_addr(),
mini_server_->bound_rpc_addr().host());
@@ -5038,7 +5038,7 @@ TEST_F(TabletServerTest, TestScannerCheckMatchingUser) {
SCOPED_TRACE(resp.DebugString());
NO_FATALS(verify_authz_error(s));
}
- for (const string& id : { scanner_id, checksum_scanner_id }) {
+ for (const auto& id : { scanner_id, checksum_scanner_id }) {
ScannerKeepAliveRequestPB req;
req.set_scanner_id(id);
ScannerKeepAliveResponsePB resp;