Reapply "[Clang][Sema] Refactor collection of multi-level template argument lists (#106585, #111173)" #111852
+1,066
−704
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-modules @llvm/pr-subscribers-clang Author: Krystian Stasiowski (sdkrystian) ChangesThis patch reapplies #111173, fixing a bug when instantiating dependent expressions that name a member template that is later explicitly specialized for a class specialization that is implicitly instantiated. The bug is addressed by adding the Patch is 105.30 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/111852.diff 20 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c0019cfe4658d7..14cde29d445434 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -488,6 +488,9 @@ Bug Fixes to C++ Support
in certain friend declarations. (#GH93099)
- Clang now instantiates the correct lambda call operator when a lambda's class type is
merged across modules. (#GH110401)
+- Clang now uses the correct set of template argument lists when comparing the constraints of
+ out-of-line definitions and member templates explicitly specialized for a given implicit instantiation of
+ a class template. (#GH102320)
- Fix a crash when parsing a pseudo destructor involving an invalid type. (#GH111460)
- Fixed an assertion failure when invoking recovery call expressions with explicit attributes
and undeclared templates. (#GH107047, #GH49093)
diff --git a/clang/include/clang/AST/DeclTemplate.h b/clang/include/clang/AST/DeclTemplate.h
index 687715a22e9fd3..141f58c4600af0 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -781,15 +781,11 @@ class RedeclarableTemplateDecl : public TemplateDecl,
EntryType *Entry, void *InsertPos);
struct CommonBase {
- CommonBase() : InstantiatedFromMember(nullptr, false) {}
+ CommonBase() {}
/// The template from which this was most
/// directly instantiated (or null).
- ///
- /// The boolean value indicates whether this template
- /// was explicitly specialized.
- llvm::PointerIntPair<RedeclarableTemplateDecl*, 1, bool>
- InstantiatedFromMember;
+ RedeclarableTemplateDecl *InstantiatedFromMember = nullptr;
/// If non-null, points to an array of specializations (including
/// partial specializations) known only by their external declaration IDs.
@@ -809,14 +805,19 @@ class RedeclarableTemplateDecl : public TemplateDecl,
};
/// Pointer to the common data shared by all declarations of this
- /// template.
- mutable CommonBase *Common = nullptr;
+ /// template, and a flag indicating if the template is a member
+ /// specialization.
+ mutable llvm::PointerIntPair<CommonBase *, 1, bool> Common;
+
+ CommonBase *getCommonPtrInternal() const { return Common.getPointer(); }
/// Retrieves the "common" pointer shared by all (re-)declarations of
/// the same template. Calling this routine may implicitly allocate memory
/// for the common pointer.
CommonBase *getCommonPtr() const;
+ void setCommonPtr(CommonBase *C) const { Common.setPointer(C); }
+
virtual CommonBase *newCommon(ASTContext &C) const = 0;
// Construct a template decl with name, parameters, and templated element.
@@ -857,15 +858,22 @@ class RedeclarableTemplateDecl : public TemplateDecl,
/// template<> template<typename T>
/// struct X<int>::Inner { /* ... */ };
/// \endcode
- bool isMemberSpecialization() const {
- return getCommonPtr()->InstantiatedFromMember.getInt();
+ 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(getCommonPtr()->InstantiatedFromMember.getPointer() &&
- "Only member templates can be member template specializations");
- getCommonPtr()->InstantiatedFromMember.setInt(true);
+ assert(!isMemberSpecialization() && "already a member specialization");
+ Common.setInt(true);
}
/// Retrieve the member template from which this template was
@@ -905,12 +913,12 @@ class RedeclarableTemplateDecl : public TemplateDecl,
/// void X<T>::f(T, U);
/// \endcode
RedeclarableTemplateDecl *getInstantiatedFromMemberTemplate() const {
- return getCommonPtr()->InstantiatedFromMember.getPointer();
+ return getCommonPtr()->InstantiatedFromMember;
}
void setInstantiatedFromMemberTemplate(RedeclarableTemplateDecl *TD) {
- assert(!getCommonPtr()->InstantiatedFromMember.getPointer());
- getCommonPtr()->InstantiatedFromMember.setPointer(TD);
+ assert(!getCommonPtr()->InstantiatedFromMember);
+ getCommonPtr()->InstantiatedFromMember = TD;
}
/// Retrieve the "injected" template arguments that correspond to the
@@ -1989,6 +1997,8 @@ class ClassTemplateSpecializationDecl : public CXXRecordDecl,
/// template arguments have been deduced.
void setInstantiationOf(ClassTemplatePartialSpecializationDecl *PartialSpec,
const TemplateArgumentList *TemplateArgs) {
+ assert(!isa<ClassTemplatePartialSpecializationDecl>(this) &&
+ "A partial specialization cannot be instantiated from a template");
assert(!SpecializedTemplate.is<SpecializedPartialSpecialization*>() &&
"Already set to a class template partial specialization!");
auto *PS = new (getASTContext()) SpecializedPartialSpecialization();
@@ -2000,6 +2010,8 @@ class ClassTemplateSpecializationDecl : public CXXRecordDecl,
/// Note that this class template specialization is an instantiation
/// of the given class template.
void setInstantiationOf(ClassTemplateDecl *TemplDecl) {
+ assert(!isa<ClassTemplatePartialSpecializationDecl>(this) &&
+ "A partial specialization cannot be instantiated from a template");
assert(!SpecializedTemplate.is<SpecializedPartialSpecialization*>() &&
"Previously set to a class template partial specialization!");
SpecializedTemplate = TemplDecl;
@@ -2187,19 +2199,23 @@ class ClassTemplatePartialSpecializationDecl
/// struct X<int>::Inner<T*> { /* ... */ };
/// \endcode
bool isMemberSpecialization() const {
- const auto *First =
- cast<ClassTemplatePartialSpecializationDecl>(getFirstDecl());
- return First->InstantiatedFromMember.getInt();
+ return InstantiatedFromMember.getInt();
}
- /// Note that this member template is a specialization.
- void setMemberSpecialization() {
- auto *First = cast<ClassTemplatePartialSpecializationDecl>(getFirstDecl());
- assert(First->InstantiatedFromMember.getPointer() &&
- "Only member templates can be member template specializations");
- return First->InstantiatedFromMember.setInt(true);
+ /// 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); }
+
/// Retrieves the injected specialization type for this partial
/// specialization. This is not the same as the type-decl-type for
/// this partial specialization, which is an InjectedClassNameType.
@@ -2268,10 +2284,6 @@ class ClassTemplateDecl : public RedeclarableTemplateDecl {
return static_cast<Common *>(RedeclarableTemplateDecl::getCommonPtr());
}
- void setCommonPtr(Common *C) {
- RedeclarableTemplateDecl::Common = C;
- }
-
public:
friend class ASTDeclReader;
@@ -2754,6 +2766,8 @@ class VarTemplateSpecializationDecl : public VarDecl,
/// template arguments have been deduced.
void setInstantiationOf(VarTemplatePartialSpecializationDecl *PartialSpec,
const TemplateArgumentList *TemplateArgs) {
+ assert(!isa<VarTemplatePartialSpecializationDecl>(this) &&
+ "A partial specialization cannot be instantiated from a template");
assert(!SpecializedTemplate.is<SpecializedPartialSpecialization *>() &&
"Already set to a variable template partial specialization!");
auto *PS = new (getASTContext()) SpecializedPartialSpecialization();
@@ -2765,6 +2779,8 @@ class VarTemplateSpecializationDecl : public VarDecl,
/// Note that this variable template specialization is an instantiation
/// of the given variable template.
void setInstantiationOf(VarTemplateDecl *TemplDecl) {
+ assert(!isa<VarTemplatePartialSpecializationDecl>(this) &&
+ "A partial specialization cannot be instantiated from a template");
assert(!SpecializedTemplate.is<SpecializedPartialSpecialization *>() &&
"Previously set to a variable template partial specialization!");
SpecializedTemplate = TemplDecl;
@@ -2949,19 +2965,24 @@ class VarTemplatePartialSpecializationDecl
/// U* X<int>::Inner<T*> = (T*)(0) + 1;
/// \endcode
bool isMemberSpecialization() const {
- const auto *First =
- cast<VarTemplatePartialSpecializationDecl>(getFirstDecl());
- return First->InstantiatedFromMember.getInt();
+ return InstantiatedFromMember.getInt();
}
- /// Note that this member template is a specialization.
- void setMemberSpecialization() {
- auto *First = cast<VarTemplatePartialSpecializationDecl>(getFirstDecl());
- assert(First->InstantiatedFromMember.getPointer() &&
- "Only member templates can be member template specializations");
- return First->InstantiatedFromMember.setInt(true);
+ /// 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); }
+
SourceRange getSourceRange() const override LLVM_READONLY;
void Profile(llvm::FoldingSetNodeID &ID) const {
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index ef010fafb1573e..c4d15c776c19c6 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -11329,9 +11329,9 @@ class Sema final : public SemaBase {
CXXScopeSpec &SS, IdentifierInfo *Name, SourceLocation NameLoc,
const ParsedAttributesView &Attr, TemplateParameterList *TemplateParams,
AccessSpecifier AS, SourceLocation ModulePrivateLoc,
- SourceLocation FriendLoc, unsigned NumOuterTemplateParamLists,
- TemplateParameterList **OuterTemplateParamLists,
- SkipBodyInfo *SkipBody = nullptr);
+ SourceLocation FriendLoc,
+ ArrayRef<TemplateParameterList *> OuterTemplateParamLists,
+ bool IsMemberSpecialization, SkipBodyInfo *SkipBody = nullptr);
/// Translates template arguments as provided by the parser
/// into template arguments used by semantic analysis.
@@ -11370,7 +11370,8 @@ class Sema final : public SemaBase {
DeclResult ActOnVarTemplateSpecialization(
Scope *S, Declarator &D, TypeSourceInfo *DI, LookupResult &Previous,
SourceLocation TemplateKWLoc, TemplateParameterList *TemplateParams,
- StorageClass SC, bool IsPartialSpecialization);
+ StorageClass SC, bool IsPartialSpecialization,
+ bool IsMemberSpecialization);
/// Get the specialization of the given variable template corresponding to
/// the specified argument list, or a null-but-valid result if the arguments
@@ -13026,28 +13027,14 @@ class Sema final : public SemaBase {
/// dealing with a specialization. This is only relevant for function
/// template specializations.
///
- /// \param Pattern If non-NULL, indicates the pattern from which we will be
- /// instantiating the definition of the given declaration, \p ND. This is
- /// used to determine the proper set of template instantiation arguments for
- /// friend function template specializations.
- ///
/// \param ForConstraintInstantiation when collecting arguments,
/// ForConstraintInstantiation indicates we should continue looking when
/// encountering a lambda generic call operator, and continue looking for
/// arguments on an enclosing class template.
- ///
- /// \param SkipForSpecialization when specified, any template specializations
- /// in a traversal would be ignored.
- /// \param ForDefaultArgumentSubstitution indicates we should continue looking
- /// when encountering a specialized member function template, rather than
- /// returning immediately.
MultiLevelTemplateArgumentList getTemplateInstantiationArgs(
const NamedDecl *D, const DeclContext *DC = nullptr, bool Final = false,
std::optional<ArrayRef<TemplateArgument>> Innermost = std::nullopt,
- bool RelativeToPrimary = false, const FunctionDecl *Pattern = nullptr,
- bool ForConstraintInstantiation = false,
- bool SkipForSpecialization = false,
- bool ForDefaultArgumentSubstitution = false);
+ bool RelativeToPrimary = false, bool ForConstraintInstantiation = false);
/// RAII object to handle the state changes required to synthesize
/// a function body.
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 84ef9f74582ef6..6f1bcf397bba04 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -2696,21 +2696,21 @@ VarDecl *VarDecl::getTemplateInstantiationPattern() const {
if (isTemplateInstantiation(VDTemplSpec->getTemplateSpecializationKind())) {
auto From = VDTemplSpec->getInstantiatedFrom();
if (auto *VTD = From.dyn_cast<VarTemplateDecl *>()) {
- while (!VTD->isMemberSpecialization()) {
- auto *NewVTD = VTD->getInstantiatedFromMemberTemplate();
- if (!NewVTD)
+ while (!VTD->hasMemberSpecialization()) {
+ if (auto *NewVTD = VTD->getInstantiatedFromMemberTemplate())
+ VTD = NewVTD;
+ else
break;
- VTD = NewVTD;
}
return getDefinitionOrSelf(VTD->getTemplatedDecl());
}
if (auto *VTPSD =
From.dyn_cast<VarTemplatePartialSpecializationDecl *>()) {
- while (!VTPSD->isMemberSpecialization()) {
- auto *NewVTPSD = VTPSD->getInstantiatedFromMember();
- if (!NewVTPSD)
+ while (!VTPSD->hasMemberSpecialization()) {
+ if (auto *NewVTPSD = VTPSD->getInstantiatedFromMember())
+ VTPSD = NewVTPSD;
+ else
break;
- VTPSD = NewVTPSD;
}
return getDefinitionOrSelf<VarDecl>(VTPSD);
}
@@ -2719,15 +2719,14 @@ 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 *VarTemplate = VD->getDescribedVarTemplate()) {
- while (!VarTemplate->isMemberSpecialization()) {
- auto *NewVT = VarTemplate->getInstantiatedFromMemberTemplate();
- if (!NewVT)
+ if (VarTemplateDecl *VTD = VD->getDescribedVarTemplate()) {
+ while (!VTD->hasMemberSpecialization()) {
+ if (auto *NewVTD = VTD->getInstantiatedFromMemberTemplate())
+ VTD = NewVTD;
+ else
break;
- VarTemplate = NewVT;
}
-
- return getDefinitionOrSelf(VarTemplate->getTemplatedDecl());
+ return getDefinitionOrSelf(VTD->getTemplatedDecl());
}
if (VD == this)
@@ -4142,11 +4141,11 @@ 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->isMemberSpecialization()) {
- auto *NewPrimary = Primary->getInstantiatedFromMemberTemplate();
- if (!NewPrimary)
+ while (!ForDefinition || !Primary->hasMemberSpecialization()) {
+ if (auto *NewPrimary = Primary->getInstantiatedFromMemberTemplate())
+ Primary = NewPrimary;
+ else
break;
- Primary = NewPrimary;
}
return getDefinitionOrSelf(Primary->getTemplatedDecl());
diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index 1364ccc745ba01..407ec14bbc00d5 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -2023,19 +2023,21 @@ const CXXRecordDecl *CXXRecordDecl::getTemplateInstantiationPattern() const {
if (auto *TD = dyn_cast<ClassTemplateSpecializationDecl>(this)) {
auto From = TD->getInstantiatedFrom();
if (auto *CTD = From.dyn_cast<ClassTemplateDecl *>()) {
- while (auto *NewCTD = CTD->getInstantiatedFromMemberTemplate()) {
- if (NewCTD->isMemberSpecialization())
+ while (!CTD->hasMemberSpecialization()) {
+ if (auto *NewCTD = CTD->getInstantiatedFromMemberTemplate())
+ CTD = NewCTD;
+ else
break;
- CTD = NewCTD;
}
return GetDefinitionOrSelf(CTD->getTemplatedDecl());
}
if (auto *CTPSD =
From.dyn_cast<ClassTemplatePartialSpecializationDecl *>()) {
- while (auto *NewCTPSD = CTPSD->getInstantiatedFromMember()) {
- if (NewCTPSD->isMemberSpecialization())
+ while (!CTPSD->hasMemberSpecialization()) {
+ if (auto *NewCTPSD = CTPSD->getInstantiatedFromMemberTemplate())
+ CTPSD = NewCTPSD;
+ else
break;
- CTPSD = NewCTPSD;
}
return GetDefinitionOrSelf(CTPSD);
}
diff --git a/clang/lib/AST/DeclTemplate.cpp b/clang/lib/AST/DeclTemplate.cpp
index 6fe817c5ef1c6b..d9b67b7bedf5a5 100644
--- a/clang/lib/AST/DeclTemplate.cpp
+++ b/clang/lib/AST/DeclTemplate.cpp
@@ -309,16 +309,16 @@ bool TemplateDecl::isTypeAlias() const {
void RedeclarableTemplateDecl::anchor() {}
RedeclarableTemplateDecl::CommonBase *RedeclarableTemplateDecl::getCommonPtr() const {
- if (Common)
- return Common;
+ if (CommonBase *C = getCommonPtrInternal())
+ return C;
// Walk the previous-declaration chain until we either find a declaration
// with a common pointer or we run out of previous declarations.
SmallVector<const RedeclarableTemplateDecl *, 2> PrevDecls;
for (const RedeclarableTemplateDecl *Prev = getPreviousDecl(); Prev;
Prev = Prev->getPreviousDecl()) {
- if (Prev->Common) {
- Common = Prev->Common;
+ if (CommonBase *C = Prev->getCommonPtrInternal()) {
+ setCommonPtr(C);
break;
}
@@ -326,18 +326,18 @@ RedeclarableTemplateDecl::CommonBase *RedeclarableTemplateDecl::getCommonPtr() c
}
// If we never found a common pointer, allocate one now.
- if (!Common) {
+ if (!getCommonPtrInternal()) {
// FIXME: If any of the declarations is from an AST file, we probably
// need an update record to add the common data.
- Common = newCommon(getASTContext());
+ setCommonPtr(newCommon(getASTContext()));
}
// Update any previous declarations we saw with the common pointer.
for (const RedeclarableTemplateDecl *Prev : PrevDecls)
- Prev->Common = Common;
+ Prev->setCommonPtr(getCommonPtrInternal());
- return Common;
+ return getCommonPtrInternal();
}
void RedeclarableTemplateDecl::loadLazySpecializationsImpl() const {
@@ -463,19 +463,17 @@ void FunctionTemplateDecl::addSpecialization(
}
void FunctionTemplateDecl::mergePrevDecl(FunctionTemplateDecl *Prev) {
- using Base = RedeclarableTemplateDecl;
-
// If we haven't created a common pointer yet, then it can just be created
// with the usual method.
- if (!Base::Common)
+ if (!getCommonPtrInternal())
return;
- Common *ThisCommon = static_cast<Common *>(Base::Common);
+ Common *ThisCommon = static_cast<Common *>(getCommonPtrInternal());
Common *PrevCommon = nullptr;
SmallVector<FunctionTemplateDecl *, 8> PreviousDecls;
for (; Prev; Prev = Prev->getPreviousDecl()) {
- if (Prev->Base::Common) {
- PrevCommon = static_cast<Common *>(Prev->Base::Common);
+ if (CommonBase *C = Prev->getCommonPtrInternal()) {
+ PrevCommon = static_cast<Common *>(C);
break;
}
PreviousDecls.push_back(Prev);
@@ -485,7 +483,7 @@ void FunctionTemplateDecl::mergePrevDecl(FunctionTemplateDecl *Prev) {
// use this common pointer.
if (!PrevCommon) {
for (auto *D : Previou...
[truncated]
|
|
@mstorsjo Tested this patch with QT and it builds without issue. |
Nice, thanks! |
mizvekov
approved these changes
Oct 10, 2024
sdkrystian
force-pushed
the
reapply-targ-collection-again
branch
from
5465a7f
to
72df5e6
Compare
|
We are seeing some internal tests (and tools) failing because the canonical declaration for template specializations has changed in cases like this: template <typename T>
void Foo(T target); // #1
template <typename T>
void Foo(T defn) {} // #2
template <>
void Foo(int specialization) {} // #3a matcher functionDecl(
hasName("Foo"), isDefinition(),
hasParameter(
0, parmVarDecl(hasType(isInt()),
mostCanonicalDecl(parmVarDecl().bind("p")))))),with if (const auto* param = clang::dyn_cast<clang::ParmVarDecl>(&Node)) {
if (const auto* parent = clang::dyn_cast_or_null<clang::FunctionDecl>(
Node.getParentFunctionOrMethod())) {
const clang::FunctionDecl* func = parent;
if (const auto* pattern = func->getTemplateInstantiationPattern()) {
// Traverse from the instantiation to the pattern.
func = pattern;
}
func = func->getCanonicalDecl();
if (parent != func) {
/* remap indicies and get the matching parameter*/
/* ... */
}
}
}has previously returned Is this change in the canonical declaration intended? |
|
Looking through the patch, I think the |
|
@ilya-biryukov This is caused by the change to |
|
Could you dive a little deeper why this change is necessary? E.g. does the most recent redecl have some properties that the canonical declaration (which would be the first one) does not? Also, how do we emulate the old behavior in places that are needed? E.g. in the example above, the code prefers the first redeclaration as a heuristic to find the public declaration instead of the (implementation detail) definition that usually follows later. It's slightly surprising that the call to I understand that Clang APIs are not stable, but we would need some migration path for our tools. |
@ilya-biryukov When instantiating a template from a member template that was explicitly specialized for a given implicitly instantiated specialization of its enclosing class template, we need to keep track of which template was actually used as the primary template (since we consider the specialization to be a redeclaration of the (instantiated) member template). Since the initial declaration & the specialized declaration have different template depths, this (may) result in |
so this is for cases like this, right? template <class T>
struct Foo {
template <class>
struct Bar {
int foo = 10;
};
};
template <>
template <class T>
struct Foo<int>::Bar {
int foo = 10;
};who do we strictly need to change what we pick in the simpler cases above? (my guess would be that we want simpler code, but I want to confirm it) Do you have any ideas on how to revert back to the old behavior? In the code example above, I would've expected that using canonical declaration after getting template instantiation pattern would give us the same results as before. |
|
cc @gribozavr because this commit seems to break the nullability analysis (not upstream, but we've had previous discussions about it and have made changes to keep it working) |
|
It looks like this is linked to this crash: #112222 |
sdkrystian
added a commit
that referenced
this pull request
Oct 16, 2024
… specializations when collecting multi-level template argument lists (#112381) After #111852 refactored multi-level template argument list collection, the following results in a crash: ``` template<typename T, bool B> struct A; template<bool B> struct A<int, B> { void f() requires B; }; template<bool B> void A<int, B>::f() requires B { } // crash here ``` This happens because when collecting template arguments for constraint normalization from a partial specialization, we incorrectly use the template argument list of the partial specialization. We should be using the template argument list of the _template-head_ (as defined in [temp.arg.general] p2). Fixes #112222.
DanielCChen
pushed a commit
to DanielCChen/llvm-project
that referenced
this pull request
Oct 16, 2024
…gument lists (llvm#106585, llvm#111173)" (llvm#111852) This patch reapplies llvm#111173, fixing a bug when instantiating dependent expressions that name a member template that is later explicitly specialized for a class specialization that is implicitly instantiated. The bug is addressed by adding the `hasMemberSpecialization` function, which return `true` if _any_ redeclaration is a member specialization. This is then used when determining the instantiation pattern for a specialization of a template, and when collecting template arguments for a specialization of a template.
bricknerb
pushed a commit
to bricknerb/llvm-project
that referenced
this pull request
Oct 17, 2024
…gument lists (llvm#106585, llvm#111173)" (llvm#111852) This patch reapplies llvm#111173, fixing a bug when instantiating dependent expressions that name a member template that is later explicitly specialized for a class specialization that is implicitly instantiated. The bug is addressed by adding the `hasMemberSpecialization` function, which return `true` if _any_ redeclaration is a member specialization. This is then used when determining the instantiation pattern for a specialization of a template, and when collecting template arguments for a specialization of a template.
bricknerb
pushed a commit
to bricknerb/llvm-project
that referenced
this pull request
Oct 17, 2024
… specializations when collecting multi-level template argument lists (llvm#112381) After llvm#111852 refactored multi-level template argument list collection, the following results in a crash: ``` template<typename T, bool B> struct A; template<bool B> struct A<int, B> { void f() requires B; }; template<bool B> void A<int, B>::f() requires B { } // crash here ``` This happens because when collecting template arguments for constraint normalization from a partial specialization, we incorrectly use the template argument list of the partial specialization. We should be using the template argument list of the _template-head_ (as defined in [temp.arg.general] p2). Fixes llvm#112222.
EricWF
pushed a commit
to efcs/llvm-project
that referenced
this pull request
Oct 22, 2024
… specializations when collecting multi-level template argument lists (llvm#112381) After llvm#111852 refactored multi-level template argument list collection, the following results in a crash: ``` template<typename T, bool B> struct A; template<bool B> struct A<int, B> { void f() requires B; }; template<bool B> void A<int, B>::f() requires B { } // crash here ``` This happens because when collecting template arguments for constraint normalization from a partial specialization, we incorrectly use the template argument list of the partial specialization. We should be using the template argument list of the _template-head_ (as defined in [temp.arg.general] p2). Fixes llvm#112222.
|
I've (unfortunately) stumbled upon a crash caused by this patch: 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 hereI'll investigate tomorrow. Likely another case where we gather template arguments from the "wrong" declaration of |
|
There's a weird error after this change + #112381. It only reproduces with multiple modules
When compiling the binary, I'm getting: I'm still working on getting the full commandline from our build system, but maybe this gives some idea on what's wrong |
|
@steelannelida I'm unable to reproduce this error... by "module" do you mean C++ module (a la |
|
I can reproduce this. I do not know how to build modules in godbolt. Here is the script and the relevant files: 518572d The new breakage is: ./module_reproducer/args.sh
In module 'another_lib':
module_reproducer/another_lib.h:10:10: error: no viable conversion from returned value of type 'vector<std::basic_string<char>>' to function return type 'vector<int>'
10 | return std::vector<T>{};
| ^~~~~~~~~~~~~~~~
module_reproducer/test_lib.cc:4:6: note: in instantiation of function template specialization 'f::A<int>' requested here
4 | f::A<int>();
| ^
/usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_vector.h:598:7: note: candidate constructor not viable: no known conversion from 'std::vector<basic_string<char, char_traits<char>, allocator<char>>>' to 'const vector<int> &' for 1st argument
598 | vector(const vector& __x)
| ^ ~~~~~~~~~~~~~~~~~
/usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_vector.h:617:7: note: candidate constructor not viable: no known conversion from 'std::vector<basic_string<char, char_traits<char>, allocator<char>>>' to 'vector<int> &&' for 1st argument
617 | vector(vector&&) noexcept = default;
| ^ ~~~~~~~~
/usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_vector.h:675:7: note: candidate constructor not viable: no known conversion from 'std::vector<basic_string<char, char_traits<char>, allocator<char>>>' to 'initializer_list<value_type>' (aka 'initializer_list<int>') for 1st argument
675 | vector(initializer_list<value_type> __l,
| ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_vector.h:539:7: note: explicit constructor is not a candidate
539 | vector(const allocator_type& __a) _GLIBCXX_NOEXCEPT
| ^
/usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/stl_vector.h:553:7: note: explicit constructor is not a candidate
553 | vector(size_type __n, const allocator_type& __a = allocator_type())
| ^
1 error generated.Please let me know if you can reproduce or need help reproducing it. |
|
@usx95 Minimal reproducer: // 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 failedI already have a patch that will fix this and the crash I posted a few days ago. Just waiting until two of my other PRs are merged first before I open a PR. |
|
Any updates on this ? This is blocking us in google. When do we expect the fix to be submitted ? |
|
@adam-smnk Should have the fix merged today |
|
@asmok-g @usx95 @steelannelida I have a fix ready in #114258 |
sdkrystian
added a commit
that referenced
this pull request
Oct 30, 2024
…14258) This 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 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
…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
…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 ```
|
Note this is also linked to this crash: #114758 |
Unfortunately, #114258 (and its second iteration - #114569 / b24650e) don't fix the original problem. It looks like something important was lost in the reduction process. We'll try to come up with another reproducer, but this usually takes significant time, and then who knows how many more iterations are needed to fix all issues triggered by this revision. I wonder if it would be possible to temporarily back off this change in order to return the tree to a good state? |
|
@alexfh I can have a comprehensive reversion commit ready tomorrow. If possible, please provide a minimal repro :) |
We appreciate that! We'll try to provide new reduced cases soon. |
|
Hi @sdkrystian , FYI, we also hit an error/assertion that bisected to this #114569 I'm trying working on reducing the repro(it seems very slow). |
|
We've also found a clang crash and a few new compilation errors after #114569. We definitely need to revert it as well as this commit. |
sdkrystian
added a commit
to sdkrystian/llvm-project
that referenced
this pull request
Nov 6, 2024
…plate argument lists (llvm#106585, llvm#111173)" (llvm#111852)" This reverts commit 2bb3d3a.
sdkrystian
added a commit
that referenced
this pull request
Nov 6, 2024
sdkrystian
added a commit
to sdkrystian/llvm-project
that referenced
this pull request
Nov 6, 2024
|
@alexfh @wlei-llvm Please send a repro when you can! |
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 reapplies #111173, fixing a bug when instantiating dependent expressions that name a member template that is later explicitly specialized for a class specialization that is implicitly instantiated.
The bug is addressed by adding the
hasMemberSpecializationfunction, which returntrueif any redeclaration is a member specialization. This is then used when determining the instantiation pattern for a specialization of a template, and when collecting template arguments for a specialization of a template.