[Clang] Introduce a trait to determine the structure binding size #131515
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
bindings that would be produced by ```cpp auto [...p] = expr; ``` This is necessary to implement P2300 (https://eel.is/c++draft/exec#snd.concepts-5), but can also be used to implement a general get<N> function that supports aggregates __builtin_structured_binding_size works like sizeof in that it supports both type and expression arguments. If the argument cannot be destructured, a sfinae-friendly error is produced. A type is considered a valid tuple if `std::tuple_size_v<T>` is a valid expression, even if there is no valid `std::tuple_element` specialization or suitable `get` function for that type. This is modeled as a UnaryExprOrTypeTraitExpr, but it is wrapped in a ConstantExpr because the structured binding size can only be established during sema.
cor3ntin
added
enhancement
|
@llvm/pr-subscribers-clang-static-analyzer-1 @llvm/pr-subscribers-clang Author: cor3ntin (cor3ntin) Changesbindings that would be produced by auto [...p] = expr;
This is necessary to implement P2300 (https://eel.is/c++draft/exec#snd.concepts-5), but can also be used to implement a general get<N> function that supports aggregates __builtin_structured_binding_size works like sizeof in that it supports both type and expression arguments. If the argument cannot be destructured, a sfinae-friendly error is produced. A type is considered a valid tuple if This is modeled as a UnaryExprOrTypeTraitExpr, but it is wrapped in a ConstantExpr because the structured binding size can only be established during sema. Patch is 27.05 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/131515.diff 14 Files Affected:
diff --git a/clang/docs/LanguageExtensions.rst b/clang/docs/LanguageExtensions.rst
index cc12ff5bad353..9a5cd8f1e5f5d 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -434,6 +434,36 @@ __datasizeof
``__datasizeof`` behaves like ``sizeof``, except that it returns the size of the
type ignoring tail padding.
+.. _builtin_structured_binding_size-doc:
+
+__builtin_structured_binding_size (C++)
+---------------------------------------
+``__builtin_structured_binding_size`` returns the *structured binding size*
+([dcl.struct.bind]) of the type ``T`` (or unevaluate expression ``arg``)
+passed as argument.
+
+This is equivalent to the size of the pack ``p`` in ``auto&& [...p] = arg;``.
+If the argument is not destructurable (ie not an array, vector, complex,
+*tuple-like* type or destructurable class type), ``__builtin_structured_binding_size(T)``
+is not a valid expression (``__builtin_structured_binding_size`` is SFINEA-friendly).
+
+A type is considered a valid tuple if ``std::tuple_size_v<T>`` is a valid expression,
+even if there is no valid ``std::tuple_element`` specialization or suitable
+``get`` function for that type.
+
+.. code-block:: c++
+
+ template<std::size_t Idx, typename T>
+ requires (Idx < __builtin_structured_binding_size(T))
+ decltype(auto) constexpr get_binding(T&& obj) {
+ auto && [...p] = std::forward<T>(obj);
+ return p...[Idx];
+ }
+ struct S { int a = 0, b = 42; };
+ static_assert(__builtin_structured_binding_size(S) == 2);
+ static_assert(get_binding<1>(S{}) == 42);
+
+
_BitInt, _ExtInt
----------------
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 2a1c5ee2d788e..f49e389773e4e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -74,6 +74,9 @@ What's New in Clang |release|?
C++ Language Changes
--------------------
+- Added a :ref:`__builtin_structured_binding_size <builtin_structured_binding_size-doc>` (T)
+ builtin that returns the number of structured bindings that would be produced by destructuring ``T``.
+
C++2c Feature Support
^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 86c9c955c1c78..fad826c1c6336 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -591,6 +591,8 @@ def err_decomp_decl_std_tuple_size_not_constant : Error<
"is not a valid integral constant expression">;
def note_in_binding_decl_init : Note<
"in implicit initialization of binding declaration %0">;
+def err_arg_is_not_destructurable : Error<
+ "type %0 is not destructurable">;
def err_std_type_trait_not_class_template : Error<
"unsupported standard library implementation: "
diff --git a/clang/include/clang/Basic/TokenKinds.def b/clang/include/clang/Basic/TokenKinds.def
index 397a5d95709fb..bad9387673ef9 100644
--- a/clang/include/clang/Basic/TokenKinds.def
+++ b/clang/include/clang/Basic/TokenKinds.def
@@ -553,8 +553,8 @@ TYPE_TRAIT_2(__reference_converts_from_temporary, ReferenceConvertsFromTemporary
// IsDeducible is only used internally by clang for CTAD implementation and
// is not exposed to users.
TYPE_TRAIT_2(/*EmptySpellingName*/, IsDeducible, KEYCXX)
-
TYPE_TRAIT_1(__is_bitwise_cloneable, IsBitwiseCloneable, KEYALL)
+UNARY_EXPR_OR_TYPE_TRAIT(__builtin_structured_binding_size, StructuredBindingSize, KEYCXX)
// Embarcadero Expression Traits
EXPRESSION_TRAIT(__is_lvalue_expr, IsLValueExpr, KEYCXX)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 657350fa843b9..cd7078b119712 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -6095,7 +6095,8 @@ class Sema final : public SemaBase {
RecordDecl *ClassDecl,
const IdentifierInfo *Name);
- unsigned GetDecompositionElementCount(QualType DecompType);
+ std::optional<unsigned int> GetDecompositionElementCount(QualType DecompType,
+ SourceLocation Loc);
void CheckCompleteDecompositionDeclaration(DecompositionDecl *DD);
/// Stack containing information needed when in C++2a an 'auto' is encountered
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index b9f88230007b5..0259605086b21 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -2154,6 +2154,8 @@ bool Compiler<Emitter>::VisitUnaryExprOrTypeTraitExpr(
E->getArgumentType()),
E);
}
+ assert(Kind != UETT_StructuredBindingSize &&
+ "should have been evaluated in Sema");
return false;
}
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index f8e8aaddbfdbd..1763bbc18043d 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -14878,6 +14878,11 @@ bool IntExprEvaluator::VisitUnaryExprOrTypeTraitExpr(
}
return Success(Sizeof, E);
}
+ case UETT_StructuredBindingSize:
+ // This can only be computed from Sema and has been cached.
+ // We can still get there from code that strips the outer ConstantExpr.
+ return false;
+
case UETT_OpenMPRequiredSimdAlign:
assert(E->isArgumentType());
return Success(
diff --git a/clang/lib/AST/ItaniumMangle.cpp b/clang/lib/AST/ItaniumMangle.cpp
index b6ba36784f38a..12993d5cb35f1 100644
--- a/clang/lib/AST/ItaniumMangle.cpp
+++ b/clang/lib/AST/ItaniumMangle.cpp
@@ -5389,6 +5389,15 @@ void CXXNameMangler::mangleExpression(const Expr *E, unsigned Arity,
Diags.Report(DiagID);
return;
}
+ case UETT_StructuredBindingSize:
+ Out << "u11__builtin_structured_binding_size";
+ if (SAE->isArgumentType())
+ mangleType(SAE->getArgumentType());
+ else
+ mangleTemplateArgExpr(SAE->getArgumentExpr());
+ Out << 'E';
+ break;
+ return;
}
break;
}
diff --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp
index 0c28972d6ed8f..2e3f0ce3194f5 100644
--- a/clang/lib/Parse/ParseExpr.cpp
+++ b/clang/lib/Parse/ParseExpr.cpp
@@ -1544,6 +1544,7 @@ ExprResult Parser::ParseCastExpression(CastParseKind ParseKind,
// unary-expression: '__builtin_omp_required_simd_align' '(' type-name ')'
case tok::kw___builtin_omp_required_simd_align:
case tok::kw___builtin_vectorelements:
+ case tok::kw___builtin_structured_binding_size:
if (NotPrimaryExpression)
*NotPrimaryExpression = true;
AllowSuffix = false;
@@ -2463,7 +2464,8 @@ Parser::ParseExprAfterUnaryExprOrTypeTrait(const Token &OpTok,
tok::kw___datasizeof, tok::kw___alignof, tok::kw_alignof,
tok::kw__Alignof, tok::kw_vec_step,
tok::kw___builtin_omp_required_simd_align,
- tok::kw___builtin_vectorelements) &&
+ tok::kw___builtin_vectorelements,
+ tok::kw___builtin_structured_binding_size) &&
"Not a typeof/sizeof/alignof/vec_step expression!");
ExprResult Operand;
@@ -2473,7 +2475,8 @@ Parser::ParseExprAfterUnaryExprOrTypeTrait(const Token &OpTok,
// If construct allows a form without parenthesis, user may forget to put
// pathenthesis around type name.
if (OpTok.isOneOf(tok::kw_sizeof, tok::kw___datasizeof, tok::kw___alignof,
- tok::kw_alignof, tok::kw__Alignof)) {
+ tok::kw_alignof, tok::kw__Alignof,
+ tok::kw___builtin_structured_binding_size)) {
if (isTypeIdUnambiguously()) {
DeclSpec DS(AttrFactory);
ParseSpecifierQualifierList(DS);
@@ -2599,7 +2602,8 @@ ExprResult Parser::ParseUnaryExprOrTypeTraitExpression() {
assert(Tok.isOneOf(tok::kw_sizeof, tok::kw___datasizeof, tok::kw___alignof,
tok::kw_alignof, tok::kw__Alignof, tok::kw_vec_step,
tok::kw___builtin_omp_required_simd_align,
- tok::kw___builtin_vectorelements) &&
+ tok::kw___builtin_vectorelements,
+ tok::kw___builtin_structured_binding_size) &&
"Not a sizeof/alignof/vec_step expression!");
Token OpTok = Tok;
ConsumeToken();
@@ -2687,6 +2691,9 @@ ExprResult Parser::ParseUnaryExprOrTypeTraitExpression() {
case tok::kw___datasizeof:
ExprKind = UETT_DataSizeOf;
break;
+ case tok::kw___builtin_structured_binding_size:
+ ExprKind = UETT_StructuredBindingSize;
+ break;
case tok::kw___builtin_vectorelements:
ExprKind = UETT_VectorElements;
break;
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index a02bd8335fa20..164e81e1cfa61 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -1475,6 +1475,48 @@ static DeclAccessPair findDecomposableBaseClass(Sema &S, SourceLocation Loc,
return DeclAccessPair::make(const_cast<CXXRecordDecl*>(ClassWithFields), AS);
}
+static bool CheckMemberDecompositionFields(Sema &S, SourceLocation Loc,
+ const CXXRecordDecl *OrigRD,
+ QualType DecompType,
+ DeclAccessPair BasePair) {
+ const CXXRecordDecl *RD = cast_or_null<CXXRecordDecl>(BasePair.getDecl());
+ if (!RD)
+ return true;
+
+ for (auto *FD : RD->fields()) {
+ if (FD->isUnnamedBitField())
+ continue;
+
+ // All the non-static data members are required to be nameable, so they
+ // must all have names.
+ if (!FD->getDeclName()) {
+ if (RD->isLambda()) {
+ S.Diag(Loc, diag::err_decomp_decl_lambda);
+ S.Diag(RD->getLocation(), diag::note_lambda_decl);
+ return true;
+ }
+
+ if (FD->isAnonymousStructOrUnion()) {
+ S.Diag(Loc, diag::err_decomp_decl_anon_union_member)
+ << DecompType << FD->getType()->isUnionType();
+ S.Diag(FD->getLocation(), diag::note_declared_at);
+ return true;
+ }
+
+ // FIXME: Are there any other ways we could have an anonymous member?
+ }
+ // The field must be accessible in the context of the structured binding.
+ // We already checked that the base class is accessible.
+ // FIXME: Add 'const' to AccessedEntity's classes so we can remove the
+ // const_cast here.
+ S.CheckStructuredBindingMemberAccess(
+ Loc, const_cast<CXXRecordDecl *>(OrigRD),
+ DeclAccessPair::make(FD, CXXRecordDecl::MergeAccess(
+ BasePair.getAccess(), FD->getAccess())));
+ }
+ return false;
+}
+
static bool checkMemberDecomposition(Sema &S, ArrayRef<BindingDecl*> Bindings,
ValueDecl *Src, QualType DecompType,
const CXXRecordDecl *OrigRD) {
@@ -1503,43 +1545,20 @@ static bool checkMemberDecomposition(Sema &S, ArrayRef<BindingDecl*> Bindings,
auto FlatBindings = DD->flat_bindings();
assert(llvm::range_size(FlatBindings) == NumFields);
auto FlatBindingsItr = FlatBindings.begin();
+
+ if (CheckMemberDecompositionFields(S, Src->getLocation(), OrigRD, DecompType,
+ BasePair))
+ return true;
+
for (auto *FD : RD->fields()) {
if (FD->isUnnamedBitField())
continue;
- // All the non-static data members are required to be nameable, so they
- // must all have names.
- if (!FD->getDeclName()) {
- if (RD->isLambda()) {
- S.Diag(Src->getLocation(), diag::err_decomp_decl_lambda);
- S.Diag(RD->getLocation(), diag::note_lambda_decl);
- return true;
- }
-
- if (FD->isAnonymousStructOrUnion()) {
- S.Diag(Src->getLocation(), diag::err_decomp_decl_anon_union_member)
- << DecompType << FD->getType()->isUnionType();
- S.Diag(FD->getLocation(), diag::note_declared_at);
- return true;
- }
-
- // FIXME: Are there any other ways we could have an anonymous member?
- }
-
// We have a real field to bind.
assert(FlatBindingsItr != FlatBindings.end());
BindingDecl *B = *(FlatBindingsItr++);
SourceLocation Loc = B->getLocation();
- // The field must be accessible in the context of the structured binding.
- // We already checked that the base class is accessible.
- // FIXME: Add 'const' to AccessedEntity's classes so we can remove the
- // const_cast here.
- S.CheckStructuredBindingMemberAccess(
- Loc, const_cast<CXXRecordDecl *>(OrigRD),
- DeclAccessPair::make(FD, CXXRecordDecl::MergeAccess(
- BasePair.getAccess(), FD->getAccess())));
-
// Initialize the binding to Src.FD.
ExprResult E = S.BuildDeclRefExpr(Src, DecompType, VK_LValue, Loc);
if (E.isInvalid())
@@ -1642,6 +1661,50 @@ void Sema::CheckCompleteDecompositionDeclaration(DecompositionDecl *DD) {
DD->setInvalidDecl();
}
+std::optional<unsigned> Sema::GetDecompositionElementCount(QualType T,
+ SourceLocation Loc) {
+ const ASTContext &Ctx = getASTContext();
+ assert(!T->isDependentType());
+ if (auto *CAT = Ctx.getAsConstantArrayType(T))
+ return CAT->getSize().getZExtValue();
+ if (auto *VT = T->getAs<VectorType>())
+ return VT->getNumElements();
+ if (T->getAs<ComplexType>())
+ return 2;
+
+ llvm::APSInt TupleSize(Ctx.getTypeSize(Ctx.getSizeType()));
+ switch (isTupleLike(*this, Loc, T, TupleSize)) {
+ case IsTupleLike::Error:
+ return {};
+ case IsTupleLike::TupleLike:
+ return TupleSize.getExtValue();
+ case IsTupleLike::NotTupleLike:
+ break;
+ }
+ CXXRecordDecl *OrigRD = T->getAsCXXRecordDecl();
+ if (!OrigRD || OrigRD->isUnion()) {
+ return std::nullopt;
+ }
+
+ if (RequireCompleteType(Loc, T, diag::err_incomplete_type))
+ return std::nullopt;
+
+ CXXCastPath BasePath;
+ DeclAccessPair BasePair =
+ findDecomposableBaseClass(*this, Loc, OrigRD, BasePath);
+ const CXXRecordDecl *RD = cast_or_null<CXXRecordDecl>(BasePair.getDecl());
+ if (!RD)
+ return std::nullopt;
+
+ unsigned NumFields = llvm::count_if(
+ RD->fields(), [](FieldDecl *FD) { return !FD->isUnnamedBitField(); });
+
+ if (CheckMemberDecompositionFields(*this, Loc, OrigRD, T, BasePair))
+ return true;
+
+ return NumFields;
+}
+
void Sema::MergeVarDeclExceptionSpecs(VarDecl *New, VarDecl *Old) {
// Shortcut if exceptions are disabled.
if (!getLangOpts().CXXExceptions)
@@ -17262,8 +17325,8 @@ void Sema::DiagnoseStaticAssertDetails(const Expr *E) {
Expr::EvalResult Result;
SmallString<12> ValueString;
bool Print;
- } DiagSide[2] = {{LHS, Expr::EvalResult(), {}, false},
- {RHS, Expr::EvalResult(), {}, false}};
+ } DiagSide[2] = {{Op->getLHS(), Expr::EvalResult(), {}, false},
+ {Op->getRHS(), Expr::EvalResult(), {}, false}};
for (unsigned I = 0; I < 2; I++) {
const Expr *Side = DiagSide[I].Cond;
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index e19136b394800..8766331a0df59 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -4160,6 +4160,54 @@ static bool CheckVecStepTraitOperandType(Sema &S, QualType T,
return false;
}
+static ExprResult BuildStructuredBindingSizeTraitImpl(Sema &S, QualType T,
+ Expr *E,
+ TypeSourceInfo *TInfo,
+ SourceLocation Loc,
+ SourceRange ArgRange) {
+ assert(!!E != !!TInfo);
+ assert(!T->isDependentType());
+ std::optional<unsigned> Size =
+ S.GetDecompositionElementCount(T, ArgRange.getBegin());
+ if (!Size) {
+ return S.Diag(Loc, diag::err_arg_is_not_destructurable) << T << ArgRange;
+ return ExprError();
+ }
+ Expr *Inner;
+ if (E)
+ Inner = new (S.getASTContext()) UnaryExprOrTypeTraitExpr(
+ UnaryExprOrTypeTrait::UETT_StructuredBindingSize, E,
+ S.getASTContext().getSizeType(), Loc, E->getEndLoc());
+
+ else
+ Inner = new (S.getASTContext()) UnaryExprOrTypeTraitExpr(
+ UnaryExprOrTypeTrait::UETT_StructuredBindingSize, TInfo,
+ S.getASTContext().getSizeType(), Loc, ArgRange.getEnd());
+
+ // Computing the number of bindings requires Sema and is non-trivial,
+ // so we cache the result now.
+ llvm::APSInt V =
+ S.getASTContext().MakeIntValue(*Size, S.getASTContext().getSizeType());
+ return ConstantExpr::Create(S.getASTContext(), Inner, APValue{V});
+}
+
+static ExprResult BuildStructuredBindingSizeTrait(Sema &S,
+ TypeSourceInfo *TInfo,
+ SourceLocation Loc,
+ SourceRange ArgRange) {
+ return BuildStructuredBindingSizeTraitImpl(S, TInfo->getType(),
+ /*Expr=*/nullptr, TInfo, Loc,
+ ArgRange);
+}
+
+static ExprResult BuildStructuredBindingSizeTrait(Sema &S, SourceLocation OpLoc,
+ Expr *E) {
+
+ return BuildStructuredBindingSizeTraitImpl(S, E->getType(), E,
+ /*TInfo=*/nullptr, OpLoc,
+ E->getSourceRange());
+}
+
static bool CheckVectorElementsTraitOperandType(Sema &S, QualType T,
SourceLocation Loc,
SourceRange ArgRange) {
@@ -4650,10 +4698,14 @@ ExprResult Sema::CreateUnaryExprOrTypeTraitExpr(TypeSourceInfo *TInfo,
QualType T = TInfo->getType();
- if (!T->isDependentType() &&
- CheckUnaryExprOrTypeTraitOperand(T, OpLoc, R, ExprKind,
- getTraitSpelling(ExprKind)))
- return ExprError();
+ if (!T->isDependentType()) {
+ if (ExprKind == UETT_StructuredBindingSize)
+ return BuildStructuredBindingSizeTrait(*this, TInfo, OpLoc, R);
+
+ if (CheckUnaryExprOrTypeTraitOperand(T, OpLoc, R, ExprKind,
+ getTraitSpelling(ExprKind)))
+ return ExprError();
+ }
// Adds overload of TransformToPotentiallyEvaluated for TypeSourceInfo to
// properly deal with VLAs in nested calls of sizeof and typeof.
@@ -4680,14 +4732,17 @@ Sema::CreateUnaryExprOrTypeTraitExpr(Expr *E, SourceLocation OpLoc,
bool isInvalid = false;
if (E->isTypeDependent()) {
// Delay type-checking for type-dependent expressions.
+ } else if (ExprKind == UETT_StructuredBindingSize) {
+ // Custom logic
+ return BuildStructuredBindingSizeTrait(*this, OpLoc, E);
} else if (ExprKind == UETT_AlignOf || ExprKind == UETT_PreferredAlignOf) {
isInvalid = CheckAlignOfExpr(*this, E, ExprKind);
} else if (ExprKind == UETT_VecStep) {
isInvalid = CheckVecStepExpr(E);
} else if (ExprKind == UETT_OpenMPRequiredSimdAlign) {
- Diag(E->getExprLoc(), diag::err_openmp_default_simd_align_expr);
- isInvalid = true;
- } else if (E->refersToBitField()) { // C99 6.5.3.4p1.
+ Diag(E->getExprLoc(), diag::err_openmp_default_simd_align_expr);
+ isInvalid = true;
+ } else if (E->refersToBitField()) { // C99 6.5.3.4p1.
Diag(E->getExprLoc(), diag::err_sizeof_alignof_typeof_bitfield) << 0;
isInvalid = true;
} else if (ExprKind == UETT_VectorElements) {
diff --git a/clang/test/CodeGenCXX/builtins.cpp b/clang/test/CodeGenCXX/builtins.cpp
index 37f9491d12d04..9169f3a3276d3 100644
--- a/clang/test/CodeGenCXX/builtins.cpp
+++ b/clang/test/CodeGenCXX/builtins.cpp
@@ -77,3 +77,9 @@ int constexpr_overflow_result() {
// CHECK: [[RET_VAL:%.+]] = load i32, ptr [[Z]]
// CHECK: ret i32 [[RET_VAL]]
}
+
+int structured_binding_size() {
+ struct S2 {int a, b;};
+ return __builtin_structured_binding_size(S2);
+ // CHECK: ret i32 2
+}
diff --git a/clang/test/CodeGenCXX/mangle-structured-binding-size.cpp b/clang/test/CodeGenCXX/mangle-structured-binding-size.cpp
new file mode 100644
index 0000000000000..5b53ed8d7166d
--- /dev/null
+++ b/clang/test/CodeGenCXX/mangle-structured-binding-si...
[truncated]
|
|
Please look at the doc/tests and confirm this would be suitable to implement |
cor3ntin
changed the title
ericniebler
reviewed
Mar 16, 2025
halbi2
reviewed
Mar 17, 2025
zyn0217
reviewed
Mar 17, 2025
| requires (__builtin_structured_binding_size(T) >=2) | ||
| consteval auto tag_of_impl(T& t) { | ||
| auto && [tag, ..._] = t; | ||
| return type_identity<decltype(auto(tag))>{}; |
There was a problem hiding this comment.
Can we test __builtin_structured_binding_size(decltype(t))?
There was a problem hiding this comment.
in the function body :)
like, static_assert(__builtin_structured_binding_size(decltype(t)) == ...)
llvmbot
added
clang:modules
erichkeane
reviewed
Mar 17, 2025
erichkeane
approved these changes
Mar 18, 2025
erichkeane
approved these changes
Mar 18, 2025
shafik
reviewed
Mar 18, 2025
clang/docs/LanguageExtensions.rst
Outdated
| the *structured binding size* ([dcl.struct.bind]) of type ``T`` | ||
|
|
||
| This is equivalent to the size of the pack ``p`` in ``auto&& [...p] = declval<T&>();``. | ||
| If the argument cannot be decomposed (ie not a builtin array, builtin SIMD vector, |
There was a problem hiding this comment.
I feel a bullet list would be better here. Also a positive form would read better, the following can be decomposed, otherwise it is not well-formed something like that.
It took me a few tries to make sense of this.
Comment on lines
+1669
to
+1672
| Qualifiers Quals; | ||
| QualType Unqual = Context.getUnqualifiedArrayType(T, Quals); | ||
| Quals.removeCVRQualifiers(); | ||
| T = Context.getQualifiedType(Unqual, Quals); |
There was a problem hiding this comment.
Why do we need to do this dance? (Should the code be moved down to above the isTupleLike call?)
There was a problem hiding this comment.
Why do we need to do this dance
const objects of class types are decomposable
Should the code be moved down to above the isTupleLike call?
No, for tuple-like types we already handle their constness
clang/lib/Sema/SemaExprCXX.cpp
Outdated
| @@ -5066,6 +5066,10 @@ static bool CheckUnaryTypeTraitTypeCompleteness(Sema &S, TypeTrait UTT, | |||
| case UTT_IsInterfaceClass: | |||
| return true; | |||
|
|
|||
| // We diagnose incomplete class types later | |||
There was a problem hiding this comment.
Suggested change
| // We diagnose incomplete class types later | |
| // We diagnose incomplete class types later. |
| struct S0 {}; | ||
| struct S1 {int a;}; | ||
| struct S2 {int a; int b;}; | ||
| struct S3 {double a; int b; int c;}; |
There was a problem hiding this comment.
Test with bit-fields? Test with unnamed bit-field? Test with an anonymous union member? Test with a flexible array member? Test where the structure being decomposed as a static_assert declaration in it (which wouldn't be a member as far as decomposition is concerned)?
What about a monstrosity like:
struct S {
int x;
int y;
static_assert(__builtin_structured_binding_size(S) == 2); // ?
int z;
static_assert(__builtin_structured_binding_size(S) == 3);
};
There was a problem hiding this comment.
I was just about to make a similar comment
static members? bit-field? anon bit-fields? Reference members?
I guess it will just failed w/ anon union members?
There was a problem hiding this comment.
I added tests for all of these
…n/structured_binding_size
shafik
reviewed
Mar 18, 2025
| struct S0 {}; | ||
| struct S1 {int a;}; | ||
| struct S2 {int a; int b;}; | ||
| struct S3 {double a; int b; int c;}; |
There was a problem hiding this comment.
I was just about to make a similar comment
static members? bit-field? anon bit-fields? Reference members?
I guess it will just failed w/ anon union members?
cor3ntin
changed the title
shafik
reviewed
Mar 18, 2025
| return makeTruthVal(TE->getBoolValue(), TE->getType()); | ||
| if (TE->getType()->isIntegralOrEnumerationType()) | ||
| return makeIntVal(TE->getAPValue().getInt()); | ||
| return std::nullopt; |
There was a problem hiding this comment.
How could we end up here? Is this tested?
There was a problem hiding this comment.
I replaced by an assert.
|
@AaronBallman @shafik Hopefully all yiour feedback has been applied :) |
AaronBallman
approved these changes
Mar 18, 2025
| // expected-error@-1 {{static assertion expression is not an integral constant expression}} \ | ||
| // expected-note@-4 {{definition of 'S' is not complete until the closing '}'}} | ||
| }; | ||
|
|
There was a problem hiding this comment.
Some other edge case tests:
void func(int array[14], int x = __builtin_structured_binding_size(decltype(array))); // Is x 14?
struct S {
static int array[14];
int x = __builtin_structured_binding_size(decltype(array)); // Is x 14?
};
template <typename Ty, int N = __builtin_structured_binding_size(Ty)>
struct T {
static constexpr int value = N;
};
T<int> t1; // Error
static_assert(T<S3>::value == 3);
1187752 to
2745db0
Compare
zwuis
reviewed
Mar 19, 2025
| RD->fields(), [](FieldDecl *FD) { return !FD->isUnnamedBitField(); }); | ||
|
|
||
| if (CheckMemberDecompositionFields(*this, Loc, OrigRD, T, BasePair)) | ||
| return true; |
There was a problem hiding this comment.
return true or return std::nullopt?
|
It looks like this PR introduced some undefined behavior. https://green.lab.llvm.org/job/llvm.org/job/clang-stage2-cmake-RgSan/767/testReport/ Could you take a look at the failing tests and fix them or revert the PR? |
Huh, interesting! Looks like the APValue wasn't initialized in SOME path but we tried to get it. Corentin will be in tomorrow morning and will fix it, else I will revert tomorrow. |
cor3ntin
added a commit
to cor3ntin/llvm-project
that referenced
this pull request
Mar 19, 2025
It turns out trailing objects are uninitialized and APValue assignment operator requires a fully initialized object.
|
See: #132091 |
cor3ntin
added a commit
to cor3ntin/llvm-project
that referenced
this pull request
Mar 19, 2025
It turns out trailing objects are uninitialized and APValue assignment operator requires a fully initialized object.
cor3ntin
added a commit
that referenced
this pull request
Mar 19, 2025
Thanks for letting me know. |
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.
Introduce a trait to determine the number of bindings that would be produced by
auto [...p] = expr;This is necessary to implement P2300 (https://eel.is/c++draft/exec#snd.concepts-5), but can also be used to implement a general get function that supports aggregates
__builtin_structured_binding_sizeis a unary type trait that evaluates to the number of bindings in a decompositionIf the argument cannot be destructured, a sfinae-friendly error is produced.
A type is considered a valid tuple if
std::tuple_size_v<T>is a valid expression, even if there is no validstd::tuple_elementspecialization or suitablegetfunction for that type.This is modeled as a UnaryExprOrTypeTraitExpr, but it is wrapped in a ConstantExpr because the structured binding size can only be established during sema.
Fixes #46049