Fix compiler warnings and make warnings fatal (#5375)

This commit is contained in:
Philipp Moritz
2019-08-07 14:04:05 -07:00
committed by GitHub
parent 7d747da420
commit 8d6c50c821
27 changed files with 61 additions and 63 deletions
+5
View File
@@ -2,5 +2,10 @@
build --compilation_mode=opt
build --action_env=PATH
build --action_env=PYTHON_BIN_PATH
# Warnings should be errors
build --per_file_copt=-src/ray/thirdparty/hiredis/dict.c,-.*/arrow/util/logging.cc@-Werror
# Ignore warnings for protobuf generated files and external projects.
build --per_file_copt='\\.pb\\.cc$@-w'
build --per_file_copt='external*@-w'
# This workaround is needed due to https://github.com/bazelbuild/bazel/issues/4341
build --per_file_copt="external/com_github_grpc_grpc/.*@-DGRPC_BAZEL_BUILD"
+1 -1
View File
@@ -86,7 +86,7 @@ cdef VectorToObjectIDs(c_vector[CObjectID] object_ids):
def compute_put_id(TaskID task_id, int64_t put_index):
if put_index < 1 or put_index > CObjectID.MaxObjectIndex():
if put_index < 1 or put_index > <int64_t>CObjectID.MaxObjectIndex():
raise ValueError("The range of 'put_index' should be [1, %d]"
% CObjectID.MaxObjectIndex())
return ObjectID(CObjectID.ForPut(task_id.native(), put_index, 0).Binary())
+2 -2
View File
@@ -99,7 +99,7 @@ inline uint8_t GetTransportType(ObjectIDFlagsType flags) {
template <typename T>
void FillNil(T *data) {
RAY_CHECK(data != nullptr);
for (int i = 0; i < data->size(); i++) {
for (size_t i = 0; i < data->size(); i++) {
(*data)[i] = static_cast<uint8_t>(0xFF);
}
}
@@ -129,7 +129,7 @@ plasma::UniqueID ObjectID::ToPlasmaId() const {
}
ObjectID::ObjectID(const plasma::UniqueID &from) {
RAY_CHECK(from.size() <= ObjectID::Size()) << "Out of size.";
RAY_CHECK(from.size() <= static_cast<int64_t>(ObjectID::Size())) << "Out of size.";
std::memcpy(this->MutableData(), from.data(), ObjectID::Size());
}
+1 -1
View File
@@ -151,7 +151,7 @@ RAY_CONFIG(int64_t, max_task_lease_timeout_ms, 60000)
/// Note: this number should be set to at least 2. Because saving a application
/// checkpoint isn't atomic with saving the backend checkpoint, and it will break
/// if this number is set to 1 and users save application checkpoints in place.
RAY_CONFIG(uint32_t, num_actor_checkpoints_to_keep, 20)
RAY_CONFIG(int32_t, num_actor_checkpoints_to_keep, 20)
/// Maximum number of ids in one batch to send to GCS to delete keys.
RAY_CONFIG(uint32_t, maximum_gcs_deletion_batch_size, 1000)
+1 -1
View File
@@ -16,7 +16,7 @@ const std::vector<ObjectID> &Task::GetDependencies() const { return dependencies
void Task::ComputeDependencies() {
dependencies_.clear();
for (int i = 0; i < task_spec_.NumArgs(); ++i) {
for (size_t i = 0; i < task_spec_.NumArgs(); ++i) {
int count = task_spec_.ArgIdCount(i);
for (int j = 0; j < count; j++) {
dependencies_.push_back(task_spec_.ArgId(i, j));
+1 -1
View File
@@ -156,7 +156,7 @@ std::string TaskSpecification::DebugString() const {
const auto list = VectorFromProtobuf(message_->function_descriptor());
// The 4th is the code hash which is binary bits. No need to output it.
const size_t size = std::min(static_cast<size_t>(3), list.size());
for (int i = 0; i < size; ++i) {
for (size_t i = 0; i < size; ++i) {
if (i != 0) {
stream << ",";
}
+1 -4
View File
@@ -58,16 +58,13 @@ std::unique_ptr<CoreWorkerStoreProvider> CoreWorkerObjectInterface::CreateStoreP
case StoreProviderType::LOCAL_PLASMA:
return std::unique_ptr<CoreWorkerStoreProvider>(
new CoreWorkerLocalPlasmaStoreProvider(store_socket_));
break;
case StoreProviderType::PLASMA:
return std::unique_ptr<CoreWorkerStoreProvider>(
new CoreWorkerPlasmaStoreProvider(store_socket_, raylet_client_));
break;
default:
RAY_LOG(FATAL) << "unknown store provider type " << static_cast<int>(type);
break;
return nullptr;
}
return nullptr;
}
} // namespace ray
@@ -67,7 +67,7 @@ Status CoreWorkerLocalPlasmaStoreProvider::Wait(const std::vector<ObjectID> &obj
int num_objects, int64_t timeout_ms,
const TaskID &task_id,
std::vector<bool> *results) {
if (num_objects != object_ids.size()) {
if (num_objects != static_cast<int>(object_ids.size())) {
return Status::Invalid("num_objects should equal to number of items in object_ids");
}
@@ -140,7 +140,7 @@ Status CoreWorkerMemoryStore::Get(const std::vector<ObjectID> &object_ids,
std::unique_lock<std::mutex> lock(lock_);
// Check for existing objects and see if this get request can be fullfilled.
for (int i = 0; i < object_ids.size(); i++) {
for (size_t i = 0; i < object_ids.size(); i++) {
const auto &object_id = object_ids[i];
auto iter = objects_.find(object_id);
if (iter != objects_.end()) {
@@ -178,7 +178,7 @@ Status CoreWorkerMemoryStore::Get(const std::vector<ObjectID> &object_ids,
{
std::unique_lock<std::mutex> lock(lock_);
// Populate results.
for (int i = 0; i < object_ids.size(); i++) {
for (size_t i = 0; i < object_ids.size(); i++) {
const auto &object_id = object_ids[i];
if ((*results)[i] == nullptr) {
(*results)[i] = get_request->Get(object_id);
@@ -31,7 +31,7 @@ Status CoreWorkerMemoryStoreProvider::Wait(const std::vector<ObjectID> &object_i
int num_objects, int64_t timeout_ms,
const TaskID &task_id,
std::vector<bool> *results) {
if (num_objects != object_ids.size()) {
if (num_objects != static_cast<int>(object_ids.size())) {
return Status::Invalid("num_objects should equal to number of items in object_ids");
}
@@ -41,7 +41,7 @@ Status CoreWorkerMemoryStoreProvider::Wait(const std::vector<ObjectID> &object_i
auto status = store_->Get(object_ids, timeout_ms, false, &result_objects);
if (status.ok()) {
RAY_CHECK(result_objects.size() == object_ids.size());
for (int i = 0; i < object_ids.size(); i++) {
for (size_t i = 0; i < object_ids.size(); i++) {
(*results)[i] = (result_objects[i] != nullptr);
}
}
+1 -1
View File
@@ -83,7 +83,7 @@ Status CoreWorkerTaskExecutionInterface::BuildArgsForExecutor(
std::vector<ObjectID> object_ids_to_fetch;
std::vector<int> indices;
for (int i = 0; i < task.NumArgs(); ++i) {
for (size_t i = 0; i < task.NumArgs(); ++i) {
int count = task.ArgIdCount(i);
if (count > 0) {
// pass by reference.
+1 -1
View File
@@ -134,7 +134,7 @@ void CoreWorkerTaskInterface::BuildCommonTaskSpec(
// Compute return IDs.
(*return_ids).resize(num_returns);
for (int i = 0; i < num_returns; i++) {
for (size_t i = 0; i < num_returns; i++) {
(*return_ids)[i] = ObjectID::ForTaskReturn(task_id, i + 1, /*transport_type=*/0);
}
}
+2 -2
View File
@@ -360,7 +360,7 @@ void CoreWorkerTest::TestActorReconstruction(
for (int i = 0; i < num_tasks; i++) {
if (i == task_index_to_kill_worker) {
RAY_LOG(INFO) << "killing worker";
system("pkill mock_worker");
ASSERT_EQ(system("pkill mock_worker"), 0);
// Wait for actor restruction event, and then for alive event.
ASSERT_TRUE(WaitForDirectCallActorState(driver, actor_handle->ActorID(), false,
@@ -413,7 +413,7 @@ void CoreWorkerTest::TestActorFailure(
for (int i = 0; i < num_tasks; i++) {
if (i == task_index_to_kill_worker) {
RAY_LOG(INFO) << "killing worker";
system("pkill mock_worker");
ASSERT_EQ(system("pkill mock_worker"), 0);
}
// wait for actor being reconstructed.
@@ -7,7 +7,7 @@ using ray::rpc::ActorTableData;
namespace ray {
bool HasByReferenceArgs(const TaskSpecification &spec) {
for (int i = 0; i < spec.NumArgs(); ++i) {
for (size_t i = 0; i < spec.NumArgs(); ++i) {
if (spec.ArgIdCount(i) > 0) {
return true;
}
@@ -149,7 +149,8 @@ Status CoreWorkerDirectActorTaskSubmitter::PushTask(rpc::DirectActorClient &clie
reinterpret_cast<const uint8_t *>(return_object.metadata().data())),
return_object.metadata().size());
}
store_provider_->Put(RayObject(data_buffer, metadata_buffer), object_id);
RAY_CHECK_OK(
store_provider_->Put(RayObject(data_buffer, metadata_buffer), object_id));
}
});
return status;
@@ -163,7 +164,7 @@ void CoreWorkerDirectActorTaskSubmitter::TreatTaskAsFailed(
std::string meta = std::to_string(static_cast<int>(error_type));
auto metadata = const_cast<uint8_t *>(reinterpret_cast<const uint8_t *>(meta.data()));
auto meta_buffer = std::make_shared<LocalMemoryBuffer>(metadata, meta.size());
store_provider_->Put(RayObject(nullptr, meta_buffer), object_id);
RAY_CHECK_OK(store_provider_->Put(RayObject(nullptr, meta_buffer), object_id));
}
}
@@ -203,7 +204,7 @@ void CoreWorkerDirectActorTaskReceiver::HandlePushTask(
auto status = task_handler_(task_spec, &results);
RAY_CHECK(results.size() == num_returns) << results.size() << " " << num_returns;
for (int i = 0; i < results.size(); i++) {
for (size_t i = 0; i < results.size(); i++) {
auto return_object = (*reply).add_return_objects();
ObjectID id = ObjectID::ForTaskReturn(task_spec.TaskId(), /*index=*/i + 1,
/*transport_type=*/0);
@@ -40,10 +40,10 @@ void CoreWorkerRayletTaskReceiver::HandleAssignTask(
}
RAY_CHECK(results.size() == num_returns);
for (int i = 0; i < num_returns; i++) {
for (size_t i = 0; i < num_returns; i++) {
ObjectID id = ObjectID::ForTaskReturn(task_spec.TaskId(), /*index=*/i + 1,
/*transport_type=*/0);
object_interface_.Put(*results[i], id);
RAY_CHECK_OK(object_interface_.Put(*results[i], id));
}
// Notify raylet that current task is done via a `TaskDone` message. This is to
+17 -17
View File
@@ -22,7 +22,7 @@ class ActorStateAccessorTest : public ::testing::Test {
RAY_CHECK_OK(gcs_client_->Connect(io_service_));
work_thread.reset(new std::thread([this] {
std::auto_ptr<boost::asio::io_service::work> work(
std::unique_ptr<boost::asio::io_service::work> work(
new boost::asio::io_service::work(io_service_));
io_service_.run();
}));
@@ -91,10 +91,10 @@ TEST_F(ActorStateAccessorTest, RegisterAndGet) {
for (const auto &elem : actor_datas_) {
const auto &actor = elem.second;
++pending_count_;
actor_accessor.AsyncRegister(actor, [this](Status status) {
RAY_CHECK_OK(actor_accessor.AsyncRegister(actor, [this](Status status) {
RAY_CHECK_OK(status);
--pending_count_;
});
}));
}
std::chrono::milliseconds timeout(10000);
@@ -102,16 +102,15 @@ TEST_F(ActorStateAccessorTest, RegisterAndGet) {
// get
for (const auto &elem : actor_datas_) {
const auto &actor = elem.second;
++pending_count_;
actor_accessor.AsyncGet(elem.first,
[this](Status status, std::vector<ActorTableData> datas) {
ASSERT_EQ(datas.size(), 1U);
ActorID actor_id = ActorID::FromBinary(datas[0].actor_id());
auto it = actor_datas_.find(actor_id);
ASSERT_TRUE(it != actor_datas_.end());
--pending_count_;
});
RAY_CHECK_OK(actor_accessor.AsyncGet(
elem.first, [this](Status status, std::vector<ActorTableData> datas) {
ASSERT_EQ(datas.size(), 1U);
ActorID actor_id = ActorID::FromBinary(datas[0].actor_id());
auto it = actor_datas_.find(actor_id);
ASSERT_TRUE(it != actor_datas_.end());
--pending_count_;
}));
}
WaitPendingDone(timeout);
@@ -135,7 +134,7 @@ TEST_F(ActorStateAccessorTest, Subscribe) {
};
++do_sub_pending_count;
actor_accessor.AsyncSubscribe(subscribe, done);
RAY_CHECK_OK(actor_accessor.AsyncSubscribe(subscribe, done));
// Wait until subscribe finishes.
WaitPendingDone(do_sub_pending_count, timeout);
@@ -145,10 +144,11 @@ TEST_F(ActorStateAccessorTest, Subscribe) {
const auto &actor = elem.second;
++sub_pending_count;
++register_pending_count;
actor_accessor.AsyncRegister(actor, [&register_pending_count](Status status) {
RAY_CHECK_OK(status);
--register_pending_count;
});
RAY_CHECK_OK(
actor_accessor.AsyncRegister(actor, [&register_pending_count](Status status) {
RAY_CHECK_OK(status);
--register_pending_count;
}));
}
// Wait until register finishes.
WaitPendingDone(register_pending_count, timeout);
+1 -1
View File
@@ -701,7 +701,7 @@ TEST_F(TestGcsWithAsio, TestSetSubscribeAll) {
void TestTableSubscribeId(const JobID &job_id,
std::shared_ptr<gcs::RedisGcsClient> client) {
int num_modifications = 3;
size_t num_modifications = 3;
// Add a table entry.
TaskID task_id1 = RandomTaskId();
+3 -3
View File
@@ -585,7 +585,7 @@ int HashUpdate_DoWrite(RedisModuleCtx *ctx, RedisModuleString **argv, int argc,
// This code path means they are updating command.
size_t total_size = gcs_entry.entries_size();
REPLY_AND_RETURN_IF_FALSE(total_size % 2 == 0, "Invalid Hash Update data vector.");
for (int i = 0; i < total_size; i += 2) {
for (size_t i = 0; i < total_size; i += 2) {
// Reconstruct a key-value pair from a flattened list.
RedisModuleString *entry_key = RedisModule_CreateString(
ctx, gcs_entry.entries(i).data(), gcs_entry.entries(i).size());
@@ -603,7 +603,7 @@ int HashUpdate_DoWrite(RedisModuleCtx *ctx, RedisModuleString **argv, int argc,
updated.set_change_mode(gcs_entry.change_mode());
size_t total_size = gcs_entry.entries_size();
for (int i = 0; i < total_size; i++) {
for (size_t i = 0; i < total_size; i++) {
RedisModuleString *entry_key = RedisModule_CreateString(
ctx, gcs_entry.entries(i).data(), gcs_entry.entries(i).size());
int deleted_num = RedisModule_HashSet(key, REDISMODULE_HASH_NONE, entry_key,
@@ -929,7 +929,7 @@ Status IsNil(bool *out, const std::string &data) {
return Status::RedisError("Size of data doesn't match size of UniqueID");
}
const uint8_t *d = reinterpret_cast<const uint8_t *>(data.data());
for (int i = 0; i < kUniqueIDSize; ++i) {
for (size_t i = 0; i < kUniqueIDSize; ++i) {
if (d[i] != 255) {
*out = false;
}
+2 -2
View File
@@ -92,7 +92,7 @@ Status Log<ID, Data>::Lookup(const JobID &job_id, const ID &id, const Callback &
GcsEntry gcs_entry;
gcs_entry.ParseFromString(reply.ReadAsString());
RAY_CHECK(ID::FromBinary(gcs_entry.id()) == id);
for (size_t i = 0; i < gcs_entry.entries_size(); i++) {
for (int64_t i = 0; i < gcs_entry.entries_size(); i++) {
Data data;
data.ParseFromString(gcs_entry.entries(i));
results.emplace_back(std::move(data));
@@ -142,7 +142,7 @@ Status Log<ID, Data>::Subscribe(const JobID &job_id, const ClientID &client_id,
gcs_entry.ParseFromString(data);
ID id = ID::FromBinary(gcs_entry.id());
std::vector<Data> results;
for (size_t i = 0; i < gcs_entry.entries_size(); i++) {
for (int64_t i = 0; i < gcs_entry.entries_size(); i++) {
Data result;
result.ParseFromString(gcs_entry.entries(i));
results.emplace_back(std::move(result));
+2 -2
View File
@@ -17,7 +17,7 @@ ActorRegistration::ActorRegistration(const ActorTableData &actor_table_data,
execution_dependency_(
ObjectID::FromBinary(checkpoint_data.execution_dependency())) {
// Restore `frontier_`.
for (size_t i = 0; i < checkpoint_data.handle_ids_size(); i++) {
for (int64_t i = 0; i < checkpoint_data.handle_ids_size(); i++) {
auto handle_id = ActorHandleID::FromBinary(checkpoint_data.handle_ids(i));
auto &frontier_entry = frontier_[handle_id];
frontier_entry.task_counter = checkpoint_data.task_counters(i);
@@ -25,7 +25,7 @@ ActorRegistration::ActorRegistration(const ActorTableData &actor_table_data,
ObjectID::FromBinary(checkpoint_data.frontier_dependencies(i));
}
// Restore `dummy_objects_`.
for (size_t i = 0; i < checkpoint_data.unreleased_dummy_objects_size(); i++) {
for (int64_t i = 0; i < checkpoint_data.unreleased_dummy_objects_size(); i++) {
auto dummy = ObjectID::FromBinary(checkpoint_data.unreleased_dummy_objects(i));
dummy_objects_[dummy] = checkpoint_data.num_dummy_object_dependencies(i);
}
+1 -1
View File
@@ -160,7 +160,7 @@ std::vector<ObjectID> InsertTaskChain(LineageCache &lineage_cache,
lineage_cache.AddUncommittedLineage(task.GetTaskSpecification().TaskId(), lineage);
inserted_tasks.push_back(task);
arguments.clear();
for (int j = 0; j < task.GetTaskSpecification().NumReturns(); j++) {
for (size_t j = 0; j < task.GetTaskSpecification().NumReturns(); j++) {
arguments.push_back(task.GetTaskSpecification().ReturnId(j));
}
}
+4 -4
View File
@@ -1021,7 +1021,7 @@ void NodeManager::HandleFetchOrReconstructRequest(
WorkerID worker_id = WorkerID::FromBinary(request.worker_id());
const auto &object_ids = request.object_ids();
std::vector<ObjectID> required_object_ids;
for (size_t i = 0; i < object_ids.size(); ++i) {
for (int64_t i = 0; i < object_ids.size(); ++i) {
ObjectID object_id = ObjectID::FromBinary(object_ids[i]);
if (request.fetch_only()) {
// If only a fetch is required, then do not subscribe to the
@@ -1513,7 +1513,7 @@ void NodeManager::SubmitTask(const Task &task, const Lineage &uncommitted_lineag
// The actor is local.
int64_t expected_task_counter = GetExpectedTaskCounter(
actor_registry_, spec.ActorId(), spec.ActorHandleId());
if (spec.ActorCounter() < expected_task_counter) {
if (static_cast<int64_t>(spec.ActorCounter()) < expected_task_counter) {
// A task that has already been executed before has been found. The
// task will be treated as failed if at least one of the task's
// return values have been evicted, to prevent the application from
@@ -1751,7 +1751,7 @@ bool NodeManager::AssignTask(const Task &task) {
// expected task counter.
int64_t expected_task_counter =
GetExpectedTaskCounter(actor_registry_, spec.ActorId(), spec.ActorHandleId());
RAY_CHECK(spec.ActorCounter() == expected_task_counter)
RAY_CHECK(static_cast<int64_t>(spec.ActorCounter()) == expected_task_counter)
<< "Expected actor counter: " << expected_task_counter << ", task "
<< spec.TaskId() << " has: " << spec.ActorCounter();
}
@@ -2341,7 +2341,7 @@ void NodeManager::ForwardTask(
// Iterate through the object's arguments. NOTE(swang): We do not include
// the execution dependencies here since those cannot be transferred
// between nodes.
for (int i = 0; i < spec.NumArgs(); ++i) {
for (size_t i = 0; i < spec.NumArgs(); ++i) {
int count = spec.ArgIdCount(i);
for (int j = 0; j < count; j++) {
ObjectID argument_id = spec.ArgId(i, j);
@@ -94,7 +94,7 @@ std::vector<Task> MakeTaskChain(int chain_size,
auto task = ExampleTask(arguments, num_returns);
task_chain.push_back(task);
arguments.clear();
for (int j = 0; j < task.GetTaskSpecification().NumReturns(); j++) {
for (size_t j = 0; j < task.GetTaskSpecification().NumReturns(); j++) {
arguments.push_back(task.GetTaskSpecification().ReturnId(j));
}
}
+1 -1
View File
@@ -109,7 +109,7 @@ ray::Status RayletClient::GetTask(std::unique_ptr<ray::TaskSpecification> *task_
if (status.ok()) {
resource_ids_.clear();
// Parse resources that would be used by this assigned task.
for (size_t i = 0; i < reply.fractional_resource_ids().size(); ++i) {
for (int64_t i = 0; i < reply.fractional_resource_ids().size(); ++i) {
auto const &fractional_resource_ids = reply.fractional_resource_ids()[i];
auto &acquired_resources = resource_ids_[fractional_resource_ids.resource_name()];
-4
View File
@@ -41,10 +41,6 @@ class StatsConfig final {
bool is_stats_disabled_ = true;
};
/// The helper function for registering a view.
static void RegisterAsView(opencensus::stats::ViewDescriptor view_descriptor,
const std::vector<opencensus::tags::TagKey> &keys);
/// A thin wrapper that wraps the `opencensus::tag::measure` for using it simply.
class Metric {
public:
+1 -1
View File
@@ -29,7 +29,7 @@ class MockExporter : public opencensus::stats::StatsExporter::Handler {
ASSERT_EQ(opencensus::stats::ViewData::Type::kDouble, view_data.type());
for (const auto row : view_data.double_data()) {
for (int i = 0; i < descriptor.columns().size(); ++i) {
for (size_t i = 0; i < descriptor.columns().size(); ++i) {
if (descriptor.columns()[i].name() == "NodeAddress") {
ASSERT_EQ("Localhost", row.first[i]);
}
-1
View File
@@ -138,7 +138,6 @@ void RayLog::StartRayLog(const std::string &app_name, RayLogLevel severity_thres
log_dir_ = log_dir;
#ifdef RAY_USE_GLOG
google::InitGoogleLogging(app_name_.c_str());
int mapped_severity_threshold = GetMappedSeverity(severity_threshold_);
google::SetStderrLogging(GetMappedSeverity(RayLogLevel::ERROR));
for (int i = static_cast<int>(severity_threshold_);
i <= static_cast<int>(RayLogLevel::FATAL); ++i) {