feat: add clang-tidy integration

.clang-tidy

@@ -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-*,
+-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-*,
+-misc-const-correctness,
+'
+HeaderFilterRegex: '^include/.*$'
+WarningsAsErrors: '*'
+FormatStyle: file

.github/PULL_REQUEST_TEMPLATE.md

@@ -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.

.github/workflows/lint.yml

@@ -67,6 +67,10 @@ jobs:
         run: |
           make clang-format
 
+      - name: clang-tidy
+        run: |
+          make clang-tidy
+
       - name: addlicense
         run: |
           make addlicense

.gitignore

@@ -348,6 +348,7 @@ DerivedData/
 # *.ipr
 
 # CMake
+cmake-build/
 cmake-build-*/
 
 # Mongo Explorer plugin

CONTRIBUTING.md

@@ -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`
 

CPPLINT.cfg

@@ -1 +1,4 @@
 linelength=100
+filter=-readability/nolint
+filter=-readability/braces
+filter=-whitespace/newline

Makefile

@@ -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; \
@@ -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)
 
@@ -126,12 +129,28 @@ 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)" \
+		-DCMAKE_LIBRARY_OUTPUT_DIRECTORY_DEBUG="cmake-build-debug" \
+		-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
@@ -153,7 +172,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)

include/registry.h

@@ -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.

include/treespec.h

@@ -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);
@@ -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,
@@ -114,23 +114,23 @@ 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;
@@ -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);
@@ -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.");
@@ -179,13 +179,8 @@ class PyTreeSpec {
                 INTERNAL_ERROR();
         }
 
-        h = H::combine(std::move(h),
-                       n.kind,
-                       n.arity,
-                       n.custom,
-                       n.num_leaves,
-                       n.num_nodes,
-                       std::move(data_hash));
+        h = H::combine(
+            std::move(h), n.kind, n.arity, n.custom, n.num_leaves, n.num_nodes, data_hash);
         return h;
     }
 
@@ -195,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__`.
@@ -245,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.
@@ -259,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);

include/utils.h

@@ -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.
@@ -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;
 }
@@ -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)`
@@ -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;
         }
     }

src/optree.cpp

@@ -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;
@@ -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); }