Skip to content

[clang] diagnose invalid std::tuple_size sizes #159677

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.

Sign up for GitHub

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 1 commit into from
Sep 19, 2025
Merged

[clang] diagnose invalid std::tuple_size sizes #159677

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 4 additions & 2 deletions clang/docs/ReleaseNotes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,8 @@ Improvements to Clang's diagnostics
Moved the warning for a missing (though implied) attribute on a redeclaration into this group.
Added a new warning in this group for the case where the attribute is missing/implicit on
an override of a virtual method.
- Implemented diagnostics when retrieving the tuple size for types where its specialization of `std::tuple_size`
produces an invalid size (either negative or greater than the implementation limit). (#GH159563)
- Fixed fix-it hint for fold expressions. Clang now correctly places the suggested right
parenthesis when diagnosing malformed fold expressions. (#GH151787)
- Added fix-it hint for when scoped enumerations require explicit conversions for binary operations. (#GH24265)
Expand Down Expand Up @@ -356,8 +358,8 @@ Bug Fixes in This Version
and vector of 4 ``float`` values. (#GH155405)
- Fixed inconsistent shadow warnings for lambda capture of structured bindings.
Previously, ``[val = val]`` (regular parameter) produced no warnings with ``-Wshadow``
while ``[a = a]`` (where ``a`` is from ``auto [a, b] = std::make_pair(1, 2)``)
incorrectly produced warnings. Both cases now consistently show no warnings with
while ``[a = a]`` (where ``a`` is from ``auto [a, b] = std::make_pair(1, 2)``)
incorrectly produced warnings. Both cases now consistently show no warnings with
``-Wshadow`` and show uncaptured-local warnings with ``-Wshadow-all``. (#GH68605)
- Fixed a failed assertion with a negative limit parameter value inside of
``__has_embed``. (#GH157842)
Expand Down
3 changes: 3 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,9 @@ def err_decomp_decl_std_tuple_element_not_specialized : Error<
def err_decomp_decl_std_tuple_size_not_constant : Error<
"cannot decompose this type; 'std::tuple_size<%0>::value' "
"is not a valid integral constant expression">;
def err_decomp_decl_std_tuple_size_invalid
: Error<"cannot decompose this type; 'std::tuple_size<%0>::value' "
"is not a valid size: %1">;
def note_in_binding_decl_init : Note<
"in implicit initialization of binding declaration %0">;
def err_arg_is_not_destructurable : Error<
Expand Down
25 changes: 19 additions & 6 deletions clang/lib/Sema/SemaDeclCXX.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1177,7 +1177,7 @@ getTrivialTypeTemplateArgument(Sema &S, SourceLocation Loc, QualType T) {
namespace { enum class IsTupleLike { TupleLike, NotTupleLike, Error }; }

static IsTupleLike isTupleLike(Sema &S, SourceLocation Loc, QualType T,
llvm::APSInt &Size) {
unsigned &OutSize) {
EnterExpressionEvaluationContext ContextRAII(
S, Sema::ExpressionEvaluationContext::ConstantEvaluated);

Expand Down Expand Up @@ -1218,10 +1218,24 @@ static IsTupleLike isTupleLike(Sema &S, SourceLocation Loc, QualType T,
if (E.isInvalid())
return IsTupleLike::Error;

llvm::APSInt Size;
E = S.VerifyIntegerConstantExpression(E.get(), &Size, Diagnoser);
if (E.isInvalid())
return IsTupleLike::Error;

// The implementation limit is UINT_MAX-1, to allow this to be passed down on
// an UnsignedOrNone.
if (Size < 0 || Size >= UINT_MAX) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this UINT_MAX but the calling code uses Ctx.getSizeType()?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test doesn't seem all that right. I think we should be checking if (Size.isNegative() || !Size.isSignedIntN(Ctx.getTypeSize(Ctx.getSizeType())) or something like that like that ?

(should that just be isIntN?).

host-compiler UINT_MAX is almost definitely the wrong thing here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The host compiler uintmax is the right thing, because we pass this down on an UnsignedOrNone, which uses unsigned as the representation, so the implementation limit is uintmax-1.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh! sure, yeah, that is reasonable.

llvm::SmallVector<char, 16> Str;
Size.toString(Str);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't format the value and it's lower than UINT_MAX anyway, just calling getExtValue() should work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure what you mean, here the value can be arbitrarily larger than UINT_MAX, so calling getExtValue can truncate them.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the separately formatting? ADT/APInt.h already ahs an operator <<, so we should be able to just << Size.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't a raw_ostream though. We usually pass toString(Size, 10) or similar.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, hrmph... It seems to me that StreamingDiagnostic should have something that defers to raw_ostream but I see we don't have one. That is unfortunate.

S.Diag(Loc, diag::err_decomp_decl_std_tuple_size_invalid)
<< printTemplateArgs(S.Context.getPrintingPolicy(), Args,
/*Params=*/nullptr)
<< StringRef(Str.data(), Str.size());
return IsTupleLike::Error;
}

OutSize = Size.getExtValue();
return IsTupleLike::TupleLike;
}

Expand Down Expand Up @@ -1279,9 +1293,8 @@ struct InitializingBinding {
static bool checkTupleLikeDecomposition(Sema &S,
ArrayRef<BindingDecl *> Bindings,
VarDecl *Src, QualType DecompType,
const llvm::APSInt &TupleSize) {
unsigned NumElems) {
auto *DD = cast<DecompositionDecl>(Src);
unsigned NumElems = (unsigned)TupleSize.getLimitedValue(UINT_MAX);
if (CheckBindingsCount(S, DD, DecompType, Bindings, NumElems))
return true;

Expand Down Expand Up @@ -1641,7 +1654,7 @@ void Sema::CheckCompleteDecompositionDeclaration(DecompositionDecl *DD) {
// C++1z [dcl.decomp]/3:
// if the expression std::tuple_size<E>::value is a well-formed integral
// constant expression, [...]
llvm::APSInt TupleSize(32);
unsigned TupleSize;
switch (isTupleLike(*this, DD->getLocation(), DecompType, TupleSize)) {
case IsTupleLike::Error:
DD->setInvalidDecl();
Expand Down Expand Up @@ -1690,12 +1703,12 @@ UnsignedOrNone Sema::GetDecompositionElementCount(QualType T,
if (T->getAs<ComplexType>())
return 2u;

llvm::APSInt TupleSize(Ctx.getTypeSize(Ctx.getSizeType()));
unsigned TupleSize;
switch (isTupleLike(*this, Loc, T, TupleSize)) {
case IsTupleLike::Error:
return std::nullopt;
case IsTupleLike::TupleLike:
return static_cast<unsigned>(TupleSize.getExtValue());
return TupleSize;
case IsTupleLike::NotTupleLike:
break;
}
Expand Down
20 changes: 18 additions & 2 deletions clang/test/SemaCXX/builtin-structured-binding-size.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// RUN: %clang_cc1 %s -std=c++2c -fsyntax-only -verify
// RUN: %clang_cc1 %s -std=c++2c -fsyntax-only -verify -fexperimental-new-constant-interpreter
// RUN: %clang_cc1 %s -triple=x86_64 -std=c++2c -fsyntax-only -verify
// RUN: %clang_cc1 %s -triple=x86_64 -std=c++2c -fsyntax-only -verify -fexperimental-new-constant-interpreter


struct S0 {};
Expand Down Expand Up @@ -229,3 +229,19 @@ static_assert(__is_same_as(tag_of_t<S1>, int));
static_assert(__is_same_as(tag_of_t<int>, int)); // error
// expected-error@-1 {{constraints not satisfied for alias template 'tag_of_t' [with T = int]}}
// expected-note@#tag-of-constr {{because substituted constraint expression is ill-formed: type 'int' cannot be decomposed}}

struct MinusOne;
template <> struct ::std::tuple_size<MinusOne> {
static constexpr int value = -1;
};
int minus_one = __builtin_structured_binding_size(MinusOne);
// expected-error@-1 {{cannot decompose this type; 'std::tuple_size<MinusOne>::value' is not a valid size: -1}}
// expected-error@-2 {{type 'MinusOne' cannot be decomposed}}

struct UintMax;
template <> struct ::std::tuple_size<UintMax> {
static constexpr unsigned value = -1;
};
int uint_max = __builtin_structured_binding_size(UintMax);
// expected-error@-1 {{cannot decompose this type; 'std::tuple_size<UintMax>::value' is not a valid size: 4294967295}}
// expected-error@-2 {{type 'UintMax' cannot be decomposed}}
Loading