Skip to content

[Clang][Sema] Handle invalid variable template specialization whose type depends on itself #134522

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
May 7, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Promote isSameTemplateArg, and use llvm::APSInt::isSameValue
  • Loading branch information
zwuis committed Apr 30, 2025
commit 781d3c64f1ff47df3e4e7e3af78302814cf82979
5 changes: 3 additions & 2 deletions clang/lib/AST/ASTContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7648,7 +7648,7 @@ bool ASTContext::isSameTemplateArgument(const TemplateArgument &Arg1,

switch (Arg1.getKind()) {
case TemplateArgument::Null:
return true;
llvm_unreachable("Comparing NULL template argument");

case TemplateArgument::Type:
return hasSameType(Arg1.getAsType(), Arg2.getAsType());
Expand All @@ -7666,7 +7666,8 @@ bool ASTContext::isSameTemplateArgument(const TemplateArgument &Arg1,
getCanonicalTemplateName(Arg2.getAsTemplateOrTemplatePattern());

case TemplateArgument::Integral:
return Arg1.getAsIntegral() == Arg2.getAsIntegral();
return llvm::APSInt::isSameValue(Arg1.getAsIntegral(),
Arg2.getAsIntegral());

case TemplateArgument::StructuralValue:
return Arg1.structurallyEquals(Arg2);
Expand Down
65 changes: 3 additions & 62 deletions clang/lib/Sema/SemaTemplateDeduction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -114,27 +114,6 @@ namespace clang {
using namespace clang;
using namespace sema;

/// Compare two APSInts, extending and switching the sign as
/// necessary to compare their values regardless of underlying type.
static bool hasSameExtendedValue(llvm::APSInt X, llvm::APSInt Y) {
if (Y.getBitWidth() > X.getBitWidth())
X = X.extend(Y.getBitWidth());
else if (Y.getBitWidth() < X.getBitWidth())
Y = Y.extend(X.getBitWidth());

// If there is a signedness mismatch, correct it.
if (X.isSigned() != Y.isSigned()) {
// If the signed value is negative, then the values cannot be the same.
if ((Y.isSigned() && Y.isNegative()) || (X.isSigned() && X.isNegative()))
return false;

Y.setIsSigned(true);
X.setIsSigned(true);
}

return X == Y;
}

/// The kind of PartialOrdering we're performing template argument deduction
/// for (C++11 [temp.deduct.partial]).
enum class PartialOrderingKind { None, NonCall, Call };
Expand Down Expand Up @@ -273,7 +252,7 @@ checkDeducedTemplateArguments(ASTContext &Context,
if (Y.getKind() == TemplateArgument::Expression ||
Y.getKind() == TemplateArgument::Declaration ||
(Y.getKind() == TemplateArgument::Integral &&
hasSameExtendedValue(X.getAsIntegral(), Y.getAsIntegral())))
llvm::APSInt::isSameValue(X.getAsIntegral(), Y.getAsIntegral())))
return X.wasDeducedFromArrayBound() ? Y : X;

// All other combinations are incompatible.
Expand Down Expand Up @@ -2574,7 +2553,7 @@ DeduceTemplateArguments(Sema &S, TemplateParameterList *TemplateParams,

case TemplateArgument::Integral:
if (A.getKind() == TemplateArgument::Integral) {
if (hasSameExtendedValue(P.getAsIntegral(), A.getAsIntegral()))
if (llvm::APSInt::isSameValue(P.getAsIntegral(), A.getAsIntegral()))
return TemplateDeductionResult::Success;
}
Info.FirstArg = P;
Expand Down Expand Up @@ -2828,44 +2807,6 @@ TemplateDeductionResult Sema::DeduceTemplateArguments(
/*HasDeducedAnyParam=*/nullptr);
}

/// Determine whether two template arguments are the same.
static bool isSameTemplateArg(ASTContext &Context, const TemplateArgument &X,
const TemplateArgument &Y) {
if (X.getKind() != Y.getKind())
return false;

switch (X.getKind()) {
case TemplateArgument::Null:
llvm_unreachable("Comparing NULL template argument");

case TemplateArgument::Type:
case TemplateArgument::Declaration:
case TemplateArgument::NullPtr:
case TemplateArgument::Template:
case TemplateArgument::TemplateExpansion:
case TemplateArgument::StructuralValue:
case TemplateArgument::Expression:
return Context.isSameTemplateArgument(X, Y);

case TemplateArgument::Integral:
return hasSameExtendedValue(X.getAsIntegral(), Y.getAsIntegral());

case TemplateArgument::Pack: {
unsigned PackIterationSize = X.pack_size();
if (X.pack_size() != Y.pack_size())
return false;
ArrayRef<TemplateArgument> XP = X.pack_elements();
ArrayRef<TemplateArgument> YP = Y.pack_elements();
for (unsigned i = 0; i < PackIterationSize; ++i)
if (!isSameTemplateArg(Context, XP[i], YP[i]))
return false;
return true;
}
}

llvm_unreachable("Invalid TemplateArgument Kind!");
}

TemplateArgumentLoc
Sema::getTrivialTemplateArgumentLoc(const TemplateArgument &Arg,
QualType NTTPType, SourceLocation Loc,
Expand Down Expand Up @@ -3331,7 +3272,7 @@ static TemplateDeductionResult FinishTemplateArgumentDeduction(
break;
TemplateArgument PP = P.isPackExpansion() ? P.getPackExpansionPattern() : P,
PA = A.isPackExpansion() ? A.getPackExpansionPattern() : A;
if (!isSameTemplateArg(S.Context, PP, PA)) {
if (!S.Context.isSameTemplateArgument(PP, PA)) {
if (!P.isPackExpansion() && !A.isPackExpansion()) {
Info.Param = makeTemplateParameter(TPL->getParam(
(AsStack.empty() ? As.end() : AsStack.back().begin()) -
Expand Down