Skip to content
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

[Clang][P1061] Add stuctured binding packs #121417

Merged
merged 26 commits into from
Jan 29, 2025
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
3c81c5b
[Clang][P1061] stuctured binding packs
ricejasonf Jul 22, 2021
116aff1
[Clang][P1061] Improve visiting HoldingVars; Improve parsing diagnost…
ricejasonf Jan 2, 2025
66ff7c6
[Clang][P1061] Remove unrelated code changes; Address style issues
ricejasonf Jan 2, 2025
5720bd3
[Clang][P1061] Remove vestigial changes
ricejasonf Jan 3, 2025
ffefb67
[Clang][P1061] Simplify unwrapping ResolvedPacks; other style fixes
ricejasonf Jan 3, 2025
685f947
[Clang][P1061] Add ASTReader implementation
ricejasonf Jan 4, 2025
d957783
[Clang][P1061] Improve AST dump testing
ricejasonf Jan 4, 2025
91ca5b4
[Clang][P1061] Add flat_binding_iterator
ricejasonf Jan 10, 2025
abab5cf
[Clang][P1061] Remove Decomp Visit functions
ricejasonf Jan 10, 2025
80acb71
[Clang][P1061] Use range concat for flat_bindings
ricejasonf Jan 15, 2025
360708d
[Clang][P1061] Unremove llvm_unreachable in EmitDecl
ricejasonf Jan 15, 2025
ea249ac
[Clang][P1061] Move rebuild of resolved packs into TreeTransform
ricejasonf Jan 15, 2025
dda9d89
[Clang][P1061] Fix crash
ricejasonf Jan 16, 2025
0b83b99
[Clang][P1061] Fix crash (part 2)
ricejasonf Jan 16, 2025
d496c54
[Clang][P1061] Add bit-field and pack indexing tests
ricejasonf Jan 16, 2025
d064095
[Clang][P1061] Make BindingDecl have NULL TYPE by default
ricejasonf Jan 16, 2025
97257d7
[Clang][P1061] Revert change to typo correction test
ricejasonf Jan 16, 2025
39ab76e
[Clang][P1061] Add codegen test
ricejasonf Jan 17, 2025
765db8a
[Clang][P1061] Clean up stuff
ricejasonf Jan 17, 2025
8cf8205
[Clang][P1061] Use cast_if_present
ricejasonf Jan 17, 2025
6d49a3d
[Clang][P1061] Improve error message wording
ricejasonf Jan 18, 2025
5c351d7
[Clang][P1061] Fix error messages in tests
ricejasonf Jan 18, 2025
5302830
[Clang][P1061] Remove BindingInitWalker
ricejasonf Jan 21, 2025
e497a0c
[Clang][P1061] Handle explicitly capturing binding packs
zwuis Jan 22, 2025
5751d2f
[Clang][P1016] Address nits; Remove unneeded code
ricejasonf Jan 28, 2025
36f8029
[Clang][P1061] Add proper C++ compatability warnings
ricejasonf Jan 29, 2025
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
8 changes: 4 additions & 4 deletions clang/include/clang/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -698,6 +698,10 @@ class ValueDecl : public NamedDecl {
return const_cast<ValueDecl *>(this)->getPotentiallyDecomposedVarDecl();
}

/// Determine whether this value is actually a function parameter pack,
/// init-capture pack, or structured binding pack
bool isParameterPack() const;

// Implement isa/cast/dyncast/etc.
static bool classof(const Decl *D) { return classofKind(D->getKind()); }
static bool classofKind(Kind K) { return K >= firstValue && K <= lastValue; }
Expand Down Expand Up @@ -1527,10 +1531,6 @@ class VarDecl : public DeclaratorDecl, public Redeclarable<VarDecl> {
NonParmVarDeclBits.IsInitCapture = IC;
}

/// Determine whether this variable is actually a function parameter pack or
/// init-capture pack.
bool isParameterPack() const;

/// Whether this local extern variable declaration's previous declaration
/// was declared in the same block scope. Only correct in C++.
bool isPreviousDeclInSameBlockScope() const {
Expand Down
94 changes: 85 additions & 9 deletions clang/include/clang/AST/DeclCXX.h
Original file line number Diff line number Diff line change
Expand Up @@ -4131,16 +4131,18 @@ class BindingDecl : public ValueDecl {
/// binding).
Expr *Binding = nullptr;

BindingDecl(DeclContext *DC, SourceLocation IdLoc, IdentifierInfo *Id)
: ValueDecl(Decl::Binding, DC, IdLoc, Id, QualType()) {}
BindingDecl(DeclContext *DC, SourceLocation IdLoc, IdentifierInfo *Id,
QualType T)
: ValueDecl(Decl::Binding, DC, IdLoc, Id, T) {}

void anchor() override;

public:
friend class ASTDeclReader;

static BindingDecl *Create(ASTContext &C, DeclContext *DC,
SourceLocation IdLoc, IdentifierInfo *Id);
SourceLocation IdLoc, IdentifierInfo *Id,
QualType T);
static BindingDecl *CreateDeserialized(ASTContext &C, GlobalDeclID ID);

/// Get the expression to which this declaration is bound. This may be null
Expand All @@ -4152,10 +4154,6 @@ class BindingDecl : public ValueDecl {
/// decomposition of.
ValueDecl *getDecomposedDecl() const { return Decomp; }

/// Get the variable (if any) that holds the value of evaluating the binding.
/// Only present for user-defined bindings for tuple-like types.
VarDecl *getHoldingVar() const;

/// Set the binding for this BindingDecl, along with its declared type (which
/// should be a possibly-cv-qualified form of the type of the binding, or a
/// reference to such a type).
Expand All @@ -4167,6 +4165,10 @@ class BindingDecl : public ValueDecl {
/// Set the decomposed variable for this BindingDecl.
void setDecomposedDecl(ValueDecl *Decomposed) { Decomp = Decomposed; }

/// Get the variable (if any) that holds the value of evaluating the binding.
/// Only present for user-defined bindings for tuple-like types.
VarDecl *getHoldingVar() const;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we get the comment to move as well? Or at least an updated one?


static bool classof(const Decl *D) { return classofKind(D->getKind()); }
static bool classofKind(Kind K) { return K == Decl::Binding; }
};
Expand Down Expand Up @@ -4213,16 +4215,90 @@ class DecompositionDecl final
static DecompositionDecl *CreateDeserialized(ASTContext &C, GlobalDeclID ID,
unsigned NumBindings);

ArrayRef<BindingDecl *> bindings() const {
return llvm::ArrayRef(getTrailingObjects<BindingDecl *>(), NumBindings);
// Provide the range of bindings which may have a nested pack.
llvm::ArrayRef<BindingDecl *> bindings() const {
return {getTrailingObjects<BindingDecl *>(), NumBindings};
}

// Provide a flattened range to visit each binding directly.
class flat_binding_iterator;
using flat_binding_range = llvm::iterator_range<flat_binding_iterator>;

flat_binding_range flat_bindings() const;
flat_binding_iterator flat_bindings_begin() const;
flat_binding_iterator flat_bindings_end() const;

void printName(raw_ostream &OS, const PrintingPolicy &Policy) const override;

static bool classof(const Decl *D) { return classofKind(D->getKind()); }
static bool classofKind(Kind K) { return K == Decomposition; }
};

/// Iterate bindings that may be contain a nested pack of bindings.
class DecompositionDecl::flat_binding_iterator {
friend class DecompositionDecl;

using ItrTy = llvm::ArrayRef<BindingDecl *>::iterator;
using PackItrTy = llvm::ArrayRef<Expr *>::iterator;

llvm::ArrayRef<BindingDecl *> Bindings;
llvm::ArrayRef<Expr *> PackExprs;
ItrTy Itr = nullptr;
PackItrTy PackItr = nullptr;

explicit flat_binding_iterator(llvm::ArrayRef<BindingDecl *> Bindings,
ItrTy Itr, llvm::ArrayRef<Expr *> PackExprs,
PackItrTy PackItr)
: Bindings(Bindings), PackExprs(PackExprs), Itr(Itr), PackItr(PackItr) {}

public:
flat_binding_iterator() = default;

using value_type = BindingDecl *;
using reference = BindingDecl *&;
using pointer = BindingDecl **;
using iterator_category = std::forward_iterator_tag;

BindingDecl *operator*() {
if (!(*Itr)->isParameterPack())
return *Itr;

auto *DRE = cast<DeclRefExpr>(*PackItr);
return cast<BindingDecl>(DRE->getDecl());
}

flat_binding_iterator &operator++() {
if ((*Itr)->isParameterPack()) {
++PackItr;
if (PackItr != PackExprs.end())
return *this;
else
PackItr = nullptr;
}
++Itr;
return *this;
}

flat_binding_iterator operator++(int) {
flat_binding_iterator Temp = *this;
++*this;
return Temp;
}

bool operator==(const flat_binding_iterator &Other) const {
return Itr == Other.Itr && PackItr == Other.PackItr;
}

bool operator!=(const flat_binding_iterator &Other) const {
return Itr != Other.Itr || PackItr != Other.PackItr;
}
};

inline DecompositionDecl::flat_binding_range
DecompositionDecl::flat_bindings() const {
return flat_binding_range(flat_bindings_begin(), flat_bindings_end());
}

/// An instance of this class represents the declaration of a property
/// member. This is a Microsoft extension to C++, first introduced in
/// Visual Studio .NET 2003 as a parallel to similar features in C#
Expand Down
53 changes: 53 additions & 0 deletions clang/include/clang/AST/ExprCXX.h
Original file line number Diff line number Diff line change
Expand Up @@ -5321,6 +5321,59 @@ class BuiltinBitCastExpr final
}
};

// Represents an unexpanded pack where the list of expressions are
// known. These are used when structured bindings introduce a pack.
class ResolvedUnexpandedPackExpr final
Comment on lines +5324 to +5326
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems to be doing the same job as SubstNonTypeTemplateParmPackExpr and FunctionParmPackExpr. Do we need all three? If so, mentioning the relationship between the three in this comment might be nice -- but I do wonder if FunctionParmPackExpr could be generalized to cover the case of a binding declaration too.

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 definitely think they could be generalized which is what I had in mind with ResolvedUnexpandedPackExpr. This is originally what I had for parmexprs (P1221).

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we try to address that?

: public Expr,
private llvm::TrailingObjects<ResolvedUnexpandedPackExpr, Expr *> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we store BindingDecl * in trailing objects like FunctionParmPackExpr?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably. That would simplify the DeclRefExpr stuff and a few other things. BindingPackExpr?

Copy link
Contributor

Choose a reason for hiding this comment

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

... BindingPackExpr?

This is what I think: using BindingPackExpr and modifying it when implementing other features (member packs etc).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Personally, I am content to leave it as is, but storing Decls instead of Exprs does have the benefit of delaying building the DeclRefExprs until expansion which creates less garbage and would simplify flat_bindings. If anything, I would leave the name ResolvedUnexpandedPackExpr and make the trailing objects ValueDecl to support future consolidation. That is premised on never wanting to create a pack of expressions which was my original intent as this was made for P1221 (which I have abandoned). I will defer to @cor3ntin.

Copy link
Contributor

Choose a reason for hiding this comment

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

That is premised on never wanting to create a pack of expressions

A use case would be unpacking a tuple for example (or data member pack but in that case maybe we want to store decl for that too)

Either way, I don't think we should predicament the design on future evolution. If we need to change it later, we can

friend class ASTStmtReader;
friend class ASTStmtWriter;
friend TrailingObjects;

SourceLocation BeginLoc;
unsigned NumExprs;

ResolvedUnexpandedPackExpr(SourceLocation BL, QualType QT, unsigned NumExprs);

public:
static ResolvedUnexpandedPackExpr *CreateDeserialized(ASTContext &C,
unsigned NumExprs);
static ResolvedUnexpandedPackExpr *
Create(ASTContext &C, SourceLocation BeginLoc, QualType T, unsigned NumExprs);
static ResolvedUnexpandedPackExpr *Create(ASTContext &C,
SourceLocation BeginLoc, QualType T,
llvm::ArrayRef<Expr *> Exprs);

unsigned getNumExprs() const { return NumExprs; }

llvm::MutableArrayRef<Expr *> getExprs() {
return {getTrailingObjects<Expr *>(), NumExprs};
}

llvm::ArrayRef<Expr *> getExprs() const {
return {getTrailingObjects<Expr *>(), NumExprs};
}

Expr *getExpansion(unsigned Idx) { return getExprs()[Idx]; }
Expr *getExpansion(unsigned Idx) const { return getExprs()[Idx]; }

// Iterators
child_range children() {
return child_range((Stmt **)getTrailingObjects<Expr *>(),
(Stmt **)getTrailingObjects<Expr *>() + getNumExprs());
}

SourceLocation getBeginLoc() const LLVM_READONLY { return BeginLoc; }
SourceLocation getEndLoc() const LLVM_READONLY { return BeginLoc; }

// Returns the resolved pack of a decl or nullptr
static ResolvedUnexpandedPackExpr *getFromDecl(Decl *);

static bool classof(const Stmt *T) {
return T->getStmtClass() == ResolvedUnexpandedPackExprClass;
}
};

} // namespace clang

#endif // LLVM_CLANG_AST_EXPRCXX_H
1 change: 1 addition & 0 deletions clang/include/clang/AST/RecursiveASTVisitor.h
Original file line number Diff line number Diff line change
Expand Up @@ -2936,6 +2936,7 @@ DEF_TRAVERSE_STMT(FunctionParmPackExpr, {})
DEF_TRAVERSE_STMT(CXXFoldExpr, {})
DEF_TRAVERSE_STMT(AtomicExpr, {})
DEF_TRAVERSE_STMT(CXXParenListInitExpr, {})
DEF_TRAVERSE_STMT(ResolvedUnexpandedPackExpr, {})

DEF_TRAVERSE_STMT(MaterializeTemporaryExpr, {
if (S->getLifetimeExtendedTemporaryDecl()) {
Expand Down
7 changes: 7 additions & 0 deletions clang/include/clang/Basic/DiagnosticParseKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -1099,6 +1099,13 @@ def err_lambda_capture_misplaced_ellipsis : Error<
"the name of the capture">;
def err_lambda_capture_multiple_ellipses : Error<
"multiple ellipses in pack capture">;
def err_binding_multiple_ellipses : Error<
"multiple ellipses in structured binding declaration">;
def note_previous_ellipsis : Note<
"previous ellipsis specified here">;
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems a little strange to talk about ellipses rather than packs here. Maybe "multiple binding packs in [...]" instead?

def ext_cxx_binding_pack : ExtWarn<
"structured binding pack is incompatible with C++ standards before C++2c">,
InGroup<CXX26>;
def err_capture_default_first : Error<
"capture default must be first">;
def ext_decl_attrs_on_lambda : ExtWarn<
Expand Down
3 changes: 3 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -5906,6 +5906,9 @@ def warn_cxx23_pack_indexing : Warning<
"pack indexing is incompatible with C++ standards before C++2c">,
DefaultIgnore, InGroup<CXXPre26Compat>;

def err_pack_outside_template : Error<
"pack declaration outside of template">;

def err_fold_expression_packs_both_sides : Error<
"binary fold expression has unexpanded parameter packs in both operands">;
def err_fold_expression_empty : Error<
Expand Down
1 change: 1 addition & 0 deletions clang/include/clang/Basic/StmtNodes.td
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ def MaterializeTemporaryExpr : StmtNode<Expr>;
def LambdaExpr : StmtNode<Expr>;
def CXXFoldExpr : StmtNode<Expr>;
def CXXParenListInitExpr: StmtNode<Expr>;
def ResolvedUnexpandedPackExpr : StmtNode<Expr>;

// C++ Coroutines expressions
def CoroutineSuspendExpr : StmtNode<Expr, 1>;
Expand Down
1 change: 1 addition & 0 deletions clang/include/clang/Sema/DeclSpec.h
Original file line number Diff line number Diff line change
Expand Up @@ -1795,6 +1795,7 @@ class DecompositionDeclarator {
IdentifierInfo *Name;
SourceLocation NameLoc;
std::optional<ParsedAttributes> Attrs;
SourceLocation EllipsisLoc;
};

private:
Expand Down
4 changes: 3 additions & 1 deletion clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,8 @@ void threadSafetyCleanup(BeforeSet *Cache);

// FIXME: No way to easily map from TemplateTypeParmTypes to
// TemplateTypeParmDecls, so we have this horrible PointerUnion.
typedef std::pair<llvm::PointerUnion<const TemplateTypeParmType *, NamedDecl *>,
typedef std::pair<llvm::PointerUnion<const TemplateTypeParmType *, NamedDecl *,
ResolvedUnexpandedPackExpr *>,
SourceLocation>
UnexpandedParameterPack;

Expand Down Expand Up @@ -6012,6 +6013,7 @@ class Sema final : public SemaBase {
RecordDecl *ClassDecl,
const IdentifierInfo *Name);

unsigned GetDecompositionElementCount(QualType DecompType);
void CheckCompleteDecompositionDeclaration(DecompositionDecl *DD);

/// Stack containing information needed when in C++2a an 'auto' is encountered
Expand Down
1 change: 1 addition & 0 deletions clang/include/clang/Serialization/ASTBitCodes.h
Original file line number Diff line number Diff line change
Expand Up @@ -1890,6 +1890,7 @@ enum StmtCode {
EXPR_PACK_EXPANSION, // PackExpansionExpr
EXPR_PACK_INDEXING, // PackIndexingExpr
EXPR_SIZEOF_PACK, // SizeOfPackExpr
EXPR_RESOLVED_UNEXPANDED_PACK, // ResolvedUnexpandedPackExpr
EXPR_SUBST_NON_TYPE_TEMPLATE_PARM, // SubstNonTypeTemplateParmExpr
EXPR_SUBST_NON_TYPE_TEMPLATE_PARM_PACK, // SubstNonTypeTemplateParmPackExpr
EXPR_FUNCTION_PARM_PACK, // FunctionParmPackExpr
Expand Down
5 changes: 3 additions & 2 deletions clang/lib/AST/ASTContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12726,11 +12726,12 @@ bool ASTContext::DeclMustBeEmitted(const Decl *D) {

// Likewise, variables with tuple-like bindings are required if their
// bindings have side-effects.
if (const auto *DD = dyn_cast<DecompositionDecl>(VD))
for (const auto *BD : DD->bindings())
if (const auto *DD = dyn_cast<DecompositionDecl>(VD)) {
for (const auto *BD : DD->flat_bindings())
if (const auto *BindingVD = BD->getHoldingVar())
if (DeclMustBeEmitted(BindingVD))
return true;
}

return false;
}
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/AST/ASTImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2552,7 +2552,7 @@ ExpectedDecl ASTNodeImporter::VisitBindingDecl(BindingDecl *D) {

BindingDecl *ToD;
if (GetImportedOrCreateDecl(ToD, D, Importer.getToContext(), DC, Loc,
Name.getAsIdentifierInfo()))
Name.getAsIdentifierInfo(), D->getType()))
return ToD;

Error Err = Error::success();
Expand Down
11 changes: 7 additions & 4 deletions clang/lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2659,10 +2659,6 @@ bool VarDecl::checkForConstantInitialization(
return Eval->HasConstantInitialization;
}

bool VarDecl::isParameterPack() const {
return isa<PackExpansionType>(getType());
}

template<typename DeclT>
static DeclT *getDefinitionOrSelf(DeclT *D) {
assert(D);
Expand Down Expand Up @@ -5397,6 +5393,13 @@ bool ValueDecl::isInitCapture() const {
return false;
}

bool ValueDecl::isParameterPack() const {
if (const auto *NTTP = dyn_cast<NonTypeTemplateParmDecl>(this))
return NTTP->isParameterPack();

return isa_and_nonnull<PackExpansionType>(getType().getTypePtrOrNull());
}

void ImplicitParamDecl::anchor() {}

ImplicitParamDecl *ImplicitParamDecl::Create(ASTContext &C, DeclContext *DC,
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/AST/DeclBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ bool Decl::isTemplateParameterPack() const {
}

bool Decl::isParameterPack() const {
if (const auto *Var = dyn_cast<VarDecl>(this))
if (const auto *Var = dyn_cast<ValueDecl>(this))
return Var->isParameterPack();

return isTemplateParameterPack();
Expand Down
Loading
Loading