[Clang][Sema] Always use latest redeclaration of primary template #114258
+165
−66
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
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Krystian Stasiowski (sdkrystian) ChangesThis patch fixes a couple of regressions introduced in #111852. Consider: template<typename T>
struct A
{
template<bool U>
static constexpr bool f() requires U
{
return true;
}
};
template<>
template<bool U>
constexpr bool A<short>::f() requires U
{
return A<long>::f<U>();
}
template<>
template<bool U>
constexpr bool A<long>::f() requires U
{
return true;
}
static_assert(A<short>::f<true>()); // crash hereThis crashes because when collecting template arguments from the first declaration of This also fixes the following example, which happens for a similar reason: // input.cppm
export module input;
export template<int N>
constexpr int f();
template<int N>
struct A {
template<int J>
friend constexpr int f();
};
template struct A<0>;
template<int N>
constexpr int f() {
return N;
}// input.cpp
import input;
static_assert(f<1>() == 1); // error: static assertion failedFull diff: https://github.com/llvm/llvm-project/pull/114258.diff 9 Files Affected:
diff --git a/clang/include/clang/AST/DeclTemplate.h b/clang/include/clang/AST/DeclTemplate.h
index a572e3380f1655..0ca3fd48e81cf4 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -857,16 +857,6 @@ class RedeclarableTemplateDecl : public TemplateDecl,
/// \endcode
bool isMemberSpecialization() const { return Common.getInt(); }
- /// Determines whether any redeclaration of this template was
- /// a specialization of a member template.
- bool hasMemberSpecialization() const {
- for (const auto *D : redecls()) {
- if (D->isMemberSpecialization())
- return true;
- }
- return false;
- }
-
/// Note that this member template is a specialization.
void setMemberSpecialization() {
assert(!isMemberSpecialization() && "already a member specialization");
@@ -1965,13 +1955,7 @@ class ClassTemplateSpecializationDecl : public CXXRecordDecl,
/// specialization which was specialized by this.
llvm::PointerUnion<ClassTemplateDecl *,
ClassTemplatePartialSpecializationDecl *>
- getSpecializedTemplateOrPartial() const {
- if (const auto *PartialSpec =
- SpecializedTemplate.dyn_cast<SpecializedPartialSpecialization *>())
- return PartialSpec->PartialSpecialization;
-
- return SpecializedTemplate.get<ClassTemplateDecl*>();
- }
+ getSpecializedTemplateOrPartial() const;
/// Retrieve the set of template arguments that should be used
/// to instantiate members of the class template or class template partial
@@ -2208,17 +2192,6 @@ class ClassTemplatePartialSpecializationDecl
return InstantiatedFromMember.getInt();
}
- /// Determines whether any redeclaration of this this class template partial
- /// specialization was a specialization of a member partial specialization.
- bool hasMemberSpecialization() const {
- for (const auto *D : redecls()) {
- if (cast<ClassTemplatePartialSpecializationDecl>(D)
- ->isMemberSpecialization())
- return true;
- }
- return false;
- }
-
/// Note that this member template is a specialization.
void setMemberSpecialization() { return InstantiatedFromMember.setInt(true); }
@@ -2740,13 +2713,7 @@ class VarTemplateSpecializationDecl : public VarDecl,
/// Retrieve the variable template or variable template partial
/// specialization which was specialized by this.
llvm::PointerUnion<VarTemplateDecl *, VarTemplatePartialSpecializationDecl *>
- getSpecializedTemplateOrPartial() const {
- if (const auto *PartialSpec =
- SpecializedTemplate.dyn_cast<SpecializedPartialSpecialization *>())
- return PartialSpec->PartialSpecialization;
-
- return SpecializedTemplate.get<VarTemplateDecl *>();
- }
+ getSpecializedTemplateOrPartial() const;
/// Retrieve the set of template arguments that should be used
/// to instantiate the initializer of the variable template or variable
@@ -2980,18 +2947,6 @@ class VarTemplatePartialSpecializationDecl
return InstantiatedFromMember.getInt();
}
- /// Determines whether any redeclaration of this this variable template
- /// partial specialization was a specialization of a member partial
- /// specialization.
- bool hasMemberSpecialization() const {
- for (const auto *D : redecls()) {
- if (cast<VarTemplatePartialSpecializationDecl>(D)
- ->isMemberSpecialization())
- return true;
- }
- return false;
- }
-
/// Note that this member template is a specialization.
void setMemberSpecialization() { return InstantiatedFromMember.setInt(true); }
@@ -3164,6 +3119,9 @@ class VarTemplateDecl : public RedeclarableTemplateDecl {
return makeSpecIterator(getSpecializations(), true);
}
+ /// Merge \p Prev with our RedeclarableTemplateDecl::Common.
+ void mergePrevDecl(VarTemplateDecl *Prev);
+
// Implement isa/cast/dyncast support
static bool classof(const Decl *D) { return classofKind(D->getKind()); }
static bool classofKind(Kind K) { return K == VarTemplate; }
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 86913763ef9ff5..cd173d17263792 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -2708,7 +2708,7 @@ VarDecl *VarDecl::getTemplateInstantiationPattern() const {
if (isTemplateInstantiation(VDTemplSpec->getTemplateSpecializationKind())) {
auto From = VDTemplSpec->getInstantiatedFrom();
if (auto *VTD = From.dyn_cast<VarTemplateDecl *>()) {
- while (!VTD->hasMemberSpecialization()) {
+ while (!VTD->isMemberSpecialization()) {
if (auto *NewVTD = VTD->getInstantiatedFromMemberTemplate())
VTD = NewVTD;
else
@@ -2718,7 +2718,7 @@ VarDecl *VarDecl::getTemplateInstantiationPattern() const {
}
if (auto *VTPSD =
From.dyn_cast<VarTemplatePartialSpecializationDecl *>()) {
- while (!VTPSD->hasMemberSpecialization()) {
+ while (!VTPSD->isMemberSpecialization()) {
if (auto *NewVTPSD = VTPSD->getInstantiatedFromMember())
VTPSD = NewVTPSD;
else
@@ -2732,7 +2732,7 @@ VarDecl *VarDecl::getTemplateInstantiationPattern() const {
// If this is the pattern of a variable template, find where it was
// instantiated from. FIXME: Is this necessary?
if (VarTemplateDecl *VTD = VD->getDescribedVarTemplate()) {
- while (!VTD->hasMemberSpecialization()) {
+ while (!VTD->isMemberSpecialization()) {
if (auto *NewVTD = VTD->getInstantiatedFromMemberTemplate())
VTD = NewVTD;
else
@@ -4153,7 +4153,7 @@ FunctionDecl::getTemplateInstantiationPattern(bool ForDefinition) const {
if (FunctionTemplateDecl *Primary = getPrimaryTemplate()) {
// If we hit a point where the user provided a specialization of this
// template, we're done looking.
- while (!ForDefinition || !Primary->hasMemberSpecialization()) {
+ while (!ForDefinition || !Primary->isMemberSpecialization()) {
if (auto *NewPrimary = Primary->getInstantiatedFromMemberTemplate())
Primary = NewPrimary;
else
@@ -4170,7 +4170,7 @@ FunctionTemplateDecl *FunctionDecl::getPrimaryTemplate() const {
if (FunctionTemplateSpecializationInfo *Info
= TemplateOrSpecialization
.dyn_cast<FunctionTemplateSpecializationInfo*>()) {
- return Info->getTemplate();
+ return Info->getTemplate()->getMostRecentDecl();
}
return nullptr;
}
diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index db0ea62a2323eb..1c92fd9e3ff067 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -2030,7 +2030,7 @@ const CXXRecordDecl *CXXRecordDecl::getTemplateInstantiationPattern() const {
if (auto *TD = dyn_cast<ClassTemplateSpecializationDecl>(this)) {
auto From = TD->getInstantiatedFrom();
if (auto *CTD = From.dyn_cast<ClassTemplateDecl *>()) {
- while (!CTD->hasMemberSpecialization()) {
+ while (!CTD->isMemberSpecialization()) {
if (auto *NewCTD = CTD->getInstantiatedFromMemberTemplate())
CTD = NewCTD;
else
@@ -2040,7 +2040,7 @@ const CXXRecordDecl *CXXRecordDecl::getTemplateInstantiationPattern() const {
}
if (auto *CTPSD =
From.dyn_cast<ClassTemplatePartialSpecializationDecl *>()) {
- while (!CTPSD->hasMemberSpecialization()) {
+ while (!CTPSD->isMemberSpecialization()) {
if (auto *NewCTPSD = CTPSD->getInstantiatedFromMemberTemplate())
CTPSD = NewCTPSD;
else
diff --git a/clang/lib/AST/DeclTemplate.cpp b/clang/lib/AST/DeclTemplate.cpp
index 755ec72f00bf77..1db02d0d04448c 100644
--- a/clang/lib/AST/DeclTemplate.cpp
+++ b/clang/lib/AST/DeclTemplate.cpp
@@ -993,7 +993,17 @@ ClassTemplateSpecializationDecl::getSpecializedTemplate() const {
if (const auto *PartialSpec =
SpecializedTemplate.dyn_cast<SpecializedPartialSpecialization*>())
return PartialSpec->PartialSpecialization->getSpecializedTemplate();
- return SpecializedTemplate.get<ClassTemplateDecl*>();
+ return SpecializedTemplate.get<ClassTemplateDecl *>()->getMostRecentDecl();
+}
+
+llvm::PointerUnion<ClassTemplateDecl *,
+ ClassTemplatePartialSpecializationDecl *>
+ClassTemplateSpecializationDecl::getSpecializedTemplateOrPartial() const {
+ if (const auto *PartialSpec =
+ SpecializedTemplate.dyn_cast<SpecializedPartialSpecialization *>())
+ return PartialSpec->PartialSpecialization->getMostRecentDecl();
+
+ return SpecializedTemplate.get<ClassTemplateDecl *>()->getMostRecentDecl();
}
SourceRange
@@ -1283,6 +1293,39 @@ VarTemplateDecl::newCommon(ASTContext &C) const {
return CommonPtr;
}
+void VarTemplateDecl::mergePrevDecl(VarTemplateDecl *Prev) {
+ // If we haven't created a common pointer yet, then it can just be created
+ // with the usual method.
+ if (!getCommonPtrInternal())
+ return;
+
+ Common *ThisCommon = static_cast<Common *>(getCommonPtrInternal());
+ Common *PrevCommon = nullptr;
+ SmallVector<VarTemplateDecl *, 8> PreviousDecls;
+ for (; Prev; Prev = Prev->getPreviousDecl()) {
+ if (CommonBase *C = Prev->getCommonPtrInternal()) {
+ PrevCommon = static_cast<Common *>(C);
+ break;
+ }
+ PreviousDecls.push_back(Prev);
+ }
+
+ // If the previous redecl chain hasn't created a common pointer yet, then just
+ // use this common pointer.
+ if (!PrevCommon) {
+ for (auto *D : PreviousDecls)
+ D->setCommonPtr(ThisCommon);
+ return;
+ }
+
+ // Ensure we don't leak any important state.
+ assert(ThisCommon->Specializations.empty() &&
+ ThisCommon->PartialSpecializations.empty() &&
+ "Can't merge incompatible declarations!");
+
+ setCommonPtr(PrevCommon);
+}
+
VarTemplateSpecializationDecl *
VarTemplateDecl::findSpecialization(ArrayRef<TemplateArgument> Args,
void *&InsertPos) {
@@ -1405,7 +1448,16 @@ VarTemplateDecl *VarTemplateSpecializationDecl::getSpecializedTemplate() const {
if (const auto *PartialSpec =
SpecializedTemplate.dyn_cast<SpecializedPartialSpecialization *>())
return PartialSpec->PartialSpecialization->getSpecializedTemplate();
- return SpecializedTemplate.get<VarTemplateDecl *>();
+ return SpecializedTemplate.get<VarTemplateDecl *>()->getMostRecentDecl();
+}
+
+llvm::PointerUnion<VarTemplateDecl *, VarTemplatePartialSpecializationDecl *>
+VarTemplateSpecializationDecl::getSpecializedTemplateOrPartial() const {
+ if (const auto *PartialSpec =
+ SpecializedTemplate.dyn_cast<SpecializedPartialSpecialization *>())
+ return PartialSpec->PartialSpecialization->getMostRecentDecl();
+
+ return SpecializedTemplate.get<VarTemplateDecl *>()->getMostRecentDecl();
}
SourceRange VarTemplateSpecializationDecl::getSourceRange() const {
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index f8e5f3c6d309d6..3e8b76e8dfd625 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -4696,8 +4696,10 @@ void Sema::MergeVarDecl(VarDecl *New, LookupResult &Previous) {
// Keep a chain of previous declarations.
New->setPreviousDecl(Old);
- if (NewTemplate)
+ if (NewTemplate) {
+ NewTemplate->mergePrevDecl(OldTemplate);
NewTemplate->setPreviousDecl(OldTemplate);
+ }
// Inherit access appropriately.
New->setAccess(Old->getAccess());
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 573e90aced3eea..e2a59f63ccf589 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -9954,7 +9954,7 @@ QualType Sema::DeduceTemplateSpecializationFromInitializer(
auto SynthesizeAggrGuide = [&](InitListExpr *ListInit) {
auto *Pattern = Template;
while (Pattern->getInstantiatedFromMemberTemplate()) {
- if (Pattern->hasMemberSpecialization())
+ if (Pattern->isMemberSpecialization())
break;
Pattern = Pattern->getInstantiatedFromMemberTemplate();
}
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index b63063813f1b56..de0ec0128905ff 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -343,7 +343,7 @@ struct TemplateInstantiationArgumentCollecter
// If this function was instantiated from a specialized member that is
// a function template, we're done.
assert(FD->getPrimaryTemplate() && "No function template?");
- if (FD->getPrimaryTemplate()->hasMemberSpecialization())
+ if (FD->getPrimaryTemplate()->isMemberSpecialization())
return Done();
// If this function is a generic lambda specialization, we are done.
@@ -442,11 +442,11 @@ struct TemplateInstantiationArgumentCollecter
Specialized = CTSD->getSpecializedTemplateOrPartial();
if (auto *CTPSD =
Specialized.dyn_cast<ClassTemplatePartialSpecializationDecl *>()) {
- if (CTPSD->hasMemberSpecialization())
+ if (CTPSD->isMemberSpecialization())
return Done();
} else {
auto *CTD = Specialized.get<ClassTemplateDecl *>();
- if (CTD->hasMemberSpecialization())
+ if (CTD->isMemberSpecialization())
return Done();
}
return UseNextDecl(CTSD);
@@ -478,11 +478,11 @@ struct TemplateInstantiationArgumentCollecter
Specialized = VTSD->getSpecializedTemplateOrPartial();
if (auto *VTPSD =
Specialized.dyn_cast<VarTemplatePartialSpecializationDecl *>()) {
- if (VTPSD->hasMemberSpecialization())
+ if (VTPSD->isMemberSpecialization())
return Done();
} else {
auto *VTD = Specialized.get<VarTemplateDecl *>();
- if (VTD->hasMemberSpecialization())
+ if (VTD->isMemberSpecialization())
return Done();
}
return UseNextDecl(VTSD);
@@ -4141,7 +4141,7 @@ getPatternForClassTemplateSpecialization(
CXXRecordDecl *Pattern = nullptr;
Specialized = ClassTemplateSpec->getSpecializedTemplateOrPartial();
if (auto *CTD = Specialized.dyn_cast<ClassTemplateDecl *>()) {
- while (!CTD->hasMemberSpecialization()) {
+ while (!CTD->isMemberSpecialization()) {
if (auto *NewCTD = CTD->getInstantiatedFromMemberTemplate())
CTD = NewCTD;
else
@@ -4151,7 +4151,7 @@ getPatternForClassTemplateSpecialization(
} else if (auto *CTPSD =
Specialized
.dyn_cast<ClassTemplatePartialSpecializationDecl *>()) {
- while (!CTPSD->hasMemberSpecialization()) {
+ while (!CTPSD->isMemberSpecialization()) {
if (auto *NewCTPSD = CTPSD->getInstantiatedFromMemberTemplate())
CTPSD = NewCTPSD;
else
diff --git a/clang/test/AST/ast-dump-decl.cpp b/clang/test/AST/ast-dump-decl.cpp
index e84241cee922f5..7b998f20944f49 100644
--- a/clang/test/AST/ast-dump-decl.cpp
+++ b/clang/test/AST/ast-dump-decl.cpp
@@ -530,7 +530,7 @@ namespace testCanonicalTemplate {
// CHECK-NEXT: | `-ClassTemplateDecl 0x{{.+}} parent 0x{{.+}} <col:5, col:40> col:40 friend_undeclared TestClassTemplate{{$}}
// CHECK-NEXT: | |-TemplateTypeParmDecl 0x{{.+}} <col:14, col:23> col:23 typename depth 1 index 0 T2{{$}}
// CHECK-NEXT: | `-CXXRecordDecl 0x{{.+}} parent 0x{{.+}} <col:34, col:40> col:40 class TestClassTemplate{{$}}
- // CHECK-NEXT: `-ClassTemplateSpecializationDecl 0x{{.+}} <line:[[@LINE-19]]:3, line:[[@LINE-17]]:3> line:[[@LINE-19]]:31 class TestClassTemplate definition implicit_instantiation{{$}}
+ // CHECK-NEXT: `-ClassTemplateSpecializationDecl 0x{{.+}} <col:5, col:40> line:[[@LINE-19]]:31 class TestClassTemplate definition implicit_instantiation{{$}}
// CHECK-NEXT: |-DefinitionData pass_in_registers empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init{{$}}
// CHECK-NEXT: | |-DefaultConstructor exists trivial constexpr defaulted_is_constexpr{{$}}
// CHECK-NEXT: | |-CopyConstructor simple trivial has_const_param implicit_has_const_param{{$}}
diff --git a/clang/test/CXX/temp/temp.spec/temp.expl.spec/p7.cpp b/clang/test/CXX/temp/temp.spec/temp.expl.spec/p7.cpp
index 87127366eb58a5..e7e4738032f647 100644
--- a/clang/test/CXX/temp/temp.spec/temp.expl.spec/p7.cpp
+++ b/clang/test/CXX/temp/temp.spec/temp.expl.spec/p7.cpp
@@ -177,6 +177,93 @@ namespace Defined {
static_assert(A<short>::B<int*>::y == 2);
} // namespace Defined
+namespace Constrained {
+ template<typename T>
+ struct A {
+ template<typename U, bool V> requires V
+ static constexpr int f(); // expected-note {{declared here}}
+
+ template<typename U, bool V> requires V
+ static const int x; // expected-note {{declared here}}
+
+ template<typename U, bool V> requires V
+ static const int x<U*, V>; // expected-note {{declared here}}
+
+ template<typename U, bool V> requires V
+ struct B; // expected-note {{template is declared here}}
+
+ template<typename U, bool V> requires V
+ struct B<U*, V>; // expected-note {{template is declared here}}
+ };
+
+ template<>
+ template<typename U, bool V> requires V
+ constexpr int A<short>::f() {
+ return A<long>::f<U, V>();
+ }
+
+ template<>
+ template<typename U, bool V> requires V
+ constexpr int A<short>::x = A<long>::x<U, V>;
+
+ template<>
+ template<typename U, bool V> requires V
+ constexpr int A<short>::x<U*, V> = A<long>::x<U*, V>;
+
+ template<>
+ template<typename U, bool V> requires V
+ struct A<short>::B<U*, V> {
+ static constexpr int y = A<long>::B<U*, V>::y;
+ };
+
+ template<>
+ template<typename U, bool V> requires V
+ struct A<short>::B {
+ static constexpr int y = A<long>::B<U, V>::y;
+ };
+
+ template<>
+ template<typename U, bool V> requires V
+ constexpr int A<long>::f() {
+ return 1;
+ }
+
+ template<>
+ template<typename U, bool V> requires V
+ constexpr int A<long>::x = 1;
+
+ template<>
+ template<typename U, bool V> requires V
+ constexpr int A<long>::x<U*, V> = 2;
+
+ template<>
+ template<typename U, bool V> requires V
+ struct A<long>::B {
+ static constexpr int y = 1;
+ };
+
+ template<>
+ template<typename U, bool V> requires V
+ struct A<long>::B<U*, V> {
+ static constexpr int y = 2;
+ };
+
+ static_assert(A<int>::f<int, true>() == 0); // expected-error {{static assertion expression is not an integral constant expression}}
+ // expected-note@-1 {{undefined function 'f<int, true>' cannot be used in a constant expression}}
+ static_assert(A<int>::x<int, true> == 0); // expected-error {{static assertion expression is not an integral constant expression}}
+ // expected-note@-1 {{initializer of 'x<int, true>' is unknown}}
+ static_assert(A<int>::x<int*, true> == 0); // expected-error {{static assertion expression is not an integral constant expression}}
+ // expected-note@-1 {{initializer of 'x<int *, true>' is unknown}}
+ static_assert(A<int>::B<int, true>::y == 0); // expected-error {{implicit instantiation of undefined template 'Constrained::A<int>::B<int, true>'}}
+ static_assert(A<int>::B<int*, true>::y == 0); // expected-error {{implicit instantiation of undefined template 'Constrained::A<int>::B<int *, true>'}}
+
+ static_assert(A<short>::f<int, true>() == 1);
+ static_assert(A<short>::x<int, true> == 1);
+ static_assert(A<short>::x<int*, true> == 2);
+ static_assert(A<short>::B<int, true>::y == 1);
+ static_assert(A<short>::B<int*, true>::y == 2);
+} // namespace Constrained
+
namespace Dependent {
template<int I>
struct A {
|
erichkeane
approved these changes
Oct 30, 2024
|
Kind ping! This is blocking us in google and we'd appreciate merging today. Even trying to internally revert the offending patch is difficult as i touches many core files. Thanks |
|
@asmok-g You're in luck, because I was just about to merge this now :) |
|
@sdkrystian thanks a lot : ) |
|
I think this may have broken the LLDB bots: https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/14349/console Currently trying to verify (and get the actual crash message), but this is the only clang-related commit in the changelist, and the tests are all clang related |
It isn't clear to me right now what the actual failure in those tests is from that log. That said, if you get to be somewhat more sure that this is the patch that caused it, feel free to revert and @sdkrystian will look at it more closely tomorrow. I SUSPECT we might have a small issue we didn't think of on this patch, so hopefully ti is something minor/easily fixable. |
|
@erichkeane @felipepiovezan Given that all the failing tests relate to expression evaluation within modules, I think the problem is in |
|
There seems to be some kind of infinite recursion, I attached the debugger to one such test, and the stack at the time of the crash has 59,000 frames. |
|
I reverted locally and confirmed that it was this commit that introduced the regression. I don't know much about clang importer to be honest, but this can be reproed by adding If you want to attach a debugger to it, you could also: |
I'm going to revert it for the time being to unblock the bots |
Sorry, was off at lunch :) That seems reasonable/appropriate. |
|
I should hopefully have a fix ready tomorrow... apologies for the delay @asmok-g! Getting this patch right is a bit hard since it affects all template instantiation related things :) |
|
I (finally) got a minimal reproducer together:
// clang -cc1 -emit-pch -o a.ast a.cpp
template<typename T>
struct A
{
template<typename U>
friend struct A;
};
template struct A<int>;
// clang -cc1 -ast-merge a.ast b.cpp
template struct A<short>; // crash here |
|
@felipepiovezan I tested without this patch applied and was still seeing crashes... could you perhaps see if https://github.com/sdkrystian/llvm-project/tree/reapply-use-latest-primary fixes the regression? |
Let me give it a try now! |
Yup, that fixes the issue. I've also confirmed that reverting the fix brings back the crash! It is weird that you were unable to repro this 🤔 (or rather that it was also crashing when you reverted the original patch...) Are you 100% sure you recompiled LLDB after reverting? |
Great!
So... I usually compile clang with msvc on windows (I would use clang-cl if VS2022 didn't hang when debugging). I've spent many hours trying to get LLDB to compile using my current environment without success, so I instead compile in WSL. Since |
|
Once I finish writing tests, I'll open a PR to reapply this patch |
smallp-o-p
pushed a commit
to smallp-o-p/llvm-project
that referenced
this pull request
Nov 3, 2024
…vm#114258) This patch fixes a couple of regressions introduced in llvm#111852. Consider: ``` template<typename T> struct A { template<bool U> static constexpr bool f() requires U { return true; } }; template<> template<bool U> constexpr bool A<short>::f() requires U { return A<long>::f<U>(); } template<> template<bool U> constexpr bool A<long>::f() requires U { return true; } static_assert(A<short>::f<true>()); // crash here ``` This crashes because when collecting template arguments from the _first_ declaration of `A<long>::f<true>` for constraint checking, we don't add the template arguments from the enclosing class template specialization because there exists another redeclaration that is a member specialization. This also fixes the following example, which happens for a similar reason: ``` // input.cppm export module input; export template<int N> constexpr int f(); template<int N> struct A { template<int J> friend constexpr int f(); }; template struct A<0>; template<int N> constexpr int f() { return N; } ``` ``` // input.cpp import input; static_assert(f<1>() == 1); // error: static assertion failed ```
smallp-o-p
pushed a commit
to smallp-o-p/llvm-project
that referenced
this pull request
Nov 3, 2024
…late" (llvm#114304) Clang importer doesn't seem to work well with this change, see discussion in the original PR. Reverts llvm#114258
smallp-o-p
pushed a commit
to smallp-o-p/llvm-project
that referenced
this pull request
Nov 3, 2024
…plate" (llvm#114569) This patch reapplies llvm#114258, fixing an infinite recursion bug in `ASTImporter` that occurs when importing the primary template of a class template specialization when the latest redeclaration of that template is a friend declaration in the primary template.
smallp-o-p
pushed a commit
to smallp-o-p/llvm-project
that referenced
this pull request
Nov 3, 2024
…plate" (llvm#114569) This patch reapplies llvm#114258, fixing an infinite recursion bug in `ASTImporter` that occurs when importing the primary template of a class template specialization when the latest redeclaration of that template is a friend declaration in the primary template.
NoumanAmir657
pushed a commit
to NoumanAmir657/llvm-project
that referenced
this pull request
Nov 4, 2024
…vm#114258) This patch fixes a couple of regressions introduced in llvm#111852. Consider: ``` template<typename T> struct A { template<bool U> static constexpr bool f() requires U { return true; } }; template<> template<bool U> constexpr bool A<short>::f() requires U { return A<long>::f<U>(); } template<> template<bool U> constexpr bool A<long>::f() requires U { return true; } static_assert(A<short>::f<true>()); // crash here ``` This crashes because when collecting template arguments from the _first_ declaration of `A<long>::f<true>` for constraint checking, we don't add the template arguments from the enclosing class template specialization because there exists another redeclaration that is a member specialization. This also fixes the following example, which happens for a similar reason: ``` // input.cppm export module input; export template<int N> constexpr int f(); template<int N> struct A { template<int J> friend constexpr int f(); }; template struct A<0>; template<int N> constexpr int f() { return N; } ``` ``` // input.cpp import input; static_assert(f<1>() == 1); // error: static assertion failed ```
NoumanAmir657
pushed a commit
to NoumanAmir657/llvm-project
that referenced
this pull request
Nov 4, 2024
…late" (llvm#114304) Clang importer doesn't seem to work well with this change, see discussion in the original PR. Reverts llvm#114258
NoumanAmir657
pushed a commit
to NoumanAmir657/llvm-project
that referenced
this pull request
Nov 4, 2024
…plate" (llvm#114569) This patch reapplies llvm#114258, fixing an infinite recursion bug in `ASTImporter` that occurs when importing the primary template of a class template specialization when the latest redeclaration of that template is a friend declaration in the primary template.
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.
This patch fixes a couple of regressions introduced in #111852.
Consider:
This crashes because when collecting template arguments from the first declaration of
A<long>::f<true>for constraint checking, we don't add the template arguments from the enclosing class template specialization because there exists another redeclaration that is a member specialization.This also fixes the following example, which happens for a similar reason: