Skip to content

Commit 9a6f0c2

Browse files
Erich Keanetstellar
Erich Keane
authored and
tstellar
committed
Revert two patches to fix GH58452 regression
GH58452 is a regression in the 16.0 release branch caused by both: b8a1b69 and 3a0309c This patch reverts both of those to make the 'valid' code stop diagnosing at the expense of crashes on invalid + unclear diagnostics. This patch also adds the tests from GH58452 to prevent any re-application from breaking this again. Revert "[clang] Improve diagnostics for expansion length mismatch" This reverts commit 3a0309c. Revert "[clang] fix missing initialization of original number of expansions" This reverts commit b8a1b69. Differential Revision: https://reviews.llvm.org/D145605 (cherry picked from commit acecf68)
1 parent 633c6c0 commit 9a6f0c2

File tree

8 files changed

+158
-176
lines changed

8 files changed

+158
-176
lines changed

‎clang/include/clang/Sema/Sema.h

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -238,11 +238,9 @@ namespace threadSafety {
238238

239239
// FIXME: No way to easily map from TemplateTypeParmTypes to
240240
// TemplateTypeParmDecls, so we have this horrible PointerUnion.
241-
using UnexpandedParameterPack = std::pair<
242-
llvm::PointerUnion<
243-
const TemplateTypeParmType *, const SubstTemplateTypeParmPackType *,
244-
const SubstNonTypeTemplateParmPackExpr *, const NamedDecl *>,
245-
SourceLocation>;
241+
typedef std::pair<llvm::PointerUnion<const TemplateTypeParmType *, NamedDecl *>,
242+
SourceLocation>
243+
UnexpandedParameterPack;
246244

247245
/// Describes whether we've seen any nullability information for the given
248246
/// file.

‎clang/include/clang/Sema/SemaInternal.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ inline InheritableAttr *getDLLAttr(Decl *D) {
6262
}
6363

6464
/// Retrieve the depth and index of a template parameter.
65-
inline std::pair<unsigned, unsigned> getDepthAndIndex(const NamedDecl *ND) {
65+
inline std::pair<unsigned, unsigned> getDepthAndIndex(NamedDecl *ND) {
6666
if (const auto *TTP = dyn_cast<TemplateTypeParmDecl>(ND))
6767
return std::make_pair(TTP->getDepth(), TTP->getIndex());
6868

@@ -79,7 +79,7 @@ getDepthAndIndex(UnexpandedParameterPack UPP) {
7979
if (const auto *TTP = UPP.first.dyn_cast<const TemplateTypeParmType *>())
8080
return std::make_pair(TTP->getDepth(), TTP->getIndex());
8181

82-
return getDepthAndIndex(UPP.first.get<const NamedDecl *>());
82+
return getDepthAndIndex(UPP.first.get<NamedDecl *>());
8383
}
8484

8585
class TypoCorrectionConsumer : public VisibleDeclConsumer {

‎clang/lib/Sema/SemaTemplateDeduction.cpp

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -756,11 +756,8 @@ class PackDeductionScope {
756756
SmallVector<UnexpandedParameterPack, 2> Unexpanded;
757757
S.collectUnexpandedParameterPacks(Pattern, Unexpanded);
758758
for (unsigned I = 0, N = Unexpanded.size(); I != N; ++I) {
759-
UnexpandedParameterPack U = Unexpanded[I];
760-
if (U.first.is<const SubstTemplateTypeParmPackType *>() ||
761-
U.first.is<const SubstNonTypeTemplateParmPackExpr *>())
762-
continue;
763-
auto [Depth, Index] = getDepthAndIndex(U);
759+
unsigned Depth, Index;
760+
std::tie(Depth, Index) = getDepthAndIndex(Unexpanded[I]);
764761
if (Depth == Info.getDeducedDepth())
765762
AddPack(Index);
766763
}

‎clang/lib/Sema/SemaTemplateVariadic.cpp

Lines changed: 114 additions & 120 deletions
Original file line numberDiff line numberDiff line change
@@ -89,23 +89,6 @@ namespace {
8989
return true;
9090
}
9191

92-
bool
93-
VisitSubstTemplateTypeParmPackTypeLoc(SubstTemplateTypeParmPackTypeLoc TL) {
94-
Unexpanded.push_back({TL.getTypePtr(), TL.getNameLoc()});
95-
return true;
96-
}
97-
98-
bool VisitSubstTemplateTypeParmPackType(SubstTemplateTypeParmPackType *T) {
99-
Unexpanded.push_back({T, SourceLocation()});
100-
return true;
101-
}
102-
103-
bool
104-
VisitSubstNonTypeTemplateParmPackExpr(SubstNonTypeTemplateParmPackExpr *E) {
105-
Unexpanded.push_back({E, E->getParameterPackLocation()});
106-
return true;
107-
}
108-
10992
/// Record occurrences of function and non-type template
11093
/// parameter packs in an expression.
11194
bool VisitDeclRefExpr(DeclRefExpr *E) {
@@ -324,8 +307,7 @@ Sema::DiagnoseUnexpandedParameterPacks(SourceLocation Loc,
324307
auto *TTPD = dyn_cast<TemplateTypeParmDecl>(LocalPack);
325308
return TTPD && TTPD->getTypeForDecl() == TTPT;
326309
}
327-
return declaresSameEntity(Pack.first.get<const NamedDecl *>(),
328-
LocalPack);
310+
return declaresSameEntity(Pack.first.get<NamedDecl *>(), LocalPack);
329311
};
330312
if (llvm::any_of(LSI->LocalPacks, DeclaresThisPack))
331313
LambdaParamPackReferences.push_back(Pack);
@@ -377,7 +359,7 @@ Sema::DiagnoseUnexpandedParameterPacks(SourceLocation Loc,
377359
= Unexpanded[I].first.dyn_cast<const TemplateTypeParmType *>())
378360
Name = TTP->getIdentifier();
379361
else
380-
Name = Unexpanded[I].first.get<const NamedDecl *>()->getIdentifier();
362+
Name = Unexpanded[I].first.get<NamedDecl *>()->getIdentifier();
381363

382364
if (Name && NamesKnown.insert(Name).second)
383365
Names.push_back(Name);
@@ -440,7 +422,7 @@ bool Sema::DiagnoseUnexpandedParameterPackInRequiresExpr(RequiresExpr *RE) {
440422
llvm::SmallPtrSet<NamedDecl*, 8> ParmSet(Parms.begin(), Parms.end());
441423
SmallVector<UnexpandedParameterPack, 2> UnexpandedParms;
442424
for (auto Parm : Unexpanded)
443-
if (ParmSet.contains(Parm.first.dyn_cast<const NamedDecl *>()))
425+
if (ParmSet.contains(Parm.first.dyn_cast<NamedDecl *>()))
444426
UnexpandedParms.push_back(Parm);
445427
if (UnexpandedParms.empty())
446428
return false;
@@ -692,95 +674,109 @@ bool Sema::CheckParameterPacksForExpansion(
692674
bool &RetainExpansion, std::optional<unsigned> &NumExpansions) {
693675
ShouldExpand = true;
694676
RetainExpansion = false;
695-
std::pair<const IdentifierInfo *, SourceLocation> FirstPack;
696-
std::optional<std::pair<unsigned, SourceLocation>> PartialExpansion;
697-
std::optional<unsigned> CurNumExpansions;
677+
std::pair<IdentifierInfo *, SourceLocation> FirstPack;
678+
bool HaveFirstPack = false;
679+
std::optional<unsigned> NumPartialExpansions;
680+
SourceLocation PartiallySubstitutedPackLoc;
698681

699-
for (auto [P, Loc] : Unexpanded) {
682+
for (UnexpandedParameterPack ParmPack : Unexpanded) {
700683
// Compute the depth and index for this parameter pack.
701-
std::optional<std::pair<unsigned, unsigned>> Pos;
684+
unsigned Depth = 0, Index = 0;
685+
IdentifierInfo *Name;
686+
bool IsVarDeclPack = false;
687+
688+
if (const TemplateTypeParmType *TTP =
689+
ParmPack.first.dyn_cast<const TemplateTypeParmType *>()) {
690+
Depth = TTP->getDepth();
691+
Index = TTP->getIndex();
692+
Name = TTP->getIdentifier();
693+
} else {
694+
NamedDecl *ND = ParmPack.first.get<NamedDecl *>();
695+
if (isa<VarDecl>(ND))
696+
IsVarDeclPack = true;
697+
else
698+
std::tie(Depth, Index) = getDepthAndIndex(ND);
699+
700+
Name = ND->getIdentifier();
701+
}
702+
703+
// Determine the size of this argument pack.
702704
unsigned NewPackSize;
703-
const auto *ND = P.dyn_cast<const NamedDecl *>();
704-
if (ND && isa<VarDecl>(ND)) {
705-
const auto *DAP =
706-
CurrentInstantiationScope->findInstantiationOf(ND)
707-
->dyn_cast<LocalInstantiationScope::DeclArgumentPack *>();
708-
if (!DAP) {
705+
if (IsVarDeclPack) {
706+
// Figure out whether we're instantiating to an argument pack or not.
707+
typedef LocalInstantiationScope::DeclArgumentPack DeclArgumentPack;
708+
709+
llvm::PointerUnion<Decl *, DeclArgumentPack *> *Instantiation =
710+
CurrentInstantiationScope->findInstantiationOf(
711+
ParmPack.first.get<NamedDecl *>());
712+
if (Instantiation->is<DeclArgumentPack *>()) {
713+
// We could expand this function parameter pack.
714+
NewPackSize = Instantiation->get<DeclArgumentPack *>()->size();
715+
} else {
709716
// We can't expand this function parameter pack, so we can't expand
710717
// the pack expansion.
711718
ShouldExpand = false;
712719
continue;
713720
}
714-
NewPackSize = DAP->size();
715-
} else if (ND) {
716-
Pos = getDepthAndIndex(ND);
717-
} else if (const auto *TTP = P.dyn_cast<const TemplateTypeParmType *>()) {
718-
Pos = {TTP->getDepth(), TTP->getIndex()};
719-
ND = TTP->getDecl();
720-
// FIXME: We either should have some fallback for canonical TTP, or
721-
// never have canonical TTP here.
722-
} else if (const auto *STP =
723-
P.dyn_cast<const SubstTemplateTypeParmPackType *>()) {
724-
NewPackSize = STP->getNumArgs();
725-
ND = STP->getReplacedParameter();
726721
} else {
727-
const auto *SEP = P.get<const SubstNonTypeTemplateParmPackExpr *>();
728-
NewPackSize = SEP->getArgumentPack().pack_size();
729-
ND = SEP->getParameterPack();
730-
}
731-
732-
if (Pos) {
733722
// If we don't have a template argument at this depth/index, then we
734723
// cannot expand the pack expansion. Make a note of this, but we still
735724
// want to check any parameter packs we *do* have arguments for.
736-
if (Pos->first >= TemplateArgs.getNumLevels() ||
737-
!TemplateArgs.hasTemplateArgument(Pos->first, Pos->second)) {
725+
if (Depth >= TemplateArgs.getNumLevels() ||
726+
!TemplateArgs.hasTemplateArgument(Depth, Index)) {
738727
ShouldExpand = false;
739728
continue;
740729
}
730+
741731
// Determine the size of the argument pack.
742-
NewPackSize = TemplateArgs(Pos->first, Pos->second).pack_size();
743-
// C++0x [temp.arg.explicit]p9:
744-
// Template argument deduction can extend the sequence of template
745-
// arguments corresponding to a template parameter pack, even when the
746-
// sequence contains explicitly specified template arguments.
747-
if (CurrentInstantiationScope)
748-
if (const NamedDecl *PartialPack =
749-
CurrentInstantiationScope->getPartiallySubstitutedPack();
750-
PartialPack && getDepthAndIndex(PartialPack) == *Pos) {
732+
NewPackSize = TemplateArgs(Depth, Index).pack_size();
733+
}
734+
735+
// C++0x [temp.arg.explicit]p9:
736+
// Template argument deduction can extend the sequence of template
737+
// arguments corresponding to a template parameter pack, even when the
738+
// sequence contains explicitly specified template arguments.
739+
if (!IsVarDeclPack && CurrentInstantiationScope) {
740+
if (NamedDecl *PartialPack =
741+
CurrentInstantiationScope->getPartiallySubstitutedPack()) {
742+
unsigned PartialDepth, PartialIndex;
743+
std::tie(PartialDepth, PartialIndex) = getDepthAndIndex(PartialPack);
744+
if (PartialDepth == Depth && PartialIndex == Index) {
751745
RetainExpansion = true;
752746
// We don't actually know the new pack size yet.
753-
PartialExpansion = {NewPackSize, Loc};
747+
NumPartialExpansions = NewPackSize;
748+
PartiallySubstitutedPackLoc = ParmPack.second;
754749
continue;
755750
}
751+
}
756752
}
757753

758-
// FIXME: Workaround for Canonical TTP.
759-
const IdentifierInfo *Name = ND ? ND->getIdentifier() : nullptr;
760-
if (!CurNumExpansions) {
754+
if (!NumExpansions) {
761755
// The is the first pack we've seen for which we have an argument.
762756
// Record it.
763-
CurNumExpansions = NewPackSize;
764-
FirstPack = {Name, Loc};
765-
} else if (NewPackSize != *CurNumExpansions) {
757+
NumExpansions = NewPackSize;
758+
FirstPack.first = Name;
759+
FirstPack.second = ParmPack.second;
760+
HaveFirstPack = true;
761+
continue;
762+
}
763+
764+
if (NewPackSize != *NumExpansions) {
766765
// C++0x [temp.variadic]p5:
767766
// All of the parameter packs expanded by a pack expansion shall have
768767
// the same number of arguments specified.
769-
Diag(EllipsisLoc, diag::err_pack_expansion_length_conflict)
770-
<< FirstPack.first << Name << *CurNumExpansions << NewPackSize
771-
<< SourceRange(FirstPack.second) << SourceRange(Loc);
768+
if (HaveFirstPack)
769+
Diag(EllipsisLoc, diag::err_pack_expansion_length_conflict)
770+
<< FirstPack.first << Name << *NumExpansions << NewPackSize
771+
<< SourceRange(FirstPack.second) << SourceRange(ParmPack.second);
772+
else
773+
Diag(EllipsisLoc, diag::err_pack_expansion_length_conflict_multilevel)
774+
<< Name << *NumExpansions << NewPackSize
775+
<< SourceRange(ParmPack.second);
772776
return true;
773777
}
774778
}
775779

776-
if (NumExpansions && CurNumExpansions &&
777-
*NumExpansions != *CurNumExpansions) {
778-
Diag(EllipsisLoc, diag::err_pack_expansion_length_conflict_multilevel)
779-
<< FirstPack.first << *CurNumExpansions << *NumExpansions
780-
<< SourceRange(FirstPack.second);
781-
return true;
782-
}
783-
784780
// If we're performing a partial expansion but we also have a full expansion,
785781
// expand to the number of common arguments. For example, given:
786782
//
@@ -790,18 +786,17 @@ bool Sema::CheckParameterPacksForExpansion(
790786
//
791787
// ... a call to 'A<int, int>().f<int>' should expand the pack once and
792788
// retain an expansion.
793-
if (PartialExpansion) {
794-
if (CurNumExpansions && *CurNumExpansions < PartialExpansion->first) {
789+
if (NumPartialExpansions) {
790+
if (NumExpansions && *NumExpansions < *NumPartialExpansions) {
795791
NamedDecl *PartialPack =
796792
CurrentInstantiationScope->getPartiallySubstitutedPack();
797793
Diag(EllipsisLoc, diag::err_pack_expansion_length_conflict_partial)
798-
<< PartialPack << PartialExpansion->first << *CurNumExpansions
799-
<< SourceRange(PartialExpansion->second);
794+
<< PartialPack << *NumPartialExpansions << *NumExpansions
795+
<< SourceRange(PartiallySubstitutedPackLoc);
800796
return true;
801797
}
802-
NumExpansions = PartialExpansion->first;
803-
} else {
804-
NumExpansions = CurNumExpansions;
798+
799+
NumExpansions = NumPartialExpansions;
805800
}
806801

807802
return false;
@@ -814,48 +809,47 @@ std::optional<unsigned> Sema::getNumArgumentsInExpansion(
814809
CollectUnexpandedParameterPacksVisitor(Unexpanded).TraverseType(Pattern);
815810

816811
std::optional<unsigned> Result;
817-
auto setResultSz = [&Result](unsigned Size) {
818-
assert((!Result || *Result == Size) && "inconsistent pack sizes");
819-
Result = Size;
820-
};
821-
auto setResultPos = [&](const std::pair<unsigned, unsigned> &Pos) -> bool {
822-
unsigned Depth = Pos.first, Index = Pos.second;
823-
if (Depth >= TemplateArgs.getNumLevels() ||
824-
!TemplateArgs.hasTemplateArgument(Depth, Index))
825-
// The pattern refers to an unknown template argument. We're not ready to
826-
// expand this pack yet.
827-
return true;
828-
// Determine the size of the argument pack.
829-
setResultSz(TemplateArgs(Depth, Index).pack_size());
830-
return false;
831-
};
812+
for (unsigned I = 0, N = Unexpanded.size(); I != N; ++I) {
813+
// Compute the depth and index for this parameter pack.
814+
unsigned Depth;
815+
unsigned Index;
832816

833-
for (auto [I, _] : Unexpanded) {
834-
if (const auto *TTP = I.dyn_cast<const TemplateTypeParmType *>()) {
835-
if (setResultPos({TTP->getDepth(), TTP->getIndex()}))
836-
return std::nullopt;
837-
} else if (const auto *STP =
838-
I.dyn_cast<const SubstTemplateTypeParmPackType *>()) {
839-
setResultSz(STP->getNumArgs());
840-
} else if (const auto *SEP =
841-
I.dyn_cast<const SubstNonTypeTemplateParmPackExpr *>()) {
842-
setResultSz(SEP->getArgumentPack().pack_size());
817+
if (const TemplateTypeParmType *TTP =
818+
Unexpanded[I].first.dyn_cast<const TemplateTypeParmType *>()) {
819+
Depth = TTP->getDepth();
820+
Index = TTP->getIndex();
843821
} else {
844-
const auto *ND = I.get<const NamedDecl *>();
845-
// Function parameter pack or init-capture pack.
822+
NamedDecl *ND = Unexpanded[I].first.get<NamedDecl *>();
846823
if (isa<VarDecl>(ND)) {
847-
const auto *DAP =
848-
CurrentInstantiationScope->findInstantiationOf(ND)
849-
->dyn_cast<LocalInstantiationScope::DeclArgumentPack *>();
850-
if (!DAP)
824+
// Function parameter pack or init-capture pack.
825+
typedef LocalInstantiationScope::DeclArgumentPack DeclArgumentPack;
826+
827+
llvm::PointerUnion<Decl *, DeclArgumentPack *> *Instantiation =
828+
CurrentInstantiationScope->findInstantiationOf(
829+
Unexpanded[I].first.get<NamedDecl *>());
830+
if (Instantiation->is<Decl *>())
851831
// The pattern refers to an unexpanded pack. We're not ready to expand
852832
// this pack yet.
853833
return std::nullopt;
854-
setResultSz(DAP->size());
855-
} else if (setResultPos(getDepthAndIndex(ND))) {
856-
return std::nullopt;
834+
835+
unsigned Size = Instantiation->get<DeclArgumentPack *>()->size();
836+
assert((!Result || *Result == Size) && "inconsistent pack sizes");
837+
Result = Size;
838+
continue;
857839
}
840+
841+
std::tie(Depth, Index) = getDepthAndIndex(ND);
858842
}
843+
if (Depth >= TemplateArgs.getNumLevels() ||
844+
!TemplateArgs.hasTemplateArgument(Depth, Index))
845+
// The pattern refers to an unknown template argument. We're not ready to
846+
// expand this pack yet.
847+
return std::nullopt;
848+
849+
// Determine the size of the argument pack.
850+
unsigned Size = TemplateArgs(Depth, Index).pack_size();
851+
assert((!Result || *Result == Size) && "inconsistent pack sizes");
852+
Result = Size;
859853
}
860854

861855
return Result;

‎clang/lib/Sema/TreeTransform.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5897,7 +5897,6 @@ bool TreeTransform<Derived>::TransformFunctionTypeParams(
58975897
= dyn_cast<PackExpansionType>(OldType)) {
58985898
// We have a function parameter pack that may need to be expanded.
58995899
QualType Pattern = Expansion->getPattern();
5900-
NumExpansions = Expansion->getNumExpansions();
59015900
SmallVector<UnexpandedParameterPack, 2> Unexpanded;
59025901
getSema().collectUnexpandedParameterPacks(Pattern, Unexpanded);
59035902

0 commit comments

Comments
 (0)