[RecursiveASTVisitor] Skip implicit instantiations. #110899
Open
+30
−7
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
In DEF_TRAVERSE_TMPL_SPEC_DECL, we attempted to skip implicit instantiations by detecting that D->getTemplateArgsAsWritten() returns nullptr, but as this test shows, it is possible for that to return a non-null pointer even for implicit instantiations. Explicitly check for this case instead. Fixes llvm#110502
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-tools-extra Author: Harald van Dijk (hvdijk) ChangesIn DEF_TRAVERSE_TMPL_SPEC_DECL, we attempted to skip implicit instantiations by detecting that D->getTemplateArgsAsWritten() returns nullptr, but as this test shows, it is possible for that to return a non-null pointer even for implicit instantiations. Explicitly check for this case instead. Fixes #110502 cc @sdkrystian, this fixes an issue introduced by 1202837, does this look okay or do you feel there should be another way of doing it? Full diff: https://github.com/llvm/llvm-project/pull/110899.diff 2 Files Affected:
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/type-traits.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/type-traits.cpp
index 72241846384bcf..854ffa7928635c 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/type-traits.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/type-traits.cpp
@@ -14,11 +14,22 @@ namespace std {
static constexpr bool value = true;
};
+ template <typename T, typename U>
+ static constexpr bool is_same_v = is_same<T, U>::value; // NOLINT
+
template<bool, typename T = void>
struct enable_if {
using type = T;
};
+ template <bool B, typename T = void>
+ using enable_if_t = typename enable_if<B, T>::type; // NOLINT
+
+ template <typename T>
+ struct remove_reference {
+ using type = T;
+ };
+
inline namespace __std_lib_version1 {
template<typename T>
struct add_const {
@@ -117,3 +128,13 @@ namespace my_std = std;
using Alias = my_std::add_const<bool>::type;
// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: use c++14 style type templates
// CHECK-FIXES: using Alias = my_std::add_const_t<bool>;
+
+template <typename T>
+struct ImplicitlyInstantiatedConstructor {
+ template <typename U, typename = std::enable_if_t<std::is_same_v<U, T>>>
+ ImplicitlyInstantiatedConstructor(U) {}
+};
+
+const ImplicitlyInstantiatedConstructor<int> ImplicitInstantiation(std::remove_reference<int>::type(123));
+// CHECK-MESSAGES: :[[@LINE-1]]:68: warning: use c++14 style type templates
+// CHECK-FIXES: const ImplicitlyInstantiatedConstructor<int> ImplicitInstantiation(std::remove_reference_t<int>(123));
diff --git a/clang/include/clang/AST/RecursiveASTVisitor.h b/clang/include/clang/AST/RecursiveASTVisitor.h
index cd9947f7ab9805..b563b89875f08b 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -2069,22 +2069,24 @@ bool RecursiveASTVisitor<Derived>::TraverseTemplateArgumentLocsHelper(
#define DEF_TRAVERSE_TMPL_SPEC_DECL(TMPLDECLKIND, DECLKIND) \
DEF_TRAVERSE_DECL(TMPLDECLKIND##TemplateSpecializationDecl, { \
+ auto TSK = D->getTemplateSpecializationKind(); \
/* For implicit instantiations ("set<int> x;"), we don't want to \
recurse at all, since the instatiated template isn't written in \
the source code anywhere. (Note the instatiated *type* -- \
set<int> -- is written, and will still get a callback of \
TemplateSpecializationType). For explicit instantiations \
("template set<int>;"), we do need a callback, since this \
- is the only callback that's made for this instantiation. \
- We use getTemplateArgsAsWritten() to distinguish. */ \
- if (const auto *ArgsWritten = D->getTemplateArgsAsWritten()) { \
- /* The args that remains unspecialized. */ \
- TRY_TO(TraverseTemplateArgumentLocsHelper( \
- ArgsWritten->getTemplateArgs(), ArgsWritten->NumTemplateArgs)); \
+ is the only callback that's made for this instantiation. */ \
+ if (TSK != TSK_ImplicitInstantiation) { \
+ if (const auto *ArgsWritten = D->getTemplateArgsAsWritten()) { \
+ /* The args that remains unspecialized. */ \
+ TRY_TO(TraverseTemplateArgumentLocsHelper( \
+ ArgsWritten->getTemplateArgs(), ArgsWritten->NumTemplateArgs)); \
+ } \
} \
\
if (getDerived().shouldVisitTemplateInstantiations() || \
- D->getTemplateSpecializationKind() == TSK_ExplicitSpecialization) { \
+ TSK == TSK_ExplicitSpecialization) { \
/* Traverse base definition for explicit specializations */ \
TRY_TO(Traverse##DECLKIND##Helper(D)); \
} else { \
|
|
@llvm/pr-subscribers-clang-tidy Author: Harald van Dijk (hvdijk) ChangesIn DEF_TRAVERSE_TMPL_SPEC_DECL, we attempted to skip implicit instantiations by detecting that D->getTemplateArgsAsWritten() returns nullptr, but as this test shows, it is possible for that to return a non-null pointer even for implicit instantiations. Explicitly check for this case instead. Fixes #110502 cc @sdkrystian, this fixes an issue introduced by 1202837, does this look okay or do you feel there should be another way of doing it? Full diff: https://github.com/llvm/llvm-project/pull/110899.diff 2 Files Affected:
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/type-traits.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/type-traits.cpp
index 72241846384bcf..854ffa7928635c 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/type-traits.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/type-traits.cpp
@@ -14,11 +14,22 @@ namespace std {
static constexpr bool value = true;
};
+ template <typename T, typename U>
+ static constexpr bool is_same_v = is_same<T, U>::value; // NOLINT
+
template<bool, typename T = void>
struct enable_if {
using type = T;
};
+ template <bool B, typename T = void>
+ using enable_if_t = typename enable_if<B, T>::type; // NOLINT
+
+ template <typename T>
+ struct remove_reference {
+ using type = T;
+ };
+
inline namespace __std_lib_version1 {
template<typename T>
struct add_const {
@@ -117,3 +128,13 @@ namespace my_std = std;
using Alias = my_std::add_const<bool>::type;
// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: use c++14 style type templates
// CHECK-FIXES: using Alias = my_std::add_const_t<bool>;
+
+template <typename T>
+struct ImplicitlyInstantiatedConstructor {
+ template <typename U, typename = std::enable_if_t<std::is_same_v<U, T>>>
+ ImplicitlyInstantiatedConstructor(U) {}
+};
+
+const ImplicitlyInstantiatedConstructor<int> ImplicitInstantiation(std::remove_reference<int>::type(123));
+// CHECK-MESSAGES: :[[@LINE-1]]:68: warning: use c++14 style type templates
+// CHECK-FIXES: const ImplicitlyInstantiatedConstructor<int> ImplicitInstantiation(std::remove_reference_t<int>(123));
diff --git a/clang/include/clang/AST/RecursiveASTVisitor.h b/clang/include/clang/AST/RecursiveASTVisitor.h
index cd9947f7ab9805..b563b89875f08b 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -2069,22 +2069,24 @@ bool RecursiveASTVisitor<Derived>::TraverseTemplateArgumentLocsHelper(
#define DEF_TRAVERSE_TMPL_SPEC_DECL(TMPLDECLKIND, DECLKIND) \
DEF_TRAVERSE_DECL(TMPLDECLKIND##TemplateSpecializationDecl, { \
+ auto TSK = D->getTemplateSpecializationKind(); \
/* For implicit instantiations ("set<int> x;"), we don't want to \
recurse at all, since the instatiated template isn't written in \
the source code anywhere. (Note the instatiated *type* -- \
set<int> -- is written, and will still get a callback of \
TemplateSpecializationType). For explicit instantiations \
("template set<int>;"), we do need a callback, since this \
- is the only callback that's made for this instantiation. \
- We use getTemplateArgsAsWritten() to distinguish. */ \
- if (const auto *ArgsWritten = D->getTemplateArgsAsWritten()) { \
- /* The args that remains unspecialized. */ \
- TRY_TO(TraverseTemplateArgumentLocsHelper( \
- ArgsWritten->getTemplateArgs(), ArgsWritten->NumTemplateArgs)); \
+ is the only callback that's made for this instantiation. */ \
+ if (TSK != TSK_ImplicitInstantiation) { \
+ if (const auto *ArgsWritten = D->getTemplateArgsAsWritten()) { \
+ /* The args that remains unspecialized. */ \
+ TRY_TO(TraverseTemplateArgumentLocsHelper( \
+ ArgsWritten->getTemplateArgs(), ArgsWritten->NumTemplateArgs)); \
+ } \
} \
\
if (getDerived().shouldVisitTemplateInstantiations() || \
- D->getTemplateSpecializationKind() == TSK_ExplicitSpecialization) { \
+ TSK == TSK_ExplicitSpecialization) { \
/* Traverse base definition for explicit specializations */ \
TRY_TO(Traverse##DECLKIND##Helper(D)); \
} else { \
|
Sirraide
reviewed
Oct 3, 2024
| TRY_TO(TraverseTemplateArgumentLocsHelper( \ | ||
| ArgsWritten->getTemplateArgs(), ArgsWritten->NumTemplateArgs)); \ | ||
| is the only callback that's made for this instantiation. */ \ | ||
| if (TSK != TSK_ImplicitInstantiation) { \ |
There was a problem hiding this comment.
Do we have to check for TSK_Undeclared too? We do that in e.g. TraverseFunctionHelper(), which seems like a similar situation to this.
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.
In DEF_TRAVERSE_TMPL_SPEC_DECL, we attempted to skip implicit instantiations by detecting that D->getTemplateArgsAsWritten() returns nullptr, but as this test shows, it is possible for that to return a non-null pointer even for implicit instantiations. Explicitly check for this case instead.
Fixes #110502
cc @sdkrystian, this fixes an issue introduced by 1202837, does this look okay or do you feel there should be another way of doing it?