-
Notifications
You must be signed in to change notification settings - Fork 14.4k
[clang-tidy] Teach modernize-type-traits
about more type traits
#147074
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
base: main
Are you sure you want to change the base?
Conversation
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
@llvm/pr-subscribers-clang-tools-extra Author: Victor Chernyakin (localspook) ChangesThese new traits come from various standard versions: C++14:
C++17:
C++20:
C++23:
C++26:
This doesn't add Full diff: https://github.com/llvm/llvm-project/pull/147074.diff 2 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.cpp b/clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.cpp
index c0766395ec5cc..92e6f61a6d9d4 100644
--- a/clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.cpp
@@ -28,8 +28,10 @@ static const llvm::StringSet<> ValueTraits = {
"is_array",
"is_assignable",
"is_base_of",
+ "is_bind_expression",
"is_bounded_array",
"is_class",
+ "is_clock",
"is_compound",
"is_const",
"is_constructible",
@@ -40,10 +42,14 @@ static const llvm::StringSet<> ValueTraits = {
"is_destructible",
"is_empty",
"is_enum",
+ "is_error_code_enum",
+ "is_error_condition_enum",
+ "is_execution_policy",
"is_final",
"is_floating_point",
"is_function",
"is_fundamental",
+ "is_implicit_lifetime",
"is_integral",
"is_invocable",
"is_invocable_r",
@@ -65,14 +71,17 @@ static const llvm::StringSet<> ValueTraits = {
"is_nothrow_invocable_r",
"is_nothrow_move_assignable",
"is_nothrow_move_constructible",
+ "is_nothrow_relocatable",
"is_nothrow_swappable",
"is_nothrow_swappable_with",
"is_null_pointer",
"is_object",
+ "is_placeholder",
"is_pointer",
"is_pointer_interconvertible_base_of",
"is_polymorphic",
"is_reference",
+ "is_replaceable",
"is_rvalue_reference",
"is_same",
"is_scalar",
@@ -91,15 +100,27 @@ static const llvm::StringSet<> ValueTraits = {
"is_trivially_destructible",
"is_trivially_move_assignable",
"is_trivially_move_constructible",
+ "is_trivially_relocatable",
"is_unbounded_array",
"is_union",
"is_unsigned",
+ "is_virtual_base_of",
"is_void",
"is_volatile",
"negation",
"rank",
+ "ratio_equal",
+ "ratio_greater_equal",
+ "ratio_greater",
+ "ratio_less_equal",
+ "ratio_less",
+ "ratio_not_equal",
"reference_constructs_from_temporary",
"reference_converts_from_temporary",
+ "treat_as_floating_point",
+ "tuple_size",
+ "uses_allocator",
+ "variant_size",
};
static const llvm::StringSet<> TypeTraits = {
@@ -130,6 +151,12 @@ static const llvm::StringSet<> TypeTraits = {
"result_of",
"invoke_result",
"type_identity",
+ "tuple_element",
+ "variant_alternative",
+ "compare_three_way_result",
+ "common_comparison_category",
+ "unwrap_ref_decay",
+ "unwrap_reference",
};
static DeclarationName getName(const DependentScopeDeclRefExpr &D) {
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index f8f183e9de1cc..3d554733ad3fd 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -276,6 +276,9 @@ Changes in existing checks
excluding variables with ``thread_local`` storage class specifier from being
matched.
+- Improved :doc:`modernize-type-traits
+ <clang-tidy/checks/modernize/type-traits>` check by detecting more type traits.
+
- Improved :doc:`modernize-use-default-member-init
<clang-tidy/checks/modernize/use-default-member-init>` check by matching
arithmetic operations, ``constexpr`` and ``static`` values, and detecting
|
@llvm/pr-subscribers-clang-tidy Author: Victor Chernyakin (localspook) ChangesThese new traits come from various standard versions: C++14:
C++17:
C++20:
C++23:
C++26:
This doesn't add Full diff: https://github.com/llvm/llvm-project/pull/147074.diff 2 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.cpp b/clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.cpp
index c0766395ec5cc..92e6f61a6d9d4 100644
--- a/clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.cpp
@@ -28,8 +28,10 @@ static const llvm::StringSet<> ValueTraits = {
"is_array",
"is_assignable",
"is_base_of",
+ "is_bind_expression",
"is_bounded_array",
"is_class",
+ "is_clock",
"is_compound",
"is_const",
"is_constructible",
@@ -40,10 +42,14 @@ static const llvm::StringSet<> ValueTraits = {
"is_destructible",
"is_empty",
"is_enum",
+ "is_error_code_enum",
+ "is_error_condition_enum",
+ "is_execution_policy",
"is_final",
"is_floating_point",
"is_function",
"is_fundamental",
+ "is_implicit_lifetime",
"is_integral",
"is_invocable",
"is_invocable_r",
@@ -65,14 +71,17 @@ static const llvm::StringSet<> ValueTraits = {
"is_nothrow_invocable_r",
"is_nothrow_move_assignable",
"is_nothrow_move_constructible",
+ "is_nothrow_relocatable",
"is_nothrow_swappable",
"is_nothrow_swappable_with",
"is_null_pointer",
"is_object",
+ "is_placeholder",
"is_pointer",
"is_pointer_interconvertible_base_of",
"is_polymorphic",
"is_reference",
+ "is_replaceable",
"is_rvalue_reference",
"is_same",
"is_scalar",
@@ -91,15 +100,27 @@ static const llvm::StringSet<> ValueTraits = {
"is_trivially_destructible",
"is_trivially_move_assignable",
"is_trivially_move_constructible",
+ "is_trivially_relocatable",
"is_unbounded_array",
"is_union",
"is_unsigned",
+ "is_virtual_base_of",
"is_void",
"is_volatile",
"negation",
"rank",
+ "ratio_equal",
+ "ratio_greater_equal",
+ "ratio_greater",
+ "ratio_less_equal",
+ "ratio_less",
+ "ratio_not_equal",
"reference_constructs_from_temporary",
"reference_converts_from_temporary",
+ "treat_as_floating_point",
+ "tuple_size",
+ "uses_allocator",
+ "variant_size",
};
static const llvm::StringSet<> TypeTraits = {
@@ -130,6 +151,12 @@ static const llvm::StringSet<> TypeTraits = {
"result_of",
"invoke_result",
"type_identity",
+ "tuple_element",
+ "variant_alternative",
+ "compare_three_way_result",
+ "common_comparison_category",
+ "unwrap_ref_decay",
+ "unwrap_reference",
};
static DeclarationName getName(const DependentScopeDeclRefExpr &D) {
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index f8f183e9de1cc..3d554733ad3fd 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -276,6 +276,9 @@ Changes in existing checks
excluding variables with ``thread_local`` storage class specifier from being
matched.
+- Improved :doc:`modernize-type-traits
+ <clang-tidy/checks/modernize/type-traits>` check by detecting more type traits.
+
- Improved :doc:`modernize-use-default-member-init
<clang-tidy/checks/modernize/use-default-member-init>` check by matching
arithmetic operations, ``constexpr`` and ``static`` values, and detecting
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds support for more C++ type traits in the modernize-type-traits
clang-tidy check and updates the release notes accordingly.
- Expanded
ValueTraits
andTypeTraits
string sets inTypeTraitsCheck.cpp
with new traits from C++14 through C++26. - Updated
ReleaseNotes.rst
to document enhanced detection in themodernize-type-traits
check.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
clang-tools-extra/docs/ReleaseNotes.rst | Added a bullet to announce improved trait detection in the modernize-type-traits check |
clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.cpp | Inserted numerous new trait names into ValueTraits and TypeTraits sets |
Comments suppressed due to low confidence (2)
clang-tools-extra/docs/ReleaseNotes.rst:279
- [nitpick] The bullet link is split across two lines which may break RST formatting; consider combining the link and description on one line or properly indenting the continuation to match list formatting.
- Improved :doc:`modernize-type-traits
clang-tools-extra/clang-tidy/modernize/TypeTraitsCheck.cpp:152
- This line uses a tab for indentation; please replace it with spaces to match the surrounding entries and project style.
"tuple_element",
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the patch!
Could you please add tests with new traits from C++20, 23, 26 standards to make sure we get warning for newer standards than C++17. You will need to change -std=c++17
to -std=c++17-or-later
in tests.
I wonder did you find this somehow automatically or manually? |
Mostly manually: I took a list of all standard library symbols, used a hacky regex to find |
I've changed the tests to use |
clang-tools-extra/docs/clang-tidy/checks/modernize/type-traits.rst
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, we'd wait a couple of days before merge in case someone else wants to review it too.
When we're ready to merge, someone please do it for me, I don't have write permissions. |
These new traits come from various standard versions:
C++14:
tuple_element_t
C++17:
is_placeholder_v
is_bind_expression_v
ratio_equal_v
ratio_not_equal_v
ratio_less_v
ratio_less_equal_v
ratio_greater_v
ratio_greater_equal_v
is_error_code_enum_v
is_error_condition_enum_v
is_execution_policy_v
tuple_size_v
variant_size_v
uses_allocator_v
variant_alternative_t
C++20:
compare_three_way_result_t
common_comparison_category_t
unwrap_ref_decay_t
unwrap_reference_t
C++23:
is_implicit_lifetime_v
C++26:
is_nothrow_relocatable_v
is_replaceable_v
is_trivially_relocatable_v
is_virtual_base_of_v
This doesn't add
treat_as_floating_point_v
oris_clock_v
because they require more invasive changes; instead I've opened #147072 to track them.