Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add clang-tidy integration #20

Merged
merged 2 commits into from
Jan 24, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
---
# NOTE: there must be no spaces before the '-', so put the comma last.
InheritParentConfig: true
Checks: '
abseil-*,
bugprone-*,
-bugprone-easily-swappable-parameters,
clang-analyzer-*,
cppcoreguidelines-*,
XuehaiPan marked this conversation as resolved.
Show resolved Hide resolved
-cppcoreguidelines-avoid-non-const-global-variables,
-cppcoreguidelines-macro-usage,
-cppcoreguidelines-pro-type-reinterpret-cast,
hicpp-exception-baseclass,
hicpp-avoid-goto,
modernize-*,
-modernize-use-trailing-return-type,
performance-*,
readability-*,
-readability-identifier-length,
-readability-convert-member-functions-to-static,
misc-*,
XuehaiPan marked this conversation as resolved.
Show resolved Hide resolved
-misc-const-correctness,
'
HeaderFilterRegex: '^include/.*$'
WarningsAsErrors: '*'
FormatStyle: file
2 changes: 1 addition & 1 deletion .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ What types of changes does your code introduce? Put an `x` in all the boxes that
Go over all the following points, and put an `x` in all the boxes that apply.
If you are unsure about any of these, don't hesitate to ask. We are here to help!

- [ ] I have read the [CONTRIBUTION](https://optree.readthedocs.io/en/latest/developer/contributing.html) guide. (**required**)
- [ ] I have read the [CONTRIBUTION](https://github.com/metaopt/optree/blob/HEAD/CONTRIBUTING.md) guide. (**required**)
- [ ] My change requires a change to the documentation.
- [ ] I have updated the tests accordingly. (*required for a bug fix or a new feature*)
- [ ] I have updated the documentation accordingly.
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ jobs:
run: |
make clang-format

- name: clang-tidy
run: |
make clang-tidy

- name: addlicense
run: |
make addlicense
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,7 @@ DerivedData/
# *.ipr

# CMake
cmake-build/
cmake-build-*/

# Mongo Explorer plugin
Expand Down
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ We use several tools to secure code quality, including:

- PEP8 code style: `black`, `isort`, `pylint`, `flake8`
- Type hint check: `mypy`
- C++ Google-style: `cpplint`, `clang-format`
- C++ code style: `cpplint`, `clang-format`, `clang-tidy`
- License: `addlicense`
- Documentation: `pydocstyle`, `doc8`

Expand Down
3 changes: 3 additions & 0 deletions CPPLINT.cfg
Original file line number Diff line number Diff line change
@@ -1 +1,4 @@
linelength=100
filter=-readability/nolint
filter=-readability/braces
filter=-whitespace/newline
22 changes: 20 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ pre-commit-install:
$(PYTHON) -m pre_commit install --install-hooks

docs-install:
$(call check_pip_install,pydocstyle)
$(call check_pip_install_extra,pydocstyle,pydocstyle[toml])
$(call check_pip_install_extra,doc8,"doc8<1.0.0a0")
if ! $(PYTHON) -c "import sys; exit(sys.version_info < (3, 8))"; then \
$(PYTHON) -m pip uninstall --yes importlib-metadata; \
Expand All @@ -78,6 +78,9 @@ pytest-install:
$(call check_pip_install,pytest-cov)
$(call check_pip_install,pytest-xdist)

cmake-install:
command -v cmake || $(call check_pip_install,cmake)

cpplint-install:
$(call check_pip_install,cpplint)

Expand Down Expand Up @@ -126,12 +129,27 @@ pre-commit: pre-commit-install

# C++ linters

cmake-configure: cmake-install
cmake -S . -B cmake-build-debug \
-DCMAKE_BUILD_TYPE=Debug \
-DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
-DPYTHON_EXECUTABLE="$(PYTHON)" \
-DOPTREE_CXX_WERROR="$(OPTREE_CXX_WERROR)"

cmake-build: cmake-configure
cmake --build cmake-build-debug --parallel

cmake: cmake-build

cpplint: cpplint-install
$(PYTHON) -m cpplint $(CXX_FILES)

clang-format: clang-format-install
$(CLANG_FORMAT) --style=file -i $(CXX_FILES) -n --Werror

clang-tidy: clang-tidy-install cmake-configure
clang-tidy -p=cmake-build-debug $(CXX_FILES)

# Documentation

addlicense: addlicense-install
Expand All @@ -153,7 +171,7 @@ clean-docs:

# Utility functions

lint: flake8 py-format mypy pylint clang-format cpplint addlicense docstyle spelling
lint: flake8 py-format mypy pylint clang-format clang-tidy cpplint addlicense docstyle spelling

format: py-format-install clang-format-install addlicense-install
$(PYTHON) -m isort --project $(PROJECT_NAME) $(PYTHON_FILES)
Expand Down
2 changes: 1 addition & 1 deletion include/registry.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright 2022 MetaOPT Team. All Rights Reserved.
Copyright 2022-2023 MetaOPT Team. All Rights Reserved.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand Down
52 changes: 26 additions & 26 deletions include/treespec.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ class PyTreeSpec {

// Recursive helper used to implement Flatten().
bool FlattenInto(const py::handle &handle,
std::vector<py::object> &leaves, // NOLINT
std::vector<py::object> &leaves, // NOLINT[runtime/references]
const std::optional<py::function> &leaf_predicate,
const bool &none_is_leaf,
const std::string &registry_namespace);
Expand All @@ -82,27 +82,27 @@ class PyTreeSpec {

// Recursive helper used to implement FlattenWithPath().
bool FlattenIntoWithPath(const py::handle &handle,
std::vector<py::object> &leaves, // NOLINT
std::vector<py::object> &paths, // NOLINT
std::vector<py::object> &leaves, // NOLINT[runtime/references]
std::vector<py::object> &paths, // NOLINT[runtime/references]
const std::optional<py::function> &leaf_predicate,
const bool &none_is_leaf,
const std::string &registry_namespace);

// Flattens a PyTree up to this PyTreeSpec. 'this' must be a tree prefix of the tree-structure
// of 'x'. For example, if we flatten a value [(1, (2, 3)), {"foo": 4}] with a PyTreeSpec [(*,
// *), *], the result is the list of leaves [1, (2, 3), {"foo": 4}].
py::list FlattenUpTo(const py::handle &full_tree) const;
[[nodiscard]] py::list FlattenUpTo(const py::handle &full_tree) const;

// Tests whether the given list is a flat list of leaves.
static bool AllLeaves(const py::iterable &iterable,
const bool &none_is_leaf = false,
const std::string &registry_namespace = "");

// Returns an unflattened PyTree given an iterable of leaves and a PyTreeSpec.
py::object Unflatten(const py::iterable &leaves) const;
[[nodiscard]] py::object Unflatten(const py::iterable &leaves) const;

// Composes two PyTreeSpecs, replacing the leaves of this tree with copies of `inner`.
std::unique_ptr<PyTreeSpec> Compose(const PyTreeSpec &inner_treespec) const;
[[nodiscard]] std::unique_ptr<PyTreeSpec> Compose(const PyTreeSpec &inner_treespec) const;

// Makes a Tuple PyTreeSpec out of a vector of PyTreeSpecs.
static std::unique_ptr<PyTreeSpec> Tuple(const std::vector<PyTreeSpec> &treespecs,
Expand All @@ -114,30 +114,30 @@ class PyTreeSpec {
// Makes a PyTreeSpec representing a `None` node.
static std::unique_ptr<PyTreeSpec> None(const bool &none_is_leaf);

std::vector<std::unique_ptr<PyTreeSpec>> Children() const;
[[nodiscard]] std::vector<std::unique_ptr<PyTreeSpec>> Children() const;

// Maps a function over a PyTree structure, applying f_leaf to each leaf, and
// f_node(children, node_data) to each container node.
py::object Walk(const py::function &f_node,
const py::handle &f_leaf,
const py::iterable &leaves) const;
[[nodiscard]] py::object Walk(const py::function &f_node,
const py::handle &f_leaf,
const py::iterable &leaves) const;

ssize_t num_leaves() const;
[[nodiscard]] ssize_t num_leaves() const;

ssize_t num_nodes() const;
[[nodiscard]] ssize_t num_nodes() const;

ssize_t num_children() const;
[[nodiscard]] ssize_t num_children() const;

bool get_none_is_leaf() const;
[[nodiscard]] bool get_none_is_leaf() const;

std::string get_namespace() const;
[[nodiscard]] std::string get_namespace() const;

bool operator==(const PyTreeSpec &other) const;
bool operator!=(const PyTreeSpec &other) const;

template <typename H>
friend H AbslHashValue(H h, const Node &n) {
ssize_t data_hash{0};
ssize_t data_hash = 0;
switch (n.kind) {
case PyTreeKind::Custom:
// We don't hash node_data custom node types since they may not hashable.
Expand All @@ -154,7 +154,7 @@ class PyTreeSpec {
case PyTreeKind::OrderedDict:
case PyTreeKind::DefaultDict: {
py::list keys;
if (n.kind == PyTreeKind::DefaultDict) [[unlikely]] { // NOLINT
if (n.kind == PyTreeKind::DefaultDict) [[unlikely]] {
EXPECT_EQ(
GET_SIZE<py::tuple>(n.node_data), 2, "Number of auxiliary data mismatch.");
py::object default_factory = GET_ITEM_BORROW<py::tuple>(n.node_data, 0);
Expand All @@ -164,7 +164,7 @@ class PyTreeSpec {
n.arity,
"Number of keys and entries does not match.");
data_hash = py::hash(default_factory);
} else [[likely]] { // NOLINT
} else [[likely]] {
EXPECT_EQ(GET_SIZE<py::list>(n.node_data),
n.arity,
"Number of keys and entries does not match.");
Expand All @@ -190,11 +190,11 @@ class PyTreeSpec {
return h;
}

std::string ToString() const;
[[nodiscard]] std::string ToString() const;

// Transforms the PyTreeSpec into a picklable object.
// Used to implement `PyTreeSpec.__getstate__`.
py::object ToPicklable() const;
[[nodiscard]] py::object ToPicklable() const;

// Transforms the object returned by `ToPicklable()` back to PyTreeSpec.
// Used to implement `PyTreeSpec.__setstate__`.
Expand Down Expand Up @@ -240,7 +240,7 @@ class PyTreeSpec {
// Whether to treat `None` as a leaf. If false, `None` is a non-leaf node with arity 0.
bool m_none_is_leaf = false;

// The registry namespace used to resolve the custom pytree node types
// The registry namespace used to resolve the custom pytree node types.
std::string m_namespace;

// Helper that manufactures an instance of a node given its children.
Expand All @@ -254,21 +254,21 @@ class PyTreeSpec {

template <bool NoneIsLeaf, typename Span>
bool FlattenIntoImpl(const py::handle &handle,
Span &leaves, // NOLINT
Span &leaves, // NOLINT[runtime/references]
const ssize_t &depth,
const std::optional<py::function> &leaf_predicate,
const std::string &registry_namespace);

template <bool NoneIsLeaf, typename Span, typename Stack>
bool FlattenIntoWithPathImpl(const py::handle &handle,
Span &leaves, // NOLINT
Span &paths, // NOLINT
Stack &stack, // NOLINT
Span &leaves, // NOLINT[runtime/references]
Span &paths, // NOLINT[runtime/references]
Stack &stack, // NOLINT[runtime/references]
const ssize_t &depth,
const std::optional<py::function> &leaf_predicate,
const std::string &registry_namespace);

py::list FlattenUpToImpl(const py::handle &full_tree) const;
[[nodiscard]] py::list FlattenUpToImpl(const py::handle &full_tree) const;

template <bool NoneIsLeaf>
static bool AllLeavesImpl(const py::iterable &iterable, const std::string &registry_namespace);
Expand Down
17 changes: 9 additions & 8 deletions include/utils.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright 2022 MetaOPT Team. All Rights Reserved.
Copyright 2022-2023 MetaOPT Team. All Rights Reserved.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -46,25 +46,25 @@ using ssize_t = py::ssize_t;

inline const py::module& ImportCollections() {
// NOTE: Use raw pointers to leak the memory intentionally to avoid py::object deallocation and
// garbage collection
// garbage collection.
static const py::module* ptr = new py::module{py::module::import("collections")};
return *ptr;
}
inline const py::object& ImportOrderedDict() {
// NOTE: Use raw pointers to leak the memory intentionally to avoid py::object deallocation and
// garbage collection
// garbage collection.
static const py::object* ptr = new py::object{py::getattr(PyCollectionsModule, "OrderedDict")};
return *ptr;
}
inline const py::object& ImportDefaultDict() {
// NOTE: Use raw pointers to leak the memory intentionally to avoid py::object deallocation and
// garbage collection
// garbage collection.
static const py::object* ptr = new py::object{py::getattr(PyCollectionsModule, "defaultdict")};
return *ptr;
}
inline const py::object& ImportDeque() {
// NOTE: Use raw pointers to leak the memory intentionally to avoid py::object deallocation and
// garbage collection
// garbage collection.
static const py::object* ptr = new py::object{py::getattr(PyCollectionsModule, "deque")};
return *ptr;
}
Expand All @@ -85,11 +85,12 @@ inline py::list SortedDictKeys(const py::dict& dict) {

try {
// Sort directly if possible.
// NOLINTNEXTLINE[readability-implicit-bool-conversion]
if (PyList_Sort(keys.ptr())) [[unlikely]] {
throw py::error_already_set();
}
} catch (py::error_already_set& ex1) {
if (ex1.matches(PyExc_TypeError)) [[likely]] { // NOLINT
if (ex1.matches(PyExc_TypeError)) [[likely]] {
// Found incomparable keys (e.g. `int` vs. `str`, or user-defined types).
try {
// Sort with `(f'{o.__class__.__module__}.{o.__class__.__qualname__}', o)`
Expand All @@ -107,11 +108,11 @@ inline py::list SortedDictKeys(const py::dict& dict) {
// Found incomparable user-defined key types.
// The keys remain in the insertion order.
PyErr_Clear();
} else [[unlikely]] { // NOLINT
} else [[unlikely]] {
throw;
}
}
} else [[unlikely]] { // NOLINT
} else [[unlikely]] {
throw;
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/optree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ limitations under the License.

namespace optree {

void BuildModule(py::module& mod) { // NOLINT
void BuildModule(py::module& mod) { // NOLINT[runtime/references]
mod.doc() = "Optimized PyTree Utilities.";
py::register_local_exception<InternalError>(mod, "InternalError", PyExc_RuntimeError);
mod.attr("MAX_RECURSION_DEPTH") = MAX_RECURSION_DEPTH;
Expand Down Expand Up @@ -144,4 +144,5 @@ void BuildModule(py::module& mod) { // NOLINT

} // namespace optree

// NOLINTNEXTLINE[cppcoreguidelines-pro-bounds-pointer-arithmetic,cppcoreguidelines-pro-type-vararg]
PYBIND11_MODULE(_C, mod) { optree::BuildModule(mod); }
Loading