Skip to content

Commit

Permalink
Automate include grouping order in .clang-format (#15063)
Browse files Browse the repository at this point in the history
This uses the `IncludeCategories` settings in .clang-format to attempt to enforce our documented `#include` order in libcudf. See https://docs.rapids.ai/api/libcudf/stable/developer_guide

I realize that there was a [previous attempt at this](#12760) by @bdice that met with some resistance. Reading it, I wouldn't say it was vetoed; rather, reviewers requested something much simpler. I have a few reasons to attempt this again.

1. To make a separate task much easier. We are undertaking a refactoring of RMM that will replace `rmm::mr::device_memory_resource*` with `rmm::device_async_resource-ref` everywhere in RAPIDS (not just cuDF). This requires adding an include to MANY files. Getting the location of the include right everywhere is very difficult without automatic grouping of headers. I started out writing a bash script to do this before realizing clang-format has the necessary feature. And I realized that my script would never properly handle [files like this](https://github.com/rapidsai/raft/blob/branch-24.04/cpp/bench/ann/src/raft/raft_cagra_wrapper.h).
2. To increase velocity. Everywhere in RAPIDS that we have automated code standard/style/formatting/other, the benefits to velocity have outweighed the costs. To paraphrase @bdice, $auto \nearrow \rightarrow \mu \searrow \rightarrow v \nearrow$
3. The previous PR #12760 had nearly 50 categories of headers. There was no way this could be applied universally across RAPIDS repos. My proposal has 10 categories. I tried to reduce it further but realized that it wouldn't be much less configuration to maintain, so I stopped at 10.

Note that one of the ways that having few categories can work while still maintaining clear groups is that this PR updates many files to use quotes ("") instead of angle brackets (<>) for local cuDF headers that do not live in `cudf/cpp/include`. With our "near to far" include ordering policy, these are arguably the nearest files, and using quotes allows us to have our first category simply check for quotes. These files will be grouped and sorted without blank lines, but in practice this does not lose clarity because typically headers from more than two directories are not included from the same file. The downside of this change is I don't yet know how to automatically enforce it. I hope that when developers accidentally use <> for internal includes that don't start with (e.g.) "cudf", they will be grouped one of the lowest priority categories, and perhaps this will induce them to switch to "" to get the headers listed at the top. The rule is simple: if it's in libcudf but not in `cpp/include/cudf`, then use quotes. For **everything** else, use angle brackets.

Other than headers from RAPIDS repos, we have a group for all CCCL/CUDA headers, a group for all other headers that have a file extension, and a final group for all files that have no file extension (e.g. STL).

Below I'm listing the (fairly simple, in my opinion) .clang-format settings for this PR. Note that categories 2-5 will require tweaking for different RAPIDS repos. 

Some may ask why I ordered `cudf_test` headers before `cudf` headers. I tried both orders, and putting `cudf_test` first generated significantly fewer changes in the PR, meaning that it's already the more common ordering (I suppose `cudf_test` is closer to the files that include it, since they are libcudf tests).

I've opened a similar PR for RMM with only 5 groups. rapidsai/rmm#1463

CC @davidwendt @vyasr @wence- @GregoryKimball for feedback

@isVoid contributed to this PR via pair programming.

```
IncludeBlocks: Regroup
IncludeCategories:
  - Regex:           '^"' # quoted includes
    Priority:        1
  - Regex:           '^<(benchmarks|tests)/' # benchmark includes
    Priority:        2
  - Regex:           '^<cudf_test/' # cuDF includes
    Priority:        3
  - Regex:           '^<cudf/' # cuDF includes
    Priority:        4
  - Regex:           '^<(nvtext|cudf_kafka)' # other libcudf includes
    Priority:        5
  - Regex:           '^<(cugraph|cuml|cuspatial|raft|kvikio)' # Other RAPIDS includes
    Priority:        6
  - Regex:           '^<rmm/' # RMM includes
    Priority:        7
  - Regex:           '^<(thrust|cub|cuda)/' # CCCL includes
    Priority:        8
  - Regex:           '^<(cooperative_groups|cuco|cuda.h|cuda_runtime|device_types|math_constants|nvtx3)' # CUDA includes
    Priority:        8
  - Regex:           '^<.*\..*' # other system includes (e.g. with a '.')
    Priority:        9
  - Regex:           '^<[^.]+' # STL includes (no '.')
    Priority:        10
```

Authors:
  - Mark Harris (https://github.com/harrism)

Approvers:
  - David Wendt (https://github.com/davidwendt)
  - Bradley Dice (https://github.com/bdice)
  - Nghia Truong (https://github.com/ttnghia)

URL: #15063
  • Loading branch information
harrism authored Feb 21, 2024
1 parent 4ce99af commit d053323
Show file tree
Hide file tree
Showing 457 changed files with 1,493 additions and 1,531 deletions.
26 changes: 24 additions & 2 deletions .clang-format
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,30 @@ ForEachMacros:
- foreach
- Q_FOREACH
- BOOST_FOREACH
IncludeBlocks: Preserve
IncludeIsMainRegex: '([-_](test|unittest))?$'
IncludeBlocks: Regroup
IncludeCategories:
- Regex: '^"' # quoted includes
Priority: 1
- Regex: '^<(benchmarks|tests)/' # benchmark includes
Priority: 2
- Regex: '^<cudf_test/' # cuDF includes
Priority: 3
- Regex: '^<cudf/' # cuDF includes
Priority: 4
- Regex: '^<(nvtext|cudf_kafka)' # other libcudf includes
Priority: 5
- Regex: '^<(cugraph|cuml|cuspatial|raft|kvikio)' # Other RAPIDS includes
Priority: 6
- Regex: '^<rmm/' # RMM includes
Priority: 7
- Regex: '^<(thrust|cub|cuda)/' # CCCL includes
Priority: 8
- Regex: '^<(cooperative_groups|cuco|cuda.h|cuda_runtime|device_types|math_constants|nvtx3)' # CUDA includes
Priority: 8
- Regex: '^<.*\..*' # other system includes (e.g. with a '.')
Priority: 9
- Regex: '^<[^.]+' # STL includes (no '.')
Priority: 10
IndentCaseLabels: true
IndentPPDirectives: None
IndentWidth: 2
Expand Down
3 changes: 1 addition & 2 deletions cpp/benchmarks/common/generate_input.cu
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include <rmm/device_uvector.hpp>
#include <rmm/mr/device/per_device_resource.hpp>

#include <cuda/functional>
#include <thrust/binary_search.h>
#include <thrust/copy.h>
#include <thrust/device_ptr.h>
Expand All @@ -53,8 +54,6 @@
#include <thrust/transform.h>
#include <thrust/tuple.h>

#include <cuda/functional>

#include <algorithm>
#include <cstdint>
#include <memory>
Expand Down
3 changes: 2 additions & 1 deletion cpp/benchmarks/fixture/benchmark_fixture.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,15 @@

#pragma once

#include <benchmark/benchmark.h>
#include <rmm/cuda_device.hpp>
#include <rmm/mr/device/cuda_memory_resource.hpp>
#include <rmm/mr/device/owning_wrapper.hpp>
#include <rmm/mr/device/per_device_resource.hpp>
#include <rmm/mr/device/pool_memory_resource.hpp>
#include <rmm/mr/device/statistics_resource_adaptor.hpp>

#include <benchmark/benchmark.h>

namespace cudf {

namespace {
Expand Down
5 changes: 3 additions & 2 deletions cpp/benchmarks/io/cuio_common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,17 @@
*/

#include <benchmarks/io/cuio_common.hpp>

#include <cudf/detail/utilities/integer_utils.hpp>
#include <cudf/detail/utilities/logger.hpp>

#include <unistd.h>

#include <cstdio>
#include <fstream>
#include <numeric>
#include <string>

#include <unistd.h>

temp_directory const cuio_source_sink_pair::tmpdir{"cudf_gbench"};

std::string random_file_in_dir(std::string const& dir_path)
Expand Down
9 changes: 4 additions & 5 deletions cpp/benchmarks/io/fst.cu
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2022-2023, NVIDIA CORPORATION.
* Copyright (c) 2022-2024, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -14,11 +14,10 @@
* limitations under the License.
*/

#include <benchmarks/common/generate_input.hpp>

#include <io/fst/lookup_tables.cuh>
#include <io/utilities/hostdevice_vector.hpp> //TODO find better replacement
#include "io/fst/lookup_tables.cuh"
#include "io/utilities/hostdevice_vector.hpp" //TODO find better replacement

#include <benchmarks/common/generate_input.hpp>
#include <tests/io/fst/common.hpp>

#include <cudf/scalar/scalar_factories.hpp>
Expand Down
7 changes: 3 additions & 4 deletions cpp/benchmarks/io/json/nested_json.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2022-2023, NVIDIA CORPORATION.
* Copyright (c) 2022-2024, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -14,11 +14,10 @@
* limitations under the License.
*/

#include "io/json/nested_json.hpp"

#include <benchmarks/common/generate_input.hpp>
#include <benchmarks/fixture/benchmark_fixture.hpp>

#include <io/json/nested_json.hpp>

#include <tests/io/fst/common.hpp>

#include <cudf/scalar/scalar_factories.hpp>
Expand Down
3 changes: 1 addition & 2 deletions cpp/benchmarks/iterator/iterator.cu
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019-2023, NVIDIA CORPORATION.
* Copyright (c) 2019-2024, NVIDIA CORPORATION.
*
* 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 @@ -27,7 +27,6 @@
#include <rmm/device_uvector.hpp>

#include <cub/device/device_reduce.cuh>

#include <thrust/execution_policy.h>
#include <thrust/iterator/counting_iterator.h>
#include <thrust/iterator/transform_iterator.h>
Expand Down
6 changes: 3 additions & 3 deletions cpp/benchmarks/join/join_common.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021-2023, NVIDIA CORPORATION.
* Copyright (c) 2021-2024, NVIDIA CORPORATION.
*
* 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 @@ -31,14 +31,14 @@
#include <cudf/utilities/default_stream.hpp>
#include <cudf/utilities/error.hpp>

#include <nvbench/nvbench.cuh>

#include <thrust/functional.h>
#include <thrust/iterator/counting_iterator.h>
#include <thrust/iterator/transform_iterator.h>
#include <thrust/random/linear_congruential_engine.h>
#include <thrust/random/uniform_int_distribution.h>

#include <nvbench/nvbench.cuh>

#include <vector>

struct null75_generator {
Expand Down
3 changes: 2 additions & 1 deletion cpp/benchmarks/merge/merge.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020-2023, NVIDIA CORPORATION.
* Copyright (c) 2020-2024, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -16,6 +16,7 @@

#include <benchmarks/fixture/benchmark_fixture.hpp>
#include <benchmarks/synchronization/synchronization.hpp>

#include <cudf_test/column_wrapper.hpp>

#include <cudf/column/column.hpp>
Expand Down
6 changes: 3 additions & 3 deletions cpp/benchmarks/sort/rank_lists.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2023, NVIDIA CORPORATION.
* Copyright (c) 2023-2024, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -18,10 +18,10 @@

#include <benchmarks/common/generate_nested_types.hpp>

#include <cudf/sorting.hpp>

#include <cudf_test/column_utilities.hpp>

#include <cudf/sorting.hpp>

#include <nvbench/nvbench.cuh>

template <cudf::rank_method method>
Expand Down
3 changes: 2 additions & 1 deletion cpp/benchmarks/sort/rank_structs.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2023, NVIDIA CORPORATION.
* Copyright (c) 2023-2024, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -15,6 +15,7 @@
*/

#include "rank_types_common.hpp"

#include <benchmarks/common/generate_nested_types.hpp>

#include <cudf/sorting.hpp>
Expand Down
7 changes: 4 additions & 3 deletions cpp/benchmarks/stream_compaction/apply_boolean_mask.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019-2023, NVIDIA CORPORATION.
* Copyright (c) 2019-2024, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -15,11 +15,12 @@
*/

#include <benchmarks/common/generate_input.hpp>
#include <fixture/benchmark_fixture.hpp>
#include <synchronization/synchronization.hpp>

#include <cudf/stream_compaction.hpp>

#include <fixture/benchmark_fixture.hpp>
#include <synchronization/synchronization.hpp>

namespace {

constexpr cudf::size_type hundredM = 1e8;
Expand Down
6 changes: 3 additions & 3 deletions cpp/benchmarks/string/string_bench_args.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021, NVIDIA CORPORATION.
* Copyright (c) 2021-2024, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -15,10 +15,10 @@
*/
#pragma once

#include <benchmark/benchmark.h>

#include <cudf/types.hpp>

#include <benchmark/benchmark.h>

#include <limits>

/**
Expand Down
9 changes: 4 additions & 5 deletions cpp/benchmarks/synchronization/synchronization.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019-2022, NVIDIA CORPORATION.
* Copyright (c) 2019-2024, NVIDIA CORPORATION.
*
* 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 @@ -27,9 +27,10 @@
* It is built on top of the idea of Resource acquisition is initialization
* (RAII). In the following we show a minimal example of how to use this class.
#include <benchmark/benchmark.h>
#include <cudf/utilities/default_stream.hpp>
#include <benchmark/benchmark.h>
static void sample_cuda_benchmark(benchmark::State& state) {
for (auto _ : state){
Expand Down Expand Up @@ -60,14 +61,12 @@

#pragma once

// Google Benchmark library
#include <benchmark/benchmark.h>

#include <cudf/types.hpp>
#include <cudf/utilities/default_stream.hpp>

#include <rmm/cuda_stream_view.hpp>

#include <benchmark/benchmark.h>
#include <driver_types.h>

class cuda_event_timer {
Expand Down
4 changes: 2 additions & 2 deletions cpp/benchmarks/text/edit_distance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@

#include <nvtext/edit_distance.hpp>

#include <nvbench/nvbench.cuh>

#include <rmm/device_buffer.hpp>

#include <nvbench/nvbench.cuh>

static void bench_edit_distance(nvbench::state& state)
{
auto const num_rows = static_cast<cudf::size_type>(state.get_int64("num_rows"));
Expand Down
4 changes: 2 additions & 2 deletions cpp/benchmarks/text/hash_ngrams.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@

#include <nvtext/generate_ngrams.hpp>

#include <nvbench/nvbench.cuh>

#include <rmm/device_buffer.hpp>

#include <nvbench/nvbench.cuh>

static void bench_hash_ngrams(nvbench::state& state)
{
auto const num_rows = static_cast<cudf::size_type>(state.get_int64("num_rows"));
Expand Down
4 changes: 2 additions & 2 deletions cpp/benchmarks/text/jaccard.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@

#include <nvtext/jaccard.hpp>

#include <nvbench/nvbench.cuh>

#include <rmm/device_buffer.hpp>

#include <nvbench/nvbench.cuh>

static void bench_jaccard(nvbench::state& state)
{
auto const num_rows = static_cast<cudf::size_type>(state.get_int64("num_rows"));
Expand Down
4 changes: 2 additions & 2 deletions cpp/benchmarks/text/minhash.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@

#include <nvtext/minhash.hpp>

#include <nvbench/nvbench.cuh>

#include <rmm/device_buffer.hpp>

#include <nvbench/nvbench.cuh>

static void bench_minhash(nvbench::state& state)
{
auto const num_rows = static_cast<cudf::size_type>(state.get_int64("num_rows"));
Expand Down
4 changes: 2 additions & 2 deletions cpp/benchmarks/text/vocab.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@
#include <cudf_test/column_wrapper.hpp>

#include <cudf/reduction.hpp>
#include <nvtext/tokenize.hpp>

#include <cudf/scalar/scalar.hpp>
#include <cudf/strings/char_types/char_types.hpp>
#include <cudf/strings/strings_column_view.hpp>
#include <cudf/utilities/default_stream.hpp>

#include <nvtext/tokenize.hpp>

#include <nvbench/nvbench.cuh>

static void bench_vocab_tokenize(nvbench::state& state)
Expand Down
3 changes: 1 addition & 2 deletions cpp/examples/strings/custom_optimized.cu
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,9 @@
#include <rmm/device_uvector.hpp>
#include <rmm/exec_policy.hpp>

#include <thrust/scan.h>

#include <cuda_runtime.h>
#include <nvtx3/nvToolsExt.h>
#include <thrust/scan.h>

/**
* @brief Computes the size of each output row
Expand Down
5 changes: 2 additions & 3 deletions cpp/include/cudf/ast/detail/operators.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020-2023, NVIDIA CORPORATION.
* Copyright (c) 2020-2024, NVIDIA CORPORATION.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -20,9 +20,8 @@
#include <cudf/utilities/error.hpp>
#include <cudf/utilities/type_dispatcher.hpp>

#include <thrust/optional.h>

#include <cuda/std/type_traits>
#include <thrust/optional.h>

#include <cmath>
#include <type_traits>
Expand Down
Loading

0 comments on commit d053323

Please sign in to comment.