Turn on CI for clang-tidy
#10064
Closed
Turn on CI for clang-tidy
#10064
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
codereport
added
2 - In Progress
Codecov Report
@@ Coverage Diff @@
## branch-22.04 #10064 +/- ##
================================================
- Coverage 86.13% 85.97% -0.17%
================================================
Files 139 139
Lines 22438 22435 -3
================================================
- Hits 19328 19288 -40
- Misses 3110 3147 +37
Continue to review full report at Codecov.
|
rapids-bot bot
pushed a commit
that referenced
this pull request
Jan 20, 2022
This PR is adding clang-tidy to cudf and adding the initial checks. Note more checks will be enabled in the future. Relevant PRs: * `rmm`: rapidsai/rmm#857 * `cuml`: rapidsai/cuml#1945 To do list: * [x] Add `.clang-tidy` file * [x] Add python script * [x] Apply `modernize-` changes * [x] Revert `cxxopts` changes * [x] Fixed Python parquet failures * [x] Ignore `cxxopts` file * [x] Ignore the `build/_deps` directories Splitting out the following into a separate PR so we can get the changes merged for 22.02 (#10064): * ~~[ ] Disable `clang-diagnostic-errors/warnings`~~ * ~~[ ] Fix include files being skipped~~ * ~~[ ] Set up CI script~~ * ~~[ ] Clean up python script~~ Authors: - Conor Hoekstra (https://github.com/codereport) Approvers: - Bradley Dice (https://github.com/bdice) - Nghia Truong (https://github.com/ttnghia) - David Wendt (https://github.com/davidwendt) - Mark Harris (https://github.com/harrism) - Vyas Ramasubramani (https://github.com/vyasr) URL: #9860
bdice
approved these changes
Feb 9, 2022
cpp/scripts/run-clang-tidy.py
Outdated
| @@ -86,7 +85,7 @@ def get_gpu_archs(command): | |||
| def get_index(arr, item): | |||
| try: | |||
| return arr.index(item) | |||
| except: | |||
| except Exception: | |||
There was a problem hiding this comment.
arr = [1, 2, 3]
arr.index(4) # Raises ValueErrorLet's catch a more specific error:
Suggested change
| except Exception: | |
| except ValueError: |
cpp/scripts/run-clang-tidy.py
Outdated
| else: | ||
| out = "CMD: " + cmd | ||
| out += result.stdout.decode("utf-8").rstrip() | ||
| out = ("" if status else "CMD: " + cmd) + result.stdout.decode("utf-8").rstrip() |
There was a problem hiding this comment.
Decode/encode bytes str uses UTF-8 by default in Python 3.
Suggested change
| out = ("" if status else "CMD: " + cmd) + result.stdout.decode("utf-8").rstrip() | |
| out = ("" if status else "CMD: " + cmd) + result.stdout.decode().rstrip() |
|
rerun tests |
1 similar comment
|
rerun tests |
|
rerun tests |
|
This PR has been labeled |
rapids-bot bot
pushed a commit
that referenced
this pull request
Jun 12, 2024
This changeset is large, but it's not very substantial. It's all the automated fixes produced by clang-tidy using our script. The bulk of the changes are either adding `[[nodiscard]]` to many functions or changing const ref args to pass by value and then move in cases where the parameter is only used to set a value. There are also some places where clang-tidy preferred either more or less namespacing of objects depending on the current namespace. The goal is to enable clang-tidy in CI, which we made progress towards in #9860 but stalled in #10064. This PR contains the first set of changes that will required for such a check to pass. I've marked this PR as breaking because some of the functions now marked as `[[nodiscard]]` are public APIs, so if consumers were ignoring the return values they will now see warnings, and if they are compiling with warnings as errors then the builds will break. Contributes to #584 Authors: - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - Bradley Dice (https://github.com/bdice) - Nghia Truong (https://github.com/ttnghia) URL: #15894
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a follow up to #9860
To do list:
clang-diagnostic-errors/warnings