[PATCH] D150285: Fix CRTP partial specialization instantiation crash.
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9dd2af0b4cae: Fix CRTP partial specialization instantiation
crash. (authored by erichkeane).
Herald added a project: clang.
Changed prior to commit:
https://reviews.llvm.org/D150285?vs=521049=521283#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D150285/new/
https://reviews.llvm.org/D150285
Files:
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/test/SemaTemplate/partial-spec-instantiate.cpp
Index: clang/test/SemaTemplate/partial-spec-instantiate.cpp
===
--- clang/test/SemaTemplate/partial-spec-instantiate.cpp
+++ clang/test/SemaTemplate/partial-spec-instantiate.cpp
@@ -134,3 +134,23 @@
_Static_assert(S::value, "");
}
+
+namespace GH60778 {
+ template class ClassTemplate {
+ public:
+ template class Nested {};
+ };
+
+ template class Base {};
+
+ template <>
+ template
+ class ClassTemplate<>::Nested : public Base::Nested >
{};
+
+ void use() {
+// This should instantiate the body of Nested with the template arguments
+// from the Partial Specialization. This would previously get confused and
+// get the template arguments from the primary template instead.
+ClassTemplate<>::Nested instantiation;
+ }
+}
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -163,6 +163,14 @@
assert(ClassTemplSpec->getSpecializedTemplate() && "No class template?");
if (ClassTemplSpec->getSpecializedTemplate()->isMemberSpecialization())
return Response::Done();
+
+// If this was instantiated from a partial template specialization, we need
+// to get the next level of declaration context from the partial
+// specialization, as the ClassTemplateSpecializationDecl's
+// DeclContext/LexicalDeclContext will be for the primary template.
+if (auto *InstFromPartialTempl =
ClassTemplSpec->getSpecializedTemplateOrPartial()
+ .dyn_cast())
+ return
Response::ChangeDecl(InstFromPartialTempl->getLexicalDeclContext());
}
return Response::UseNextDecl(ClassTemplSpec);
}
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -391,6 +391,10 @@
at the point where it is required. This fixes:
(`#62224 `_) and
(`#62596 `_)
+- Fix an assertion when instantiating the body of a Class Template
Specialization
+ when it had been instantiated from a partial template specialization with
different
+ template arguments on the containing class. This fixes:
+ (`#60778 `_).
Bug Fixes to Compiler Builtins
^^
Index: clang/test/SemaTemplate/partial-spec-instantiate.cpp
===
--- clang/test/SemaTemplate/partial-spec-instantiate.cpp
+++ clang/test/SemaTemplate/partial-spec-instantiate.cpp
@@ -134,3 +134,23 @@
_Static_assert(S::value, "");
}
+
+namespace GH60778 {
+ template class ClassTemplate {
+ public:
+ template class Nested {};
+ };
+
+ template class Base {};
+
+ template <>
+ template
+ class ClassTemplate<>::Nested : public Base::Nested > {};
+
+ void use() {
+// This should instantiate the body of Nested with the template arguments
+// from the Partial Specialization. This would previously get confused and
+// get the template arguments from the primary template instead.
+ClassTemplate<>::Nested instantiation;
+ }
+}
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -163,6 +163,14 @@
assert(ClassTemplSpec->getSpecializedTemplate() && "No class template?");
if (ClassTemplSpec->getSpecializedTemplate()->isMemberSpecialization())
return Response::Done();
+
+// If this was instantiated from a partial template specialization, we need
+// to get the next level of declaration context from the partial
+// specialization, as the ClassTemplateSpecializationDecl's
+// DeclContext/LexicalDeclContext will be for the primary template.
+if (auto *InstFromPartialTempl = ClassTemplSpec->getSpecializedTemplateOrPartial()
+ .dyn_cast())
+ return Response::ChangeDecl(InstFromPartialTempl->getLexicalDeclContext());
}
return Response::UseNextDecl(ClassTemplSpec);
}
[PATCH] D150285: Fix CRTP partial specialization instantiation crash.
alexander-shaposhnikov accepted this revision. alexander-shaposhnikov added a comment. This revision is now accepted and ready to land. LG CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150285/new/ https://reviews.llvm.org/D150285 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D150285: Fix CRTP partial specialization instantiation crash.
erichkeane created this revision.
erichkeane added reviewers: alexander-shaposhnikov, clang-language-wg.
Herald added a project: All.
erichkeane requested review of this revision.
Fixes #60778.
When instantiating the body of a class template specialization that was
instantiated from a partial specialization, we were incorrectly
collecting template arguments from the primary template, which resulted
in the template arguments list being inaccurate. In the example from
the issue, we were trying to substitute the boolean 'false' into the
type on Nested, which caused an assertion.
https://reviews.llvm.org/D150285
Files:
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/test/SemaTemplate/partial-spec-instantiate.cpp
Index: clang/test/SemaTemplate/partial-spec-instantiate.cpp
===
--- clang/test/SemaTemplate/partial-spec-instantiate.cpp
+++ clang/test/SemaTemplate/partial-spec-instantiate.cpp
@@ -134,3 +134,23 @@
_Static_assert(S::value, "");
}
+
+namespace GH60778 {
+ template class ClassTemplate {
+ public:
+ template class Nested {};
+ };
+
+ template class Base {};
+
+ template <>
+ template
+ class ClassTemplate<>::Nested : public Base::Nested>
{};
+
+ void use() {
+// This should instantiate the body of Nested with the template arguments
+// from the Partial Specialization. This would previously get confused and
+// get the template arguments from the primary template instead.
+ClassTemplate<>::Nested instantiation;
+ }
+}
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -163,6 +163,14 @@
assert(ClassTemplSpec->getSpecializedTemplate() && "No class template?");
if (ClassTemplSpec->getSpecializedTemplate()->isMemberSpecialization())
return Response::Done();
+
+// If this was instantiated from a partial template specialization, we need
+// to get the next level of declaration context from the partial
+// specialization, as the ClassTemplateSpecializationDecl's
+// DeclContext/LexicalDeclContext will be for the primary template.
+if (auto *InstFromPartialTempl =
ClassTemplSpec->getSpecializedTemplateOrPartial()
+ .dyn_cast())
+ return
Response::ChangeDecl(InstFromPartialTempl->getLexicalDeclContext());
}
return Response::UseNextDecl(ClassTemplSpec);
}
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -391,6 +391,10 @@
at the point where it is required. This fixes:
(`#62224 `_) and
(`#62596 `_)
+- Fix an assertion when instantiating the body of a Class Template
Specialization
+ when it had been instantiated from a partial template specialization with
different
+ template arguments on the containing class. This fixes:
+ (`#60778 `_).
Bug Fixes to Compiler Builtins
^^
Index: clang/test/SemaTemplate/partial-spec-instantiate.cpp
===
--- clang/test/SemaTemplate/partial-spec-instantiate.cpp
+++ clang/test/SemaTemplate/partial-spec-instantiate.cpp
@@ -134,3 +134,23 @@
_Static_assert(S::value, "");
}
+
+namespace GH60778 {
+ template class ClassTemplate {
+ public:
+ template class Nested {};
+ };
+
+ template class Base {};
+
+ template <>
+ template
+ class ClassTemplate<>::Nested : public Base::Nested> {};
+
+ void use() {
+// This should instantiate the body of Nested with the template arguments
+// from the Partial Specialization. This would previously get confused and
+// get the template arguments from the primary template instead.
+ClassTemplate<>::Nested instantiation;
+ }
+}
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -163,6 +163,14 @@
assert(ClassTemplSpec->getSpecializedTemplate() && "No class template?");
if (ClassTemplSpec->getSpecializedTemplate()->isMemberSpecialization())
return Response::Done();
+
+// If this was instantiated from a partial template specialization, we need
+// to get the next level of declaration context from the partial
+// specialization, as the ClassTemplateSpecializationDecl's
+// DeclContext/LexicalDeclContext will be for the primary template.
+if (auto *InstFromPartialTempl = ClassTemplSpec->getSpecializedTemplateOrPartial()
+ .dyn_cast())
+ return
