Reland: [clang] fix P3310 overload resolution flag propagation #125791
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
Class templates might be only instantiated when they are required to be complete, but checking the template args against the primary template is immediate. This result is cached so that later when the class is instantiated, checking against the primary template is not repeated. The 'MatchedPackOnParmToNonPackOnArg' flag is also produced upon checking against the primary template, so it needs to be cached in the specialziation as well. This fixes a bug which has not been in any release, so there are no release notes. Fixes #125290
mizvekov
requested review from
DavidSpickett,
cor3ntin,
erichkeane and
Michael137
llvmbot
added
clang
|
@llvm/pr-subscribers-lldb @llvm/pr-subscribers-clang-modules Author: Matheus Izvekov (mizvekov) ChangesClass templates might be only instantiated when they are required to be complete, but checking the template args against the primary template is immediate. This result is cached so that later when the class is instantiated, checking against the primary template is not repeated. The 'MatchedPackOnParmToNonPackOnArg' flag is also produced upon checking against the primary template, so it needs to be cached in the specialziation as well. This fixes a bug which has not been in any release, so there are no release notes. Fixes #125290 Patch is 232.54 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/125791.diff 16 Files Affected:
diff --git a/clang/include/clang/AST/DeclTemplate.h b/clang/include/clang/AST/DeclTemplate.h
index 9ecff2c898acd5..03c43765206b18 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -1841,15 +1841,23 @@ class ClassTemplateSpecializationDecl : public CXXRecordDecl,
LLVM_PREFERRED_TYPE(TemplateSpecializationKind)
unsigned SpecializationKind : 3;
+ /// Indicate that we have matched a parameter pack with a non pack
+ /// argument, when the opposite match is also allowed (strict pack match).
+ /// This needs to be cached as deduction is performed during declaration,
+ /// and we need the information to be preserved so that it is consistent
+ /// during instantiation.
+ bool MatchedPackOnParmToNonPackOnArg : 1;
+
protected:
ClassTemplateSpecializationDecl(ASTContext &Context, Kind DK, TagKind TK,
DeclContext *DC, SourceLocation StartLoc,
SourceLocation IdLoc,
ClassTemplateDecl *SpecializedTemplate,
ArrayRef<TemplateArgument> Args,
+ bool MatchedPackOnParmToNonPackOnArg,
ClassTemplateSpecializationDecl *PrevDecl);
- explicit ClassTemplateSpecializationDecl(ASTContext &C, Kind DK);
+ ClassTemplateSpecializationDecl(ASTContext &C, Kind DK);
public:
friend class ASTDeclReader;
@@ -1859,7 +1867,7 @@ class ClassTemplateSpecializationDecl : public CXXRecordDecl,
Create(ASTContext &Context, TagKind TK, DeclContext *DC,
SourceLocation StartLoc, SourceLocation IdLoc,
ClassTemplateDecl *SpecializedTemplate,
- ArrayRef<TemplateArgument> Args,
+ ArrayRef<TemplateArgument> Args, bool MatchedPackOnParmToNonPackOnArg,
ClassTemplateSpecializationDecl *PrevDecl);
static ClassTemplateSpecializationDecl *CreateDeserialized(ASTContext &C,
GlobalDeclID ID);
@@ -1930,6 +1938,10 @@ class ClassTemplateSpecializationDecl : public CXXRecordDecl,
SpecializationKind = TSK;
}
+ bool hasMatchedPackOnParmToNonPackOnArg() const {
+ return MatchedPackOnParmToNonPackOnArg;
+ }
+
/// Get the point of instantiation (if any), or null if none.
SourceLocation getPointOfInstantiation() const {
return PointOfInstantiation;
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 472a0e25adc975..79bf6c04ee4969 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -13493,8 +13493,8 @@ class Sema final : public SemaBase {
bool InstantiateClassTemplateSpecialization(
SourceLocation PointOfInstantiation,
ClassTemplateSpecializationDecl *ClassTemplateSpec,
- TemplateSpecializationKind TSK, bool Complain = true,
- bool PrimaryHasMatchedPackOnParmToNonPackOnArg = false);
+ TemplateSpecializationKind TSK, bool Complain,
+ bool PrimaryHasMatchedPackOnParmToNonPackOnArg);
/// Instantiates the definitions of all of the member
/// of the given class, which is an instantiation of a class template
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index c9f2f905d2134c..1057f09deda073 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -6321,9 +6321,9 @@ ExpectedDecl ASTNodeImporter::VisitClassTemplateSpecializationDecl(
updateLookupTableForTemplateParameters(*ToTPList);
} else { // Not a partial specialization.
if (GetImportedOrCreateDecl(
- D2, D, Importer.getToContext(), D->getTagKind(), DC,
- *BeginLocOrErr, *IdLocOrErr, ClassTemplate, TemplateArgs,
- PrevDecl))
+ D2, D, Importer.getToContext(), D->getTagKind(), DC, *BeginLocOrErr,
+ *IdLocOrErr, ClassTemplate, TemplateArgs,
+ D->hasMatchedPackOnParmToNonPackOnArg(), PrevDecl))
return D2;
// Update InsertPos, because preceding import calls may have invalidated
diff --git a/clang/lib/AST/DeclTemplate.cpp b/clang/lib/AST/DeclTemplate.cpp
index 2e1ed9e10713a8..fe8734d262a961 100644
--- a/clang/lib/AST/DeclTemplate.cpp
+++ b/clang/lib/AST/DeclTemplate.cpp
@@ -957,18 +957,20 @@ FunctionTemplateSpecializationInfo *FunctionTemplateSpecializationInfo::Create(
// ClassTemplateSpecializationDecl Implementation
//===----------------------------------------------------------------------===//
-ClassTemplateSpecializationDecl::
-ClassTemplateSpecializationDecl(ASTContext &Context, Kind DK, TagKind TK,
- DeclContext *DC, SourceLocation StartLoc,
- SourceLocation IdLoc,
- ClassTemplateDecl *SpecializedTemplate,
- ArrayRef<TemplateArgument> Args,
- ClassTemplateSpecializationDecl *PrevDecl)
+ClassTemplateSpecializationDecl::ClassTemplateSpecializationDecl(
+ ASTContext &Context, Kind DK, TagKind TK, DeclContext *DC,
+ SourceLocation StartLoc, SourceLocation IdLoc,
+ ClassTemplateDecl *SpecializedTemplate, ArrayRef<TemplateArgument> Args,
+ bool MatchedPackOnParmToNonPackOnArg,
+ ClassTemplateSpecializationDecl *PrevDecl)
: CXXRecordDecl(DK, TK, Context, DC, StartLoc, IdLoc,
SpecializedTemplate->getIdentifier(), PrevDecl),
- SpecializedTemplate(SpecializedTemplate),
- TemplateArgs(TemplateArgumentList::CreateCopy(Context, Args)),
- SpecializationKind(TSK_Undeclared) {
+ SpecializedTemplate(SpecializedTemplate),
+ TemplateArgs(TemplateArgumentList::CreateCopy(Context, Args)),
+ SpecializationKind(TSK_Undeclared),
+ MatchedPackOnParmToNonPackOnArg(MatchedPackOnParmToNonPackOnArg) {
+ assert(DK == Kind::ClassTemplateSpecialization ||
+ MatchedPackOnParmToNonPackOnArg == false);
}
ClassTemplateSpecializationDecl::ClassTemplateSpecializationDecl(ASTContext &C,
@@ -977,18 +979,14 @@ ClassTemplateSpecializationDecl::ClassTemplateSpecializationDecl(ASTContext &C,
SourceLocation(), nullptr, nullptr),
SpecializationKind(TSK_Undeclared) {}
-ClassTemplateSpecializationDecl *
-ClassTemplateSpecializationDecl::Create(ASTContext &Context, TagKind TK,
- DeclContext *DC,
- SourceLocation StartLoc,
- SourceLocation IdLoc,
- ClassTemplateDecl *SpecializedTemplate,
- ArrayRef<TemplateArgument> Args,
- ClassTemplateSpecializationDecl *PrevDecl) {
- auto *Result =
- new (Context, DC) ClassTemplateSpecializationDecl(
- Context, ClassTemplateSpecialization, TK, DC, StartLoc, IdLoc,
- SpecializedTemplate, Args, PrevDecl);
+ClassTemplateSpecializationDecl *ClassTemplateSpecializationDecl::Create(
+ ASTContext &Context, TagKind TK, DeclContext *DC, SourceLocation StartLoc,
+ SourceLocation IdLoc, ClassTemplateDecl *SpecializedTemplate,
+ ArrayRef<TemplateArgument> Args, bool MatchedPackOnParmToNonPackOnArg,
+ ClassTemplateSpecializationDecl *PrevDecl) {
+ auto *Result = new (Context, DC) ClassTemplateSpecializationDecl(
+ Context, ClassTemplateSpecialization, TK, DC, StartLoc, IdLoc,
+ SpecializedTemplate, Args, MatchedPackOnParmToNonPackOnArg, PrevDecl);
Result->setMayHaveOutOfDateDef(false);
// If the template decl is incomplete, copy the external lexical storage from
@@ -1175,7 +1173,10 @@ ClassTemplatePartialSpecializationDecl::ClassTemplatePartialSpecializationDecl(
ClassTemplatePartialSpecializationDecl *PrevDecl)
: ClassTemplateSpecializationDecl(
Context, ClassTemplatePartialSpecialization, TK, DC, StartLoc, IdLoc,
- SpecializedTemplate, Args, PrevDecl),
+ // Tracking MatchedPackOnParmToNonPackOnArg for Partial
+ // Specializations is not needed.
+ SpecializedTemplate, Args, /*MatchedPackOnParmToNonPackOnArg=*/false,
+ PrevDecl),
TemplateParams(Params), InstantiatedFromMember(nullptr, false) {
if (AdoptTemplateParameterList(Params, this))
setInvalidDecl();
diff --git a/clang/lib/AST/JSONNodeDumper.cpp b/clang/lib/AST/JSONNodeDumper.cpp
index 36ef1fc8c79db0..4ab0e7cb5815b6 100644
--- a/clang/lib/AST/JSONNodeDumper.cpp
+++ b/clang/lib/AST/JSONNodeDumper.cpp
@@ -1003,6 +1003,11 @@ void JSONNodeDumper::VisitRecordDecl(const RecordDecl *RD) {
void JSONNodeDumper::VisitCXXRecordDecl(const CXXRecordDecl *RD) {
VisitRecordDecl(RD);
+ if (const auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(RD)) {
+ if (CTSD->hasMatchedPackOnParmToNonPackOnArg())
+ JOS.attribute("strict-pack-match", true);
+ }
+
// All other information requires a complete definition.
if (!RD->isCompleteDefinition())
return;
diff --git a/clang/lib/AST/TextNodeDumper.cpp b/clang/lib/AST/TextNodeDumper.cpp
index a57cba95974822..7f5a825b687983 100644
--- a/clang/lib/AST/TextNodeDumper.cpp
+++ b/clang/lib/AST/TextNodeDumper.cpp
@@ -2525,8 +2525,11 @@ void TextNodeDumper::VisitCXXRecordDecl(const CXXRecordDecl *D) {
OS << " instantiated_from";
dumpPointer(Instance);
}
- if (const auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(D))
+ if (const auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(D)) {
dumpTemplateSpecializationKind(CTSD->getSpecializationKind());
+ if (CTSD->hasMatchedPackOnParmToNonPackOnArg())
+ OS << " strict-pack-match";
+ }
dumpNestedNameSpecifier(D->getQualifier());
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 35ece88c603ddd..f180be2c1a5e15 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -3651,7 +3651,7 @@ QualType Sema::CheckTemplateIdType(TemplateName Name,
ClassTemplate->getDeclContext(),
ClassTemplate->getTemplatedDecl()->getBeginLoc(),
ClassTemplate->getLocation(), ClassTemplate, CTAI.CanonicalConverted,
- nullptr);
+ CTAI.MatchedPackOnParmToNonPackOnArg, nullptr);
ClassTemplate->AddSpecialization(Decl, InsertPos);
if (ClassTemplate->isOutOfLine())
Decl->setLexicalDeclContext(ClassTemplate->getLexicalDeclContext());
@@ -8526,7 +8526,8 @@ DeclResult Sema::ActOnClassTemplateSpecialization(
// this explicit specialization or friend declaration.
Specialization = ClassTemplateSpecializationDecl::Create(
Context, Kind, DC, KWLoc, TemplateNameLoc, ClassTemplate,
- CTAI.CanonicalConverted, PrevDecl);
+ CTAI.CanonicalConverted, CTAI.MatchedPackOnParmToNonPackOnArg,
+ PrevDecl);
Specialization->setTemplateArgsAsWritten(TemplateArgs);
SetNestedNameSpecifier(*this, Specialization, SS);
if (TemplateParameterLists.size() > 0) {
@@ -9869,7 +9870,8 @@ DeclResult Sema::ActOnExplicitInstantiation(
// this explicit specialization.
Specialization = ClassTemplateSpecializationDecl::Create(
Context, Kind, ClassTemplate->getDeclContext(), KWLoc, TemplateNameLoc,
- ClassTemplate, CTAI.CanonicalConverted, PrevDecl);
+ ClassTemplate, CTAI.CanonicalConverted,
+ CTAI.MatchedPackOnParmToNonPackOnArg, PrevDecl);
SetNestedNameSpecifier(*this, Specialization, SS);
// A MSInheritanceAttr attached to the previous declaration must be
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp
index 137942f0c30bfe..425c41f0f62367 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -3341,8 +3341,6 @@ FinishTemplateArgumentDeduction(
return ConstraintsNotSatisfied
? TemplateDeductionResult::ConstraintsNotSatisfied
: TemplateDeductionResult::SubstitutionFailure;
- if (InstCTAI.MatchedPackOnParmToNonPackOnArg)
- Info.setMatchedPackOnParmToNonPackOnArg();
TemplateParameterList *TemplateParams = Template->getTemplateParameters();
for (unsigned I = 0, E = TemplateParams->size(); I != E; ++I) {
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 4855e8a23689ce..31611695eba7e4 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4038,7 +4038,7 @@ TemplateDeclInstantiator::VisitClassTemplateSpecializationDecl(
ClassTemplateSpecializationDecl::Create(
SemaRef.Context, D->getTagKind(), Owner, D->getBeginLoc(),
D->getLocation(), InstClassTemplate, CTAI.CanonicalConverted,
- PrevDecl);
+ CTAI.MatchedPackOnParmToNonPackOnArg, PrevDecl);
InstD->setTemplateArgsAsWritten(InstTemplateArgs);
// Add this partial specialization to the set of class template partial
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 33d5378944ddbf..58c72e2ac8935c 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -9397,7 +9397,8 @@ bool Sema::RequireCompleteTypeImpl(SourceLocation Loc, QualType T,
runWithSufficientStackSpace(Loc, [&] {
Diagnosed = InstantiateClassTemplateSpecialization(
Loc, ClassTemplateSpec, TSK_ImplicitInstantiation,
- /*Complain=*/Diagnoser);
+ /*Complain=*/Diagnoser,
+ ClassTemplateSpec->hasMatchedPackOnParmToNonPackOnArg());
});
Instantiated = true;
}
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index 8210eb2143acf5..963122160ff4d0 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2532,6 +2532,7 @@ RedeclarableResult ASTDeclReader::VisitClassTemplateSpecializationDeclImpl(
D->TemplateArgs = TemplateArgumentList::CreateCopy(C, TemplArgs);
D->PointOfInstantiation = readSourceLocation();
D->SpecializationKind = (TemplateSpecializationKind)Record.readInt();
+ D->MatchedPackOnParmToNonPackOnArg = Record.readBool();
bool writtenAsCanonicalDecl = Record.readInt();
if (writtenAsCanonicalDecl) {
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp
index fa2294da95de8d..3505db441e829e 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1843,6 +1843,7 @@ void ASTDeclWriter::VisitClassTemplateSpecializationDecl(
Record.AddTemplateArgumentList(&D->getTemplateArgs());
Record.AddSourceLocation(D->getPointOfInstantiation());
Record.push_back(D->getSpecializationKind());
+ Record.push_back(D->hasMatchedPackOnParmToNonPackOnArg());
Record.push_back(D->isCanonicalDecl());
if (D->isCanonicalDecl()) {
diff --git a/clang/test/AST/ast-dump-templates.cpp b/clang/test/AST/ast-dump-templates.cpp
index 9fcafbcbcc46b6..86af8c50f3174e 100644
--- a/clang/test/AST/ast-dump-templates.cpp
+++ b/clang/test/AST/ast-dump-templates.cpp
@@ -1,7 +1,15 @@
-// RUN: %clang_cc1 -std=c++1z -ast-print %s > %t
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -std=c++17 -ast-dump=json %s | FileCheck --check-prefix=JSON %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -std=c++17 -ast-print %s > %t
// RUN: FileCheck < %t %s -check-prefix=CHECK1
// RUN: FileCheck < %t %s -check-prefix=CHECK2
-// RUN: %clang_cc1 -std=c++1z -ast-dump %s | FileCheck --check-prefix=DUMP %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -std=c++17 -ast-dump %s | FileCheck --check-prefix=DUMP %s
+
+// Test with serialization:
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -std=c++17 -emit-pch -o %t %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -x c++ -std=c++17 -include-pch %t \
+// RUN: -ast-dump-all /dev/null \
+// RUN: | sed -e "s/ <undeserialized declarations>//" -e "s/ imported//" \
+// RUN: | FileCheck --strict-whitespace --check-prefix=DUMP %s
template <int X, typename Y, int Z = 5>
struct foo {
@@ -118,3 +126,6038 @@ void func() {
// DUMP-NEXT: `-TemplateTypeParm {{.*}} 'Key'
}
}
+
+namespace test7 {
+ template <template<class> class TT> struct A {};
+ template <class...> class B {};
+ template struct A<B>;
+// DUMP-LABEL: NamespaceDecl {{.*}} test7{{$}}
+// DUMP: ClassTemplateSpecializationDecl {{.*}} struct A definition explicit_instantiation_definition strict-pack-match{{$}}
+} // namespce test7
+
+// NOTE: CHECK lines have been autogenerated by gen_ast_dump_json_test.py
+
+
+// JSON-NOT: {{^}}Dumping
+// JSON: "kind": "TranslationUnitDecl",
+// JSON-NEXT: "loc": {},
+// JSON-NEXT: "range": {
+// JSON-NEXT: "begin": {},
+// JSON-NEXT: "end": {}
+// JSON-NEXT: },
+// JSON-NEXT: "inner": [
+// JSON-NEXT: {
+// JSON-NEXT: "id": "0x{{.*}}",
+// JSON-NEXT: "kind": "TypedefDecl",
+// JSON-NEXT: "loc": {},
+// JSON-NEXT: "range": {
+// JSON-NEXT: "begin": {},
+// JSON-NEXT: "end": {}
+// JSON-NEXT: },
+// JSON-NEXT: "isImplicit": true,
+// JSON-NEXT: "name": "__int128_t",
+// JSON-NEXT: "type": {
+// JSON-NEXT: "qualType": "__int128"
+// JSON-NEXT: },
+// JSON-NEXT: "inner": [
+// JSON-NEXT: {
+// JSON-NEXT: "id": "0x{{.*}}",
+// JSON-NEXT: "kind": "BuiltinType",
+// JSON-NEXT: "type": {
+// JSON-NEXT: "qualType": "__int128"
+// JSON-NEXT: }
+// JSON-NEXT: }
+// JSON-NEXT: ]
+// JSON-NEXT: },
+// JSON-NEXT: {
+// JSON-NEXT: "id": "0x{{.*}}",
+// JSON-NEXT: "kind": "TypedefDecl",
+// JSON-NEXT: "loc": {},
+// JSON-NEXT: "range": {
+// JSON-NEXT: "begin": {},
+// JSON-NEXT: "end": {}
+// JSON-NEXT: },
+// JSON-NEXT: "isImplicit": true,
+// JSON-NEXT: "name": "__uint128_t",
+// JSON-NEXT: "type": {
+// JSON-NEXT: "qualType": "unsigned __int128"
+// JSON-NEXT: },
+// JSON-NEXT: "inner": [
+// JSON-NEXT: {
+// JSON-NEXT: "id": "0x{{.*}}",
+// JSON-NEXT: "kind": "BuiltinType",
+// JSON-NEXT: "type": {
+// JSON-NEXT: "qualType": "unsigned __int128"
+// JSON-NEXT: }
+// JSON-NEXT: }
+// JSON-NEXT: ]
+// JSON-NEXT: },
+// JSON-NEXT: {
+// JSON-NEXT: "id": "0x{{.*}}",
+// JSON-NEXT: "kind": "TypedefDecl",
+// JSON-NEXT: "loc": {},
+// JSON-NEXT: "range": {
+// JSON-NEXT: "begin": {},
+// JSON-NEXT: "end": {}
+// JSON-NEXT: },
+// JSON-NEXT: "isImplicit": true,
+// JSON-NEXT: "name": "__NSConstantString",
+// JSON-NEXT: "type": {
+// JSON-NEXT: "qualType": "__NSConstantString_tag"
+// JSON-NEXT: },
+// JSON-NEXT: "inner": [
+// JSON-NEXT: {
+// JSON-NEXT: "id": "0x{{.*}}",
+// JSON-NEXT: "kind": "RecordType",
+// JSON-NEXT: "type": {
+// JSON-NEXT: "qualType": "__NSConstantString_tag"
+// JSON-NEXT: },
+// JSON-NEXT: "decl": {
+// JSON-NEXT: "id": "0x{{.*}}",
+// JSON-NEXT: "kind": "CXXRecordDecl",
+// JSON-NEXT: "name": "__NSConstantString_tag"
+// JSON-NEXT: }
+// JSON-NEXT: }
+// JSON-NEXT: ]
+// JSON-NEXT: },
+// JSON-NEXT: {
+// JSON-NEXT: "id": "0x{{.*}}",
+// JSON-NEXT: "kind": "TypedefDecl",
+// JSON-NEXT: "loc": {},
+// JSON-NEXT: "range": {
+// JSON-NEXT: "begin": {},
+// JSON-NEXT: "end": {}
+// JSON-NEXT: },
+// JSON-NEXT: "isImplicit": true,
+// JSON-NEXT: "name": "__builtin_ms_va_list",
+// JSON-NEXT: "type": {
+// JSON-NEXT: "qualType": "char *"
+// JSON-NEXT: },
+// JSON-NEXT: "inner": [
+// JSON-NEXT: {
+// JSON-NEXT: "id": "0x{{.*}}",
+// JSON-NEXT: "kind": "PointerType",
+// JSON-NEXT: "type": {
+// JSON-NEXT: "qualType": "char *"
+// JSON-NEXT: },
+// JSON-NEXT: "inner": [
+// JSON-NEXT: {
+// JSON-NEXT: "id": "0x{{.*}}",
+// JSON-NEXT: "kind": "BuiltinType",
+// JSON-NEXT: "type": {
+// JSON-NEXT: "qualType": "char"
+// JSON-NEXT: }
+// JSON-NEXT: }
+// JSON-NEXT: ]
+// JSON-NEXT: }
+// JSON-NEXT: ]
+// JSON-NEXT: },
+// JSON-NEXT: {
+// JSON-NEXT: "id": "0x{{.*}}",
+// JSON-NEXT: "kind": "TypedefDecl",
+// JSON-NEXT: "loc": {},
+// JSON-NEXT: "range": {
+// JSON-NEXT: "begin": {},
+// JSON-NEXT: "end": {}
+// JSON-NEXT: },
+// JSON-NEXT: "isIm...
[truncated]
|
mizvekov
commented
Feb 5, 2025
erichkeane
approved these changes
Feb 5, 2025
mizvekov
deleted the
users/mizvekov/clang-fix-P3310-strict-match-incomplete-types
branch
|
Heads-up: this patch triggers a MSAN failure in lldb tests (e.g. |
|
The msan warning indicates that the So far, I've only observed this failure in lldb tests, which may suggests it might be an LLDB-related issue -- but I don't have enough context on LLDB to say for sure, cc @Michael137. |
Oh I guess that can happen if we created the decl using |
|
Actually, @mizvekov, can we make the |
On the ASTReader, we don't need a setter because it's friends with the class. I'd worry a little bit about leaving it always set to a fixed value, this could be a hard to track bug in the future for the lldb folks, even if it's probably not observable right now. |
Yea that's fair. Would you prefer us adding a setter for it then? And set it to a fixed value from LLDB (until we have a better way to deal with it)? |
Sure, a setter is fine for me. I am not sure from what data you are populating the decl, but if it's like the ast import case, you can forward the flag from the original decl. If it's being formed from matching as-written template arguments, you can forward the result from CheckTemplateArgument. Otherwise if it's fully synthetic, I guess you can leave a comment explaining it. |
Yea we're creating it from debug-info. Haven't thought about whether we can infer this field from debug-info, but either way, I'll go the setter approach then, thanks! |
I see. It looks like you would need to call CheckTemplateArgument with the arguments you get from debug info anyway, otherwise some things wouldn't work, like pack parameters. You can use the result from this call to get the value for the flag. Otherwise if you don't currently check the arguments, you could leave a FIXME. |
Michael137
added a commit
to Michael137/llvm-project
that referenced
this pull request
Feb 7, 2025
…StrictPackMatch field This addresses the MSAN failure reported in llvm#125791 (comment): ``` ==5633==WARNING: MemorySanitizer: use-of-uninitialized-value #0 in clang::ASTNodeImporter::CallOverloadedCreateFun<clang::ClassTemplateSpecializationDecl>::operator() llvm#1 in bool clang::ASTNodeImporter::GetImportedOrCreateSpecialDecl<...> ... ``` The ASTImporter reads `D->hasStrictPackMatch()` and forwards it to the constructor of the destination `ClassTemplateSpecializationDecl`. But if `D` is a decl that LLDB created from debug-info, it would've been created using `ClassTemplateSpecializationDecl::CreateDeserialized`, which doesn't initialize the `StrictPackMatch` field. This patch just initializes the field to a fixed value of `false`, to preserve previous behaviour and avoid the use-of-uninitialized-value. An alternative would be to always initialize it in the `ClassTemplateSpecializationDecl` constructor, but there were reservations about providing a default value for it because it might lead to hard-to-diagnose problems down the line.
Michael137
added a commit
that referenced
this pull request
Feb 7, 2025
…StrictPackMatch field (#126215) This addresses the MSAN failure reported in #125791 (comment): ``` ==5633==WARNING: MemorySanitizer: use-of-uninitialized-value #0 in clang::ASTNodeImporter::CallOverloadedCreateFun<clang::ClassTemplateSpecializationDecl>::operator() #1 in bool clang::ASTNodeImporter::GetImportedOrCreateSpecialDecl<...> ... ``` The ASTImporter reads `D->hasStrictPackMatch()` and forwards it to the constructor of the destination `ClassTemplateSpecializationDecl`. But if `D` is a decl that LLDB created from debug-info, it would've been created using `ClassTemplateSpecializationDecl::CreateDeserialized`, which doesn't initialize the `StrictPackMatch` field. This patch just initializes the field to a fixed value of `false`, to preserve previous behaviour and avoid the use-of-uninitialized-value. An alternative would be to always initialize it in the `ClassTemplateSpecializationDecl` constructor, but there were reservations about providing a default value for it because it might lead to hard-to-diagnose problems down the line.
github-actions bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Feb 7, 2025
…tionDecl's StrictPackMatch field (#126215) This addresses the MSAN failure reported in llvm/llvm-project#125791 (comment): ``` ==5633==WARNING: MemorySanitizer: use-of-uninitialized-value #0 in clang::ASTNodeImporter::CallOverloadedCreateFun<clang::ClassTemplateSpecializationDecl>::operator() #1 in bool clang::ASTNodeImporter::GetImportedOrCreateSpecialDecl<...> ... ``` The ASTImporter reads `D->hasStrictPackMatch()` and forwards it to the constructor of the destination `ClassTemplateSpecializationDecl`. But if `D` is a decl that LLDB created from debug-info, it would've been created using `ClassTemplateSpecializationDecl::CreateDeserialized`, which doesn't initialize the `StrictPackMatch` field. This patch just initializes the field to a fixed value of `false`, to preserve previous behaviour and avoid the use-of-uninitialized-value. An alternative would be to always initialize it in the `ClassTemplateSpecializationDecl` constructor, but there were reservations about providing a default value for it because it might lead to hard-to-diagnose problems down the line.
Icohedron
pushed a commit
to Icohedron/llvm-project
that referenced
this pull request
Feb 11, 2025
…125791) Class templates might be only instantiated when they are required to be complete, but checking the template args against the primary template is immediate. This result is cached so that later when the class is instantiated, checking against the primary template is not repeated. The 'MatchedPackOnParmToNonPackOnArg' flag is also produced upon checking against the primary template, so it needs to be cached in the specialziation as well. This fixes a bug which has not been in any release, so there are no release notes. Fixes llvm#125290
Icohedron
pushed a commit
to Icohedron/llvm-project
that referenced
this pull request
Feb 11, 2025
…StrictPackMatch field (llvm#126215) This addresses the MSAN failure reported in llvm#125791 (comment): ``` ==5633==WARNING: MemorySanitizer: use-of-uninitialized-value #0 in clang::ASTNodeImporter::CallOverloadedCreateFun<clang::ClassTemplateSpecializationDecl>::operator() llvm#1 in bool clang::ASTNodeImporter::GetImportedOrCreateSpecialDecl<...> ... ``` The ASTImporter reads `D->hasStrictPackMatch()` and forwards it to the constructor of the destination `ClassTemplateSpecializationDecl`. But if `D` is a decl that LLDB created from debug-info, it would've been created using `ClassTemplateSpecializationDecl::CreateDeserialized`, which doesn't initialize the `StrictPackMatch` field. This patch just initializes the field to a fixed value of `false`, to preserve previous behaviour and avoid the use-of-uninitialized-value. An alternative would be to always initialize it in the `ClassTemplateSpecializationDecl` constructor, but there were reservations about providing a default value for it because it might lead to hard-to-diagnose problems down the line.
|
/cherry-pick 08bda1c |
|
/pull-request #127779 |
mizvekov
added a commit
that referenced
this pull request
Feb 19, 2025
…5791) Class templates might be only instantiated when they are required to be complete, but checking the template args against the primary template is immediate. This result is cached so that later when the class is instantiated, checking against the primary template is not repeated. The 'MatchedPackOnParmToNonPackOnArg' flag is also produced upon checking against the primary template, so it needs to be cached in the specialziation as well. This fixes a bug which has not been in any release, so there are no release notes. Fixes #125290
swift-ci
pushed a commit
to swiftlang/llvm-project
that referenced
this pull request
Feb 20, 2025
…m#125791) Class templates might be only instantiated when they are required to be complete, but checking the template args against the primary template is immediate. This result is cached so that later when the class is instantiated, checking against the primary template is not repeated. The 'MatchedPackOnParmToNonPackOnArg' flag is also produced upon checking against the primary template, so it needs to be cached in the specialziation as well. This fixes a bug which has not been in any release, so there are no release notes. Fixes llvm#125290
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.
Class templates might be only instantiated when they are required to be complete, but checking the template args against the primary template is immediate.
This result is cached so that later when the class is instantiated, checking against the primary template is not repeated.
The 'MatchedPackOnParmToNonPackOnArg' flag is also produced upon checking against the primary template, so it needs to be cached in the specialziation as well.
This fixes a bug which has not been in any release, so there are no release notes.
Fixes #125290