[Clang] Profile the NNS of UnresolvedUsingType and CXXThisType correctly in concept hashing#199617
Conversation
llvmorg-github-actions
Bot
added
clang
|
@llvm/pr-subscribers-clang Author: Younan Zhang (zyn0217) ChangesThey were sometimes incorrect because the written type doesn't contain an NNS which contains template parameters we're interested in. No release note because the bug broke MS STL and I want to backport it to the last 22.x release Fixes #198663 2 Files Affected:
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index ea03c3f408986..66d1f4db29410 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -399,6 +399,16 @@ class HashParameterMapping : public RecursiveASTVisitor<HashParameterMapping> {
return true;
}
+ bool TraverseUnresolvedUsingType(UnresolvedUsingType *T,
+ bool TraverseQualifier) {
+ // Sometimes the written type doesn't contain a qualifier which contains
+ // necessary template arguments, whereas the declaration does.
+ if (NestedNameSpecifier NNS = T->getDecl()->getQualifier();
+ TraverseQualifier && NNS)
+ return inherited::TraverseNestedNameSpecifier(NNS);
+ return true;
+ }
+
bool TraverseInjectedClassNameType(InjectedClassNameType *T,
bool TraverseQualifier) {
return TraverseTemplateArguments(T->getTemplateArgs(SemaRef.Context));
diff --git a/clang/test/SemaTemplate/concepts-using-decl.cpp b/clang/test/SemaTemplate/concepts-using-decl.cpp
index 26bd0b60b691c..dfdab707e4814 100644
--- a/clang/test/SemaTemplate/concepts-using-decl.cpp
+++ b/clang/test/SemaTemplate/concepts-using-decl.cpp
@@ -197,3 +197,72 @@ struct child : base<int> {
};
}
+
+namespace GH198663 {
+
+template <class T>
+concept HasIsTransparent = requires { typename T::is_transparent; };
+
+template <class K, class V, class Compare>
+struct FlatMapBase {
+ using key_compare = Compare;
+};
+
+template <class K, class V, class Compare>
+struct FlatMap : FlatMapBase<K, V, Compare> {
+ using Base = FlatMapBase<K, V, Compare>;
+
+ using typename Base::key_compare;
+
+ void at(const K&) {}
+ void at(const K&) const {}
+ template <class Other>
+ void at(const Other&)
+ requires HasIsTransparent<key_compare>
+ {}
+ template <class Other>
+ void at(const Other&) const
+ requires HasIsTransparent<key_compare>
+ {}
+};
+
+template <class T>
+struct Transparent {
+ T t;
+};
+
+struct TransparentComparator {
+ using is_transparent = void;
+
+ template <class T>
+ bool operator()(const T&, const Transparent<T>&) const;
+
+ template <class T>
+ bool operator()(const Transparent<T>&, const T& t) const;
+
+ template <class T>
+ bool operator()(const T&, const T&) const;
+};
+
+struct NonTransparentComparator {
+ template <class T>
+ bool operator()(const T&, const Transparent<T>&) const;
+
+ template <class T>
+ bool operator()(const Transparent<T>&, const T&) const;
+
+ template <class T>
+ bool operator()(const T&, const T&) const;
+};
+
+template <class M>
+concept CanAt = requires(M m, Transparent<int> k) { m.at(k); };
+
+using TransparentMap = FlatMap<int, double, TransparentComparator>;
+using NonTransparentMap = FlatMap<int, double, NonTransparentComparator>;
+
+static_assert(CanAt<TransparentMap>);
+
+static_assert(!CanAt<NonTransparentMap>);
+
+}
|
|
@StephanTLavavej in case you're able to help us test this patch over STL tests, thanks! |
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
|
The test failure seems unrelated |
|
Thank you - unfortunately I'm not set up to build Clang locally and don't have the time to learn how. |
|
CC @gracicot in case you want to verify it fixes it for you as well. |
|
Hmm. This does not seem to fix the problem for me. I built clang from the 198663 branch and I still observe the issue on my local machine. I pushed this commit in my repo that reproduce the issue: gracicot/kangaru@04eb61d - depends on fmt and catch2 for building. I was not able to create a minimal repro sadly. |
|
@gracicot Can you please provide us with a self contained reproducer? It would be nice if we don't have to spend time setting up a build environment, thanks! |
|
@zyn0217 I took some time and was able to reduce it: template<typename T>
auto mv(T& t) -> T&&;
template<typename S, typename T>
concept does_foo = requires(S s) {
s.template foo<T>();
};
template<typename S>
struct type {
S member;
template<typename T>
auto foo() -> T requires does_foo<decltype(mv(member)), T>;
};
struct returns_int {
template<typename T>
auto foo() -> T;
};
struct nothing {};
#ifdef REPRO
static_assert(does_foo<type<returns_int>&, int>);
#endif
static_assert(not does_foo<type<nothing>&, int>);With The key thing I found is that it's specific to a concept checking for a template function, and that function has to return decltype of an expression that involves a function and a non static data member. |
…ashing They were sometimes incorrect because the written type doesn't contain an NNS which contains template parameters we're interested in.
zyn0217
changed the title
|
@gracicot The latest commit should fix that issue, thanks for the feedback! |
github-project-automation
Bot
moved this from Needs Triage
to Done
in LLVM Release Status
|
/cherry-pick 06ffb65 |
|
/pull-request #200988 |
…tly in concept hashing (llvm#199617) They were sometimes incorrect because the written type doesn't contain an NNS which contains template parameters we're interested in. No release note because the bug broke MS STL and I want to backport it to the last 22.x release Fixes llvm#198663 (cherry picked from commit 06ffb65)
6 participants
They were sometimes incorrect because the written type doesn't contain an NNS which contains template parameters we're interested in.
No release note because the bug broke MS STL and I want to backport it to the last 22.x release
Fixes #198663