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

Use latest cmake for clang-tidy. #10730

Merged
merged 7 commits into from
Aug 21, 2024
Merged

Conversation

trivialfis
Copy link
Member

@trivialfis trivialfis commented Aug 21, 2024

  • Install cmake using pip.
  • Fix compile command generation.
  • Clean up the tidy script and remove the need to load the yaml file.
  • Fix modernized type traits.
  • Fix span class. Polymorphism support is dropped as it was broken anyway.

The new cmake is required by the latest GPU tree shap.

@trivialfis trivialfis requested a review from hcho3 August 21, 2024 14:15
@trivialfis
Copy link
Member Author

trivialfis commented Aug 21, 2024

@hcho3 I haven't updated clang-tidy itself on the CI yet, but I have run clang-tidy-19 locally, and the test should pass. I referenced the issue you opened in llvm project. Do you want me to update the clang-tidy as well?

@@ -223,6 +222,14 @@ class JsonObject : public Value {
~JsonObject() override = default;
};

namespace detail {
template <typename T, typename U>
using IsSameT = std::enable_if_t<std::is_same_v<std::remove_cv_t<T>, std::remove_cv_t<U>>>;
Copy link
Member Author

@trivialfis trivialfis Aug 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only extracting this due to a potential bug in clang-tidy-19, can't be sure yet. It keeps asking me to use modernized type traits if I don't add these helpers, even though I have already been using the _v/t version.

@hcho3
Copy link
Collaborator

hcho3 commented Aug 21, 2024

Do you want me to update the clang-tidy as well?

Yes, that would be great.

@trivialfis
Copy link
Member Author

@hcho3 Done.

@trivialfis trivialfis merged commit cb54374 into dmlc:master Aug 21, 2024
29 of 30 checks passed
@trivialfis trivialfis deleted the clang-tidy branch August 21, 2024 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants