[core] Fix root variable shadow (#51743)

dentiny · web-flow · commit 3ebb02355fda · 2025-03-27T22:46:23.000-07:00
Signed-off-by: dentiny &lt;dentinyhao@gmail.com&gt;
diff --git a/src/ray/core_worker/lib/java/jni_utils.h b/src/ray/core_worker/lib/java/jni_utils.h
@@ -303,23 +303,23 @@ extern JavaVM *jvm;
 
 #define RAY_CHECK_JAVA_EXCEPTION(env)                                                 \
   {                                                                                   \
-    jthrowable throwable = env->ExceptionOccurred();                                  \
-    if (throwable) {                                                                  \
-      jstring java_file_name = env->NewStringUTF(__FILE__);                           \
-      jstring java_function = env->NewStringUTF(__func__);                            \
-      jobject java_error_message =                                                    \
+    jthrowable __throwable = env->ExceptionOccurred();                                \
+    if (__throwable) {                                                                \
+      jstring __java_file_name = env->NewStringUTF(__FILE__);                         \
+      jstring __java_function = env->NewStringUTF(__func__);                          \
+      jobject __java_error_message =                                                  \
           env->CallStaticObjectMethod(java_jni_exception_util_class,                  \
                                       java_jni_exception_util_get_stack_trace,        \
-                                      java_file_name,                                 \
+                                      __java_file_name,                               \
                                       __LINE__,                                       \
-                                      java_function,                                  \
-                                      throwable);                                     \
+                                      __java_function,                                \
+                                      __throwable);                                   \
       std::string error_message =                                                     \
-          JavaStringToNativeString(env, static_cast<jstring>(java_error_message));    \
-      env->DeleteLocalRef(throwable);                                                 \
-      env->DeleteLocalRef(java_file_name);                                            \
-      env->DeleteLocalRef(java_function);                                             \
-      env->DeleteLocalRef(java_error_message);                                        \
+          JavaStringToNativeString(env, static_cast<jstring>(__java_error_message));  \
+      env->DeleteLocalRef(__throwable);                                               \
+      env->DeleteLocalRef(__java_file_name);                                          \
+      env->DeleteLocalRef(__java_function);                                           \
+      env->DeleteLocalRef(__java_error_message);                                      \
       RAY_LOG(FATAL) << "An unexpected exception occurred while executing Java code " \
                         "from JNI ("                                                  \
                      << __FILE__ << ":" << __LINE__ << " " << __func__ << ")."        \
diff --git a/src/ray/core_worker/test/direct_actor_transport_test.cc b/src/ray/core_worker/test/direct_actor_transport_test.cc
@@ -936,7 +936,7 @@ TEST_F(TaskReceiverTest, TestNewTaskFromDifferentWorker) {
   // ignored normally, but here it's from a different worker and with a newer
   // timestamp, in this case it should succeed.
   {
-    auto worker_id = WorkerID::FromRandom();
+    worker_id = WorkerID::FromRandom();
     auto request =
         CreatePushTaskRequestHelper(actor_id, 0, worker_id, caller_id, new_timestamp);
     rpc::PushTaskReply reply;
@@ -952,7 +952,7 @@ TEST_F(TaskReceiverTest, TestNewTaskFromDifferentWorker) {
   // Push a task request with actor counter 1, but with a different worker id,
   // and a older timestamp. In this case the request should fail.
   {
-    auto worker_id = WorkerID::FromRandom();
+    worker_id = WorkerID::FromRandom();
     auto request =
         CreatePushTaskRequestHelper(actor_id, 1, worker_id, caller_id, old_timestamp);
     rpc::PushTaskReply reply;
diff --git a/src/ray/core_worker/test/reference_count_test.cc b/src/ray/core_worker/test/reference_count_test.cc
@@ -126,15 +126,14 @@ using PublisherFactoryFn =
 
 class MockDistributedSubscriber : public pubsub::SubscriberInterface {
  public:
-  MockDistributedSubscriber(
-      pubsub::pub_internal::SubscriptionIndex *directory,
-      SubscriptionCallbackMap *subscription_callback_map,
-      SubscriptionFailureCallbackMap *subscription_failure_callback_map,
-      pubsub::SubscriberID subscriber_id,
-      PublisherFactoryFn client_factory)
-      : directory_(directory),
-        subscription_callback_map_(subscription_callback_map),
-        subscription_failure_callback_map_(subscription_failure_callback_map),
+  MockDistributedSubscriber(pubsub::pub_internal::SubscriptionIndex *dict,
+                            SubscriptionCallbackMap *sub_callback_map,
+                            SubscriptionFailureCallbackMap *sub_failure_callback_map,
+                            pubsub::SubscriberID subscriber_id,
+                            PublisherFactoryFn client_factory)
+      : directory_(dict),
+        subscription_callback_map_(sub_callback_map),
+        subscription_failure_callback_map_(sub_failure_callback_map),
         subscriber_id_(subscriber_id),
         subscriber_(std::make_unique<pubsub::pub_internal::SubscriberState>(
             subscriber_id,
@@ -233,14 +232,13 @@ class MockDistributedSubscriber : public pubsub::SubscriberInterface {
 
 class MockDistributedPublisher : public pubsub::PublisherInterface {
  public:
-  MockDistributedPublisher(
-      pubsub::pub_internal::SubscriptionIndex *directory,
-      SubscriptionCallbackMap *subscription_callback_map,
-      SubscriptionFailureCallbackMap *subscription_failure_callback_map,
-      WorkerID publisher_id)
-      : directory_(directory),
-        subscription_callback_map_(subscription_callback_map),
-        subscription_failure_callback_map_(subscription_failure_callback_map),
+  MockDistributedPublisher(pubsub::pub_internal::SubscriptionIndex *dict,
+                           SubscriptionCallbackMap *sub_callback_map,
+                           SubscriptionFailureCallbackMap *sub_failure_callback_map,
+                           WorkerID publisher_id)
+      : directory_(dict),
+        subscription_callback_map_(sub_callback_map),
+        subscription_failure_callback_map_(sub_failure_callback_map),
         publisher_id_(publisher_id) {}
   ~MockDistributedPublisher() = default;
 
diff --git a/src/ray/gcs/gcs_server/test/gcs_actor_scheduler_mock_test.cc b/src/ray/gcs/gcs_server/test/gcs_actor_scheduler_mock_test.cc
@@ -62,8 +62,8 @@ class GcsActorSchedulerMockTest : public Test {
         local_node_id,
         *cluster_resource_scheduler,
         /*get_node_info=*/
-        [this](const NodeID &node_id) {
-          auto node = gcs_node_manager->GetAliveNode(node_id);
+        [this](const NodeID &nid) {
+          auto node = gcs_node_manager->GetAliveNode(nid);
           return node.has_value() ? node.value().get() : nullptr;
         },
         /*announce_infeasible_task=*/nullptr,
diff --git a/src/ray/gcs/gcs_server/test/gcs_placement_group_manager_test.cc b/src/ray/gcs/gcs_server/test/gcs_placement_group_manager_test.cc
@@ -857,11 +857,11 @@ TEST_F(GcsPlacementGroupManagerTest, TestStats) {
           return mock_placement_group_scheduler_->GetPlacementGroupCount() == 1;
         },
         10 * 1000));
-    auto placement_group = mock_placement_group_scheduler_->placement_groups_.back();
+    auto last_placement_group = mock_placement_group_scheduler_->placement_groups_.back();
     mock_placement_group_scheduler_->placement_groups_.clear();
-    ASSERT_EQ(placement_group->GetStats().scheduling_state(),
+    ASSERT_EQ(last_placement_group->GetStats().scheduling_state(),
               rpc::PlacementGroupStats::NO_RESOURCES);
-    ASSERT_EQ(placement_group->GetStats().scheduling_attempt(), 2);
+    ASSERT_EQ(last_placement_group->GetStats().scheduling_attempt(), 2);
   }
 
   /// Feasible, but failed to commit resources.
@@ -875,11 +875,11 @@ TEST_F(GcsPlacementGroupManagerTest, TestStats) {
           return mock_placement_group_scheduler_->GetPlacementGroupCount() == 1;
         },
         10 * 1000));
-    auto placement_group = mock_placement_group_scheduler_->placement_groups_.back();
+    auto last_placement_group = mock_placement_group_scheduler_->placement_groups_.back();
     mock_placement_group_scheduler_->placement_groups_.clear();
-    ASSERT_EQ(placement_group->GetStats().scheduling_state(),
+    ASSERT_EQ(last_placement_group->GetStats().scheduling_state(),
               rpc::PlacementGroupStats::FAILED_TO_COMMIT_RESOURCES);
-    ASSERT_EQ(placement_group->GetStats().scheduling_attempt(), 3);
+    ASSERT_EQ(last_placement_group->GetStats().scheduling_attempt(), 3);
   }
 
   // Check that the placement_group scheduling state is `FINISHED`.
diff --git a/src/ray/gcs/gcs_server/test/gcs_server_test_util.h b/src/ray/gcs/gcs_server/test/gcs_server_test_util.h
@@ -331,7 +331,7 @@ struct GcsServerMocker {
 
     /// ShutdownRaylet
     void ShutdownRaylet(
-        const NodeID &node_id,
+        const NodeID &raylet_node_id,
         bool graceful,
         const rpc::ClientCallback<rpc::ShutdownRayletReply> &callback) override{};
 
diff --git a/src/ray/gcs/gcs_server/test/gcs_task_manager_test.cc b/src/ray/gcs/gcs_server/test/gcs_task_manager_test.cc
@@ -1351,11 +1351,11 @@ TEST_F(GcsTaskManagerMemoryLimitedTest, TestIndexNoLeak) {
 
   // Evict all of them with tasks with single attempt, no parent, same job, no worker id.
   {
-    auto task_ids = GenTaskIDs(num_limit);
+    auto tids = GenTaskIDs(num_limit);
     auto job_id = 0;
     auto attempt_number = 0;
     for (size_t i = 0; i < num_limit; i++) {
-      auto events = GenTaskEvents({task_ids[i]},
+      auto events = GenTaskEvents({tids[i]},
                                   /* attempt_number */ attempt_number,
                                   job_id,
                                   GenProfileEvents("event", 1, 1),
diff --git a/src/ray/gcs/store_client/test/redis_store_client_test.cc b/src/ray/gcs/store_client/test/redis_store_client_test.cc
@@ -182,31 +182,31 @@ TEST_F(RedisStoreClientTest, Complicated) {
 
                    if ((i / window) % 2 == 0) {
                      // Delete non exist keys
-                     for (size_t i = 0; i < keys.size(); ++i) {
+                     for (size_t jj = 0; jj < keys.size(); ++jj) {
                        ++sent;
-                       RAY_LOG(INFO) << "S AsyncDelete: " << n_keys[i];
+                       RAY_LOG(INFO) << "S AsyncDelete: " << n_keys[jj];
                        ASSERT_TRUE(
                            store_client_
                                ->AsyncDelete("N",
-                                             n_keys[i],
-                                             {[&finished, n_keys, i](auto b) mutable {
+                                             n_keys[jj],
+                                             {[&finished, n_keys, jj](auto b) mutable {
                                                 RAY_LOG(INFO)
-                                                    << "F AsyncDelete: " << n_keys[i];
+                                                    << "F AsyncDelete: " << n_keys[jj];
                                                 ++finished;
                                                 ASSERT_FALSE(b);
                                               },
                                               *this->io_service_pool_->Get()})
                                .ok());
 
                        ++sent;
-                       RAY_LOG(INFO) << "S AsyncExists: " << p_keys[i];
+                       RAY_LOG(INFO) << "S AsyncExists: " << p_keys[jj];
                        ASSERT_TRUE(
                            store_client_
                                ->AsyncExists("N",
-                                             p_keys[i],
-                                             {[&finished, p_keys, i](auto b) mutable {
+                                             p_keys[jj],
+                                             {[&finished, p_keys, jj](auto b) mutable {
                                                 RAY_LOG(INFO)
-                                                    << "F AsyncExists: " << p_keys[i];
+                                                    << "F AsyncExists: " << p_keys[jj];
                                                 ++finished;
                                                 ASSERT_TRUE(b);
                                               },
diff --git a/src/ray/object_manager/test/create_request_queue_test.cc b/src/ray/object_manager/test/create_request_queue_test.cc
@@ -41,16 +41,16 @@ class MockClient : public ClientInterface {
     ASSERT_FALSE(queue.GetRequestResult(req_id, &result, &status)); \
   }
 
-#define ASSERT_REQUEST_FINISHED(queue, req_id, expected_status)    \
-  {                                                                \
-    PlasmaObject result = {};                                      \
-    PlasmaError status;                                            \
-                                                                   \
-    ASSERT_TRUE(queue.GetRequestResult(req_id, &result, &status)); \
-    if (expected_status == PlasmaError::OK) {                      \
-      ASSERT_EQ(result.data_size, 1234);                           \
-    }                                                              \
-    ASSERT_EQ(status, expected_status);                            \
+#define ASSERT_REQUEST_FINISHED(queue, req_id, expected_status)        \
+  {                                                                    \
+    PlasmaObject __result = {};                                        \
+    PlasmaError __status;                                              \
+                                                                       \
+    ASSERT_TRUE(queue.GetRequestResult(req_id, &__result, &__status)); \
+    if (expected_status == PlasmaError::OK) {                          \
+      ASSERT_EQ(__result.data_size, 1234);                             \
+    }                                                                  \
+    ASSERT_EQ(__status, expected_status);                              \
   }
 
 class CreateRequestQueueTest : public ::testing::Test {
diff --git a/src/ray/pubsub/test/publisher_test.cc b/src/ray/pubsub/test/publisher_test.cc
@@ -107,18 +107,17 @@ class PublisherTest : public ::testing::Test {
   std::shared_ptr<rpc::PubsubLongPollingReply> FlushSubscriber(
       SubscriberState *subscriber, int64_t max_processed_sequence_id = -1) {
     rpc::PubsubLongPollingRequest request;
-    auto reply = std::make_shared<rpc::PubsubLongPollingReply>();
+    auto pubsub_reply = std::make_shared<rpc::PubsubLongPollingReply>();
     request.set_publisher_id(kDefaultPublisherId.Binary());
     if (max_processed_sequence_id >= 0) {
       request.set_max_processed_sequence_id(max_processed_sequence_id);
     }
-    rpc::SendReplyCallback send_reply_callback = [reply](Status status,
-                                                         std::function<void()> success,
-                                                         std::function<void()> failure) {
-    };
-    subscriber->ConnectToSubscriber(request, reply.get(), send_reply_callback);
+    rpc::SendReplyCallback callback = [pubsub_reply](Status status,
+                                                     std::function<void()> success,
+                                                     std::function<void()> failure) {};
+    subscriber->ConnectToSubscriber(request, pubsub_reply.get(), callback);
     subscriber->PublishIfPossible();
-    return reply;
+    return pubsub_reply;
   }
 
   instrumented_io_context io_service_;
@@ -392,8 +391,8 @@ TEST_F(PublisherTest, TestSubscriber) {
   // Since there's no connection, objects won't be published.
   ASSERT_FALSE(subscriber->PublishIfPossible());
   subscriber->ConnectToSubscriber(request_, &reply, send_reply_callback);
-  for (auto oid : published_objects) {
-    ASSERT_TRUE(object_ids_published.contains(oid));
+  for (auto cur_oid : published_objects) {
+    ASSERT_TRUE(object_ids_published.contains(cur_oid));
   }
 
   // Queue is not cleaned up if max_processed_sequence_id hasn't
@@ -1172,32 +1171,37 @@ TEST_F(PublisherTest, TestMaxBufferSizeAllEntities) {
   subscription_index.AddEntry("", subscriber);
 
   rpc::PubMessage pub_message;
-  pub_message.set_key_id("aaa");
-  pub_message.set_channel_type(rpc::ChannelType::RAY_ERROR_INFO_CHANNEL);
-  pub_message.mutable_error_info_message()->set_error_message(std::string(4000, 'a'));
-  pub_message.set_sequence_id(GetNextSequenceId());
+  {
+    pub_message.set_key_id("aaa");
+    pub_message.set_channel_type(rpc::ChannelType::RAY_ERROR_INFO_CHANNEL);
+    pub_message.mutable_error_info_message()->set_error_message(std::string(4000, 'a'));
+    pub_message.set_sequence_id(GetNextSequenceId());
 
-  // Buffer is available.
-  EXPECT_TRUE(subscription_index.Publish(std::make_shared<rpc::PubMessage>(pub_message),
-                                         /*msg_size=*/pub_message.ByteSizeLong()));
+    // Buffer is available.
+    EXPECT_TRUE(subscription_index.Publish(std::make_shared<rpc::PubMessage>(pub_message),
+                                           /*msg_size=*/pub_message.ByteSizeLong()));
+  }
 
   // Buffer is still available.
-  pub_message.set_key_id("bbb");
-  pub_message.mutable_error_info_message()->set_error_message(std::string(4000, 'b'));
-  pub_message.set_sequence_id(GetNextSequenceId());
-  EXPECT_TRUE(subscription_index.Publish(std::make_shared<rpc::PubMessage>(pub_message),
-                                         /*msg_size=*/pub_message.ByteSizeLong()));
+  {
+    pub_message.set_key_id("bbb");
+    pub_message.mutable_error_info_message()->set_error_message(std::string(4000, 'b'));
+    pub_message.set_sequence_id(GetNextSequenceId());
+    EXPECT_TRUE(subscription_index.Publish(std::make_shared<rpc::PubMessage>(pub_message),
+                                           /*msg_size=*/pub_message.ByteSizeLong()));
+  }
 
   // Buffer is full.
-  pub_message.set_key_id("ccc");
-  pub_message.mutable_error_info_message()->set_error_message(std::string(4000, 'c'));
-  pub_message.set_sequence_id(GetNextSequenceId());
-  EXPECT_TRUE(subscription_index.Publish(std::make_shared<rpc::PubMessage>(pub_message),
-                                         /*msg_size=*/pub_message.ByteSizeLong()));
+  {
+    pub_message.set_key_id("ccc");
+    pub_message.mutable_error_info_message()->set_error_message(std::string(4000, 'c'));
+    pub_message.set_sequence_id(GetNextSequenceId());
+    EXPECT_TRUE(subscription_index.Publish(std::make_shared<rpc::PubMessage>(pub_message),
+                                           /*msg_size=*/pub_message.ByteSizeLong()));
+  }
 
   {
     // Publishing individual messages that are too large fails.
-    rpc::PubMessage pub_message;
     pub_message.set_key_id("ddd");
     pub_message.set_channel_type(rpc::ChannelType::RAY_ERROR_INFO_CHANNEL);
     pub_message.mutable_error_info_message()->set_error_message(std::string(12000, 'a'));
diff --git a/src/ray/pubsub/test/subscriber_test.cc b/src/ray/pubsub/test/subscriber_test.cc
diff --git a/src/ray/raylet/scheduling/cluster_resource_scheduler_test.cc b/src/ray/raylet/scheduling/cluster_resource_scheduler_test.cc
diff --git a/src/ray/raylet/scheduling/cluster_task_manager_test.cc b/src/ray/raylet/scheduling/cluster_task_manager_test.cc
diff --git a/src/ray/raylet/scheduling/policy/scheduling_policy_test.cc b/src/ray/raylet/scheduling/policy/scheduling_policy_test.cc
diff --git a/src/ray/stats/stats_test.cc b/src/ray/stats/stats_test.cc
