Skip to content

Commit

Permalink
[Lint] One parameter/argument per line for C++ code (ray-project#22725)
Browse files Browse the repository at this point in the history
It's really annoying to deal with parameter/argument conflicts. This is even frustrating when we merge code from the community to Ant's internal code base with hundreds of conflicts caused by parameters/arguments.

In this PR, I updated the clang-format style to make parameters/arguments stay on different lines if they can't fit into a single line.

There are several benefits:

* Conflict resolving is easier.
* Less potential human mistakes when resolving conflicts.
* Git history and Git blame are more straightforward.
* Better readability.
* Align with the new Python format style.
  • Loading branch information
kfstorm authored Mar 13, 2022
1 parent f15bcb2 commit e9755d8
Show file tree
Hide file tree
Showing 396 changed files with 10,792 additions and 5,255 deletions.
2 changes: 2 additions & 0 deletions .clang-format
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,5 @@ ColumnLimit: 90
DerivePointerAlignment: false
IndentCaseLabels: false
PointerAlignment: Right
BinPackArguments: false
BinPackParameters: false
7 changes: 4 additions & 3 deletions ci/generate_compile_commands/extract_compile_command.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ namespace {
using ::google::protobuf::io::CodedInputStream;
using ::google::protobuf::io::FileInputStream;

bool ReadExtraAction(const std::string &path, blaze::ExtraActionInfo *info,
bool ReadExtraAction(const std::string &path,
blaze::ExtraActionInfo *info,
blaze::CppCompileInfo *cpp_info) {
int fd = ::open(path.c_str(), O_RDONLY, S_IREAD | S_IWRITE);
if (fd < 0) {
Expand Down Expand Up @@ -97,8 +98,8 @@ int main(int argc, char **argv) {

std::vector<std::string> args;
args.push_back(cpp_info.tool());
args.insert(args.end(), cpp_info.compiler_option().begin(),
cpp_info.compiler_option().end());
args.insert(
args.end(), cpp_info.compiler_option().begin(), cpp_info.compiler_option().end());
if (std::find(args.begin(), args.end(), "-c") == args.end()) {
args.push_back("-c");
args.push_back(cpp_info.source_file());
Expand Down
13 changes: 8 additions & 5 deletions cpp/include/ray/api.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ std::vector<std::shared_ptr<T>> Get(const std::vector<ray::ObjectRef<T>> &object
/// \return Two arrays, one containing locally available objects, one containing the
/// rest.
template <typename T>
WaitResult<T> Wait(const std::vector<ray::ObjectRef<T>> &objects, int num_objects,
WaitResult<T> Wait(const std::vector<ray::ObjectRef<T>> &objects,
int num_objects,
int timeout_ms);

/// Create a `TaskCaller` for calling remote function.
Expand Down Expand Up @@ -196,7 +197,8 @@ inline std::vector<std::shared_ptr<T>> Get(const std::vector<ray::ObjectRef<T>>
}

template <typename T>
inline WaitResult<T> Wait(const std::vector<ray::ObjectRef<T>> &objects, int num_objects,
inline WaitResult<T> Wait(const std::vector<ray::ObjectRef<T>> &objects,
int num_objects,
int timeout_ms) {
auto object_ids = ObjectRefsToObjectIDs<T>(objects);
auto results =
Expand All @@ -214,9 +216,10 @@ inline WaitResult<T> Wait(const std::vector<ray::ObjectRef<T>> &objects, int num
}

inline ray::internal::ActorCreator<PyActorClass> Actor(PyActorClass func) {
ray::internal::RemoteFunctionHolder remote_func_holder(
func.module_name, func.function_name, func.class_name,
ray::internal::LangType::PYTHON);
ray::internal::RemoteFunctionHolder remote_func_holder(func.module_name,
func.function_name,
func.class_name,
ray::internal::LangType::PYTHON);
return {ray::internal::GetRayRuntime().get(), std::move(remote_func_holder)};
}

Expand Down
6 changes: 4 additions & 2 deletions cpp/include/ray/api/actor_creator.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,13 +80,15 @@ ActorHandle<GetActorType<F>, is_python_v<F>> ActorCreator<F>::Remote(Args &&...a

if constexpr (is_python_v<F>) {
using ArgsTuple = std::tuple<Args...>;
Arguments::WrapArgs<ArgsTuple>(/*cross_lang=*/true, &args_,
Arguments::WrapArgs<ArgsTuple>(/*cross_lang=*/true,
&args_,
std::make_index_sequence<sizeof...(Args)>{},
std::forward<Args>(args)...);
} else {
StaticCheck<F, Args...>();
using ArgsTuple = RemoveReference_t<boost::callable_traits::args_t<F>>;
Arguments::WrapArgs<ArgsTuple>(/*cross_lang=*/false, &args_,
Arguments::WrapArgs<ArgsTuple>(/*cross_lang=*/false,
&args_,
std::make_index_sequence<sizeof...(Args)>{},
std::forward<Args>(args)...);
}
Expand Down
4 changes: 2 additions & 2 deletions cpp/include/ray/api/actor_handle.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ class ActorHandle {
std::is_same<ActorType, Self>::value || std::is_base_of<Self, ActorType>::value,
"Class types must be same.");
ray::internal::RemoteFunctionHolder remote_func_holder(actor_func);
return ray::internal::ActorTaskCaller<F>(internal::GetRayRuntime().get(), id_,
std::move(remote_func_holder));
return ray::internal::ActorTaskCaller<F>(
internal::GetRayRuntime().get(), id_, std::move(remote_func_holder));
}

template <typename R>
Expand Down
9 changes: 6 additions & 3 deletions cpp/include/ray/api/actor_task_caller.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ class ActorTaskCaller {
public:
ActorTaskCaller() = default;

ActorTaskCaller(RayRuntime *runtime, const std::string &id,
ActorTaskCaller(RayRuntime *runtime,
const std::string &id,
RemoteFunctionHolder remote_function_holder)
: runtime_(runtime),
id_(id),
Expand Down Expand Up @@ -68,13 +69,15 @@ ObjectRef<boost::callable_traits::return_type_t<F>> ActorTaskCaller<F>::Remote(

if constexpr (is_python_v<F>) {
using ArgsTuple = std::tuple<Args...>;
Arguments::WrapArgs<ArgsTuple>(/*cross_lang=*/true, &args_,
Arguments::WrapArgs<ArgsTuple>(/*cross_lang=*/true,
&args_,
std::make_index_sequence<sizeof...(Args)>{},
std::forward<Args>(args)...);
} else {
StaticCheck<F, Args...>();
using ArgsTuple = RemoveReference_t<RemoveFirst_t<boost::callable_traits::args_t<F>>>;
Arguments::WrapArgs<ArgsTuple>(/*cross_lang=*/false, &args_,
Arguments::WrapArgs<ArgsTuple>(/*cross_lang=*/false,
&args_,
std::make_index_sequence<sizeof...(Args)>{},
std::forward<Args>(args)...);
}
Expand Down
12 changes: 8 additions & 4 deletions cpp/include/ray/api/arguments.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ namespace internal {
class Arguments {
public:
template <typename OriginArgType, typename InputArgTypes>
static void WrapArgsImpl(bool cross_lang, std::vector<TaskArg> *task_args,
static void WrapArgsImpl(bool cross_lang,
std::vector<TaskArg> *task_args,
InputArgTypes &&arg) {
if constexpr (is_object_ref_v<OriginArgType>) {
PushReferenceArg(task_args, std::forward<InputArgTypes>(arg));
Expand Down Expand Up @@ -65,8 +66,10 @@ class Arguments {
}

template <typename OriginArgsTuple, size_t... I, typename... InputArgTypes>
static void WrapArgs(bool cross_lang, std::vector<TaskArg> *task_args,
std::index_sequence<I...>, InputArgTypes &&...args) {
static void WrapArgs(bool cross_lang,
std::vector<TaskArg> *task_args,
std::index_sequence<I...>,
InputArgTypes &&...args) {
(void)std::initializer_list<int>{
(WrapArgsImpl<std::tuple_element_t<I, OriginArgsTuple>>(
cross_lang, task_args, std::forward<InputArgTypes>(args)),
Expand All @@ -77,7 +80,8 @@ class Arguments {
}

private:
static void PushValueArg(std::vector<TaskArg> *task_args, msgpack::sbuffer &&buffer,
static void PushValueArg(std::vector<TaskArg> *task_args,
msgpack::sbuffer &&buffer,
std::string_view meta_str = "") {
/// Pass by value.
TaskArg task_arg;
Expand Down
33 changes: 21 additions & 12 deletions cpp/include/ray/api/function_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ struct Invoker {
return result;
}

static inline msgpack::sbuffer ApplyMember(const Function &func, msgpack::sbuffer *ptr,
static inline msgpack::sbuffer ApplyMember(const Function &func,
msgpack::sbuffer *ptr,
const ArgsBufferList &args_buffer) {
using RetrunType = boost::callable_traits::return_type_t<Function>;
using ArgsTuple =
Expand Down Expand Up @@ -124,7 +125,8 @@ struct Invoker {
}
}

static inline bool GetArgsTuple(std::tuple<> &tup, const ArgsBufferList &args_buffer,
static inline bool GetArgsTuple(std::tuple<> &tup,
const ArgsBufferList &args_buffer,
std::index_sequence<>) {
return true;
}
Expand Down Expand Up @@ -155,7 +157,8 @@ struct Invoker {
}

template <typename R, typename F, size_t... I, typename... Args>
static R CallInternal(const F &f, const std::index_sequence<I...> &,
static R CallInternal(const F &f,
const std::index_sequence<I...> &,
std::tuple<Args...> args) {
(void)args;
using ArgsTuple = boost::callable_traits::args_t<F>;
Expand All @@ -165,21 +168,23 @@ struct Invoker {
template <typename R, typename F, typename Self, typename... Args>
static std::enable_if_t<std::is_void<R>::value, msgpack::sbuffer> CallMember(
const F &f, Self *self, std::tuple<Args...> args) {
CallMemberInternal<R>(f, self, std::make_index_sequence<sizeof...(Args)>{},
std::move(args));
CallMemberInternal<R>(
f, self, std::make_index_sequence<sizeof...(Args)>{}, std::move(args));
return PackVoid();
}

template <typename R, typename F, typename Self, typename... Args>
static std::enable_if_t<!std::is_void<R>::value, msgpack::sbuffer> CallMember(
const F &f, Self *self, std::tuple<Args...> args) {
auto r = CallMemberInternal<R>(f, self, std::make_index_sequence<sizeof...(Args)>{},
std::move(args));
auto r = CallMemberInternal<R>(
f, self, std::make_index_sequence<sizeof...(Args)>{}, std::move(args));
return PackReturnValue(r);
}

template <typename R, typename F, typename Self, size_t... I, typename... Args>
static R CallMemberInternal(const F &f, Self *self, const std::index_sequence<I...> &,
static R CallMemberInternal(const F &f,
Self *self,
const std::index_sequence<I...> &,
std::tuple<Args...> args) {
(void)args;
using ArgsTuple = boost::callable_traits::args_t<F>;
Expand Down Expand Up @@ -288,16 +293,20 @@ class FunctionManager {
template <typename Function>
bool RegisterNonMemberFunc(std::string const &name, Function f) {
return map_invokers_
.emplace(name, std::bind(&Invoker<Function>::Apply, std::move(f),
std::placeholders::_1))
.emplace(
name,
std::bind(&Invoker<Function>::Apply, std::move(f), std::placeholders::_1))
.second;
}

template <typename Function>
bool RegisterMemberFunc(std::string const &name, Function f) {
return map_mem_func_invokers_
.emplace(name, std::bind(&Invoker<Function>::ApplyMember, std::move(f),
std::placeholders::_1, std::placeholders::_2))
.emplace(name,
std::bind(&Invoker<Function>::ApplyMember,
std::move(f),
std::placeholders::_1,
std::placeholders::_2))
.second;
}

Expand Down
3 changes: 2 additions & 1 deletion cpp/include/ray/api/logging.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ class RayLogger {
virtual std::ostream &Stream() = 0;
};

std::unique_ptr<RayLogger> CreateRayLogger(const char *file_name, int line_number,
std::unique_ptr<RayLogger> CreateRayLogger(const char *file_name,
int line_number,
RayLoggerLevel severity);
bool IsLevelEnabled(RayLoggerLevel log_level);

Expand Down
2 changes: 2 additions & 0 deletions cpp/include/ray/api/ray_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@

#pragma once
#include <ray/api/ray_exception.h>

#include <memory>
#include <string>
#include <unordered_map>
#include <vector>

#include "boost/optional.hpp"

namespace ray {
Expand Down
3 changes: 2 additions & 1 deletion cpp/include/ray/api/ray_remote.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ inline static int RegisterRemoteFunctions(const T &t, U... u) {
(void)std::initializer_list<int>{
(FunctionManager::Instance().RegisterRemoteFunction(
std::string(func_names[index].data(), func_names[index].length()), u),
index++, 0)...};
index++,
0)...};
return 0;
}

Expand Down
9 changes: 6 additions & 3 deletions cpp/include/ray/api/ray_runtime.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ namespace internal {

struct RemoteFunctionHolder {
RemoteFunctionHolder() = default;
RemoteFunctionHolder(const std::string &module_name, const std::string &function_name,
RemoteFunctionHolder(const std::string &module_name,
const std::string &function_name,
const std::string &class_name = "",
LangType lang_type = LangType::CPP) {
this->module_name = module_name;
Expand Down Expand Up @@ -61,7 +62,8 @@ class RayRuntime {
virtual std::vector<std::shared_ptr<msgpack::sbuffer>> Get(
const std::vector<std::string> &ids) = 0;

virtual std::vector<bool> Wait(const std::vector<std::string> &ids, int num_objects,
virtual std::vector<bool> Wait(const std::vector<std::string> &ids,
int num_objects,
int timeout_ms) = 0;

virtual std::string Call(const RemoteFunctionHolder &remote_function_holder,
Expand All @@ -71,7 +73,8 @@ class RayRuntime {
std::vector<TaskArg> &args,
const ActorCreationOptions &create_options) = 0;
virtual std::string CallActor(const RemoteFunctionHolder &remote_function_holder,
const std::string &actor, std::vector<TaskArg> &args,
const std::string &actor,
std::vector<TaskArg> &args,
const CallOptions &call_options) = 0;
virtual void AddLocalReference(const std::string &id) = 0;
virtual void RemoveLocalReference(const std::string &id) = 0;
Expand Down
6 changes: 4 additions & 2 deletions cpp/include/ray/api/task_caller.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,15 @@ ObjectRef<boost::callable_traits::return_type_t<F>> TaskCaller<F>::Remote(

if constexpr (is_python_v<F>) {
using ArgsTuple = std::tuple<Args...>;
Arguments::WrapArgs<ArgsTuple>(/*cross_lang=*/true, &args_,
Arguments::WrapArgs<ArgsTuple>(/*cross_lang=*/true,
&args_,
std::make_index_sequence<sizeof...(Args)>{},
std::forward<Args>(args)...);
} else {
StaticCheck<F, Args...>();
using ArgsTuple = RemoveReference_t<boost::callable_traits::args_t<F>>;
Arguments::WrapArgs<ArgsTuple>(/*cross_lang=*/false, &args_,
Arguments::WrapArgs<ArgsTuple>(/*cross_lang=*/false,
&args_,
std::make_index_sequence<sizeof...(Args)>{},
std::forward<Args>(args)...);
}
Expand Down
3 changes: 2 additions & 1 deletion cpp/include/ray/api/task_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ struct PlacementGroupCreationOptions {
class PlacementGroup {
public:
PlacementGroup() = default;
PlacementGroup(std::string id, PlacementGroupCreationOptions options,
PlacementGroup(std::string id,
PlacementGroupCreationOptions options,
PlacementGroupState state = PlacementGroupState::UNRECOGNIZED)
: id_(std::move(id)), options_(std::move(options)), state_(state) {}
std::string GetID() const { return id_; }
Expand Down
4 changes: 2 additions & 2 deletions cpp/include/ray/api/wait_result.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@

#pragma once

#include <list>

#include <ray/api/object_ref.h>

#include <list>

namespace ray {

/// \param T The type of object.
Expand Down
Loading

0 comments on commit e9755d8

Please sign in to comment.