Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
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
2 changes: 2 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ C++2c Feature Support

- Implemented `P1061R10 Structured Bindings can introduce a Pack <https://wg21.link/P1061R10>`_.

- Implemented `P0963R3 Structured binding declaration as a condition <https://wg21.link/P0963R3>`_.

C++23 Feature Support
^^^^^^^^^^^^^^^^^^^^^

Expand Down
27 changes: 19 additions & 8 deletions clang/include/clang/AST/DeclCXX.h
Original file line number Diff line number Diff line change
Expand Up @@ -4224,15 +4224,18 @@ class DecompositionDecl final
: public VarDecl,
private llvm::TrailingObjects<DecompositionDecl, BindingDecl *> {
/// The number of BindingDecl*s following this object.
unsigned NumBindings;
unsigned NumBindings : 31;

LLVM_PREFERRED_TYPE(bool)
unsigned IsDecisionVariable : 1;

DecompositionDecl(ASTContext &C, DeclContext *DC, SourceLocation StartLoc,
SourceLocation LSquareLoc, QualType T,
TypeSourceInfo *TInfo, StorageClass SC,
ArrayRef<BindingDecl *> Bindings)
ArrayRef<BindingDecl *> Bindings, bool IsDecisionVariable)
: VarDecl(Decomposition, C, DC, StartLoc, LSquareLoc, nullptr, T, TInfo,
SC),
NumBindings(Bindings.size()) {
NumBindings(Bindings.size()), IsDecisionVariable(IsDecisionVariable) {
std::uninitialized_copy(Bindings.begin(), Bindings.end(),
getTrailingObjects<BindingDecl *>());
for (auto *B : Bindings) {
Expand All @@ -4253,12 +4256,14 @@ class DecompositionDecl final

static DecompositionDecl *Create(ASTContext &C, DeclContext *DC,
SourceLocation StartLoc,
SourceLocation LSquareLoc,
QualType T, TypeSourceInfo *TInfo,
StorageClass S,
ArrayRef<BindingDecl *> Bindings);
SourceLocation LSquareLoc, QualType T,
TypeSourceInfo *TInfo, StorageClass S,
ArrayRef<BindingDecl *> Bindings,
bool IsDecisionVariable);

static DecompositionDecl *CreateDeserialized(ASTContext &C, GlobalDeclID ID,
unsigned NumBindings);
unsigned NumBindings,
bool IsDecisionVariable);

// Provide the range of bindings which may have a nested pack.
llvm::ArrayRef<BindingDecl *> bindings() const {
Expand All @@ -4285,6 +4290,12 @@ class DecompositionDecl final
std::move(Bindings));
}

/// The decision variable of a condition that is a structured binding
/// declaration is specified in [dcl.struct.bind]p4:
/// If a structured binding declaration appears as a condition, the decision
/// variable of the condition is e.
bool isDecisionVariable() const { return IsDecisionVariable; }

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

static bool classof(const Decl *D) { return classofKind(D->getKind()); }
Expand Down
8 changes: 6 additions & 2 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -529,8 +529,12 @@ def warn_cxx14_compat_decomp_decl : Warning<
def ext_decomp_decl : ExtWarn<
"decomposition declarations are a C++17 extension">, InGroup<CXX17>;
def ext_decomp_decl_cond : ExtWarn<
"ISO C++17 does not permit structured binding declaration in a condition">,
InGroup<DiagGroup<"binding-in-condition">>;
"structured binding declaration in a condition is a C++2c extenstion">,
InGroup<CXX26>;
def warn_cxx26_decomp_decl_cond : Warning<
"structured binding declaration in a condition is incompatible with "
"C++ standards before C++2c">,
InGroup<CXXPre26Compat>, DefaultIgnore;
def err_decomp_decl_spec : Error<
"decomposition declaration cannot be declared "
"%plural{1:'%1'|:with '%1' specifiers}0">;
Expand Down
7 changes: 4 additions & 3 deletions clang/lib/AST/ASTImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4614,9 +4614,10 @@ ExpectedDecl ASTNodeImporter::VisitVarDecl(VarDecl *D) {
ImportArrayChecked(FromDecomp->bindings(), Bindings.begin()))
return std::move(Err);
DecompositionDecl *ToDecomp;
if (GetImportedOrCreateDecl(
ToDecomp, FromDecomp, Importer.getToContext(), DC, ToInnerLocStart,
Loc, ToType, ToTypeSourceInfo, D->getStorageClass(), Bindings))
if (GetImportedOrCreateDecl(ToDecomp, FromDecomp, Importer.getToContext(),
DC, ToInnerLocStart, Loc, ToType,
ToTypeSourceInfo, D->getStorageClass(),
Bindings, FromDecomp->isDecisionVariable()))
return ToDecomp;
ToVar = ToDecomp;
} else {
Expand Down
45 changes: 39 additions & 6 deletions clang/lib/AST/ByteCode/Compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5104,6 +5104,16 @@ bool Compiler<Emitter>::visitCompoundStmt(const CompoundStmt *S) {
return Scope.destroyLocals();
}

template <class Emitter>
bool Compiler<Emitter>::emitDecompositionVarInit(const DecompositionDecl *DD) {
for (auto *BD : DD->bindings())
if (auto *KD = BD->getHoldingVar()) {
if (!this->visitVarDecl(KD))
return false;
}
return true;
}

template <class Emitter>
bool Compiler<Emitter>::visitDeclStmt(const DeclStmt *DS) {
for (const auto *D : DS->decls()) {
Expand All @@ -5118,12 +5128,10 @@ bool Compiler<Emitter>::visitDeclStmt(const DeclStmt *DS) {
return false;

// Register decomposition decl holding vars.
if (const auto *DD = dyn_cast<DecompositionDecl>(VD)) {
for (auto *BD : DD->bindings())
if (auto *KD = BD->getHoldingVar()) {
if (!this->visitVarDecl(KD))
return false;
}
if (const auto *DD = dyn_cast<DecompositionDecl>(VD);
DD && !DD->isDecisionVariable()) {
if (!this->emitDecompositionVarInit(DD))
return false;
}
}

Expand Down Expand Up @@ -5189,6 +5197,12 @@ template <class Emitter> bool Compiler<Emitter>::visitIfStmt(const IfStmt *IS) {
return false;
}

if (auto *DD =
dyn_cast_if_present<DecompositionDecl>(IS->getConditionVariable());
DD && DD->isDecisionVariable())
if (!this->emitDecompositionVarInit(DD))
return false;

if (const Stmt *Else = IS->getElse()) {
LabelTy LabelElse = this->getLabel();
LabelTy LabelEnd = this->getLabel();
Expand Down Expand Up @@ -5249,6 +5263,13 @@ bool Compiler<Emitter>::visitWhileStmt(const WhileStmt *S) {

if (!this->visitBool(Cond))
return false;

if (auto *DD =
dyn_cast_if_present<DecompositionDecl>(S->getConditionVariable());
DD && DD->isDecisionVariable())
if (!this->emitDecompositionVarInit(DD))
return false;

if (!this->jumpFalse(EndLabel))
return false;

Expand Down Expand Up @@ -5330,6 +5351,12 @@ bool Compiler<Emitter>::visitForStmt(const ForStmt *S) {
return false;
}

if (auto *DD =
dyn_cast_if_present<DecompositionDecl>(S->getConditionVariable());
DD && DD->isDecisionVariable())
if (!this->emitDecompositionVarInit(DD))
return false;

if (Body && !this->visitStmt(Body))
return false;

Expand Down Expand Up @@ -5452,6 +5479,12 @@ bool Compiler<Emitter>::visitSwitchStmt(const SwitchStmt *S) {
if (!this->emitSetLocal(CondT, CondVar, S))
return false;

if (auto *DD =
dyn_cast_if_present<DecompositionDecl>(S->getConditionVariable());
DD && DD->isDecisionVariable())
if (!this->emitDecompositionVarInit(DD))
return false;

CaseMap CaseLabels;
// Create labels and comparison ops for all case statements.
for (const SwitchCase *SC = S->getSwitchCaseList(); SC;
Expand Down
1 change: 1 addition & 0 deletions clang/lib/AST/ByteCode/Compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,7 @@ class Compiler : public ConstStmtVisitor<Compiler<Emitter>, bool>,
bool compileUnionAssignmentOperator(const CXXMethodDecl *MD);

bool checkLiteralType(const Expr *E);
bool emitDecompositionVarInit(const DecompositionDecl *DD);

protected:
/// Variable to storage mapping.
Expand Down
21 changes: 12 additions & 9 deletions clang/lib/AST/DeclCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3520,21 +3520,24 @@ DecompositionDecl *DecompositionDecl::Create(ASTContext &C, DeclContext *DC,
SourceLocation LSquareLoc,
QualType T, TypeSourceInfo *TInfo,
StorageClass SC,
ArrayRef<BindingDecl *> Bindings) {
ArrayRef<BindingDecl *> Bindings,
bool IsDecisionVariable) {
size_t Extra = additionalSizeToAlloc<BindingDecl *>(Bindings.size());
return new (C, DC, Extra)
DecompositionDecl(C, DC, StartLoc, LSquareLoc, T, TInfo, SC, Bindings);
return new (C, DC, Extra) DecompositionDecl(
C, DC, StartLoc, LSquareLoc, T, TInfo, SC, Bindings, IsDecisionVariable);
}

DecompositionDecl *DecompositionDecl::CreateDeserialized(ASTContext &C,
GlobalDeclID ID,
unsigned NumBindings) {
DecompositionDecl *
DecompositionDecl::CreateDeserialized(ASTContext &C, GlobalDeclID ID,
unsigned NumBindings,
bool IsDecisionVariable) {
size_t Extra = additionalSizeToAlloc<BindingDecl *>(NumBindings);
auto *Result = new (C, ID, Extra)
DecompositionDecl(C, nullptr, SourceLocation(), SourceLocation(),
QualType(), nullptr, StorageClass(), {});
auto *Result = new (C, ID, Extra) DecompositionDecl(
C, nullptr, SourceLocation(), SourceLocation(), QualType(), nullptr,
StorageClass(), {}, IsDecisionVariable);
// Set up and clean out the bindings array.
Result->NumBindings = NumBindings;
Result->IsDecisionVariable = IsDecisionVariable;
auto *Trail = Result->getTrailingObjects<BindingDecl *>();
for (unsigned I = 0; I != NumBindings; ++I)
new (Trail + I) BindingDecl*(nullptr);
Expand Down
30 changes: 26 additions & 4 deletions clang/lib/AST/ExprConstant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5218,16 +5218,28 @@ static bool EvaluateVarDecl(EvalInfo &Info, const VarDecl *VD) {
return true;
}

static bool EvaluateDecompositionDeclInit(EvalInfo &Info,
const DecompositionDecl *DD);

static bool EvaluateDecl(EvalInfo &Info, const Decl *D) {
bool OK = true;

if (const VarDecl *VD = dyn_cast<VarDecl>(D))
OK &= EvaluateVarDecl(Info, VD);

if (const DecompositionDecl *DD = dyn_cast<DecompositionDecl>(D))
for (auto *BD : DD->flat_bindings())
if (auto *VD = BD->getHoldingVar())
OK &= EvaluateDecl(Info, VD);
if (const DecompositionDecl *DD = dyn_cast<DecompositionDecl>(D);
DD && !DD->isDecisionVariable())
OK &= EvaluateDecompositionDeclInit(Info, DD);

return OK;
}

static bool EvaluateDecompositionDeclInit(EvalInfo &Info,
const DecompositionDecl *DD) {
bool OK = true;
for (auto *BD : DD->flat_bindings())
if (auto *VD = BD->getHoldingVar())
OK &= EvaluateDecl(Info, VD);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want to short-circuit here instead? We could early return.

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 think we can, at least we have been doing so in the bytecode evaluator; though I'm not sure why there's discrepancy in the old constant evaluator

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with Erich

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can't bail out early here: EvaluateVarDecl() might fail because EvaluateInPlace() might fail, and in that case, we still need to continue evaluating the initializers; otherwise, we'll crash. Here is an example

namespace std {

template <typename T> struct tuple_size;

template <int, typename> struct tuple_element;

} // namespace std


namespace constant {
  struct Q {};
  template<int N> constexpr int get(Q &&) { return N * N; }
}
template<> struct std::tuple_size<constant::Q> { static const int value = 3; };
template<int N> struct std::tuple_element<N, constant::Q> { typedef int type; };

namespace constant {
Q q;

constexpr bool f() {
  auto [a, b, c] = q;
  return a == 0 && b == 1 && c == 4;
}
static_assert(f());
}

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 crash doesn't occur in C++23 mode)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And we crash during isPotentialConstantExpr(), wherein we set CheckingPotentialConstantExpression = true that results in EvaluateVarDecl() returning true because EvaluateInPlace() it called bailed out of evaluation in this case. So in short, we can't short-circuit the initialization of the binding decls because they're necessary in CheckingPotentialConstantExpression.


return OK;
}
Expand All @@ -5251,6 +5263,10 @@ static bool EvaluateCond(EvalInfo &Info, const VarDecl *CondDecl,
return false;
if (!EvaluateAsBooleanCondition(Cond, Result, Info))
return false;
if (auto *DD = dyn_cast_if_present<DecompositionDecl>(CondDecl);
DD && DD->isDecisionVariable() &&
!EvaluateDecompositionDeclInit(Info, DD))
return false;
return Scope.destroy();
}

Expand Down Expand Up @@ -5335,6 +5351,12 @@ static EvalStmtResult EvaluateSwitch(StmtResult &Result, EvalInfo &Info,
if (!EvaluateInteger(SS->getCond(), Value, Info))
return ESR_Failed;

if (auto *DD =
dyn_cast_if_present<DecompositionDecl>(SS->getConditionVariable());
DD && DD->isDecisionVariable() &&
!EvaluateDecompositionDeclInit(Info, DD))
return ESR_Failed;

if (!CondScope.destroy())
return ESR_Failed;
}
Expand Down
13 changes: 9 additions & 4 deletions clang/lib/CodeGen/CGDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,9 @@ void CodeGenFunction::EmitDecl(const Decl &D) {
assert(VD.isLocalVarDecl() &&
"Should not see file-scope variables inside a function!");
EmitVarDecl(VD);
if (auto *DD = dyn_cast<DecompositionDecl>(&VD))
for (auto *B : DD->flat_bindings())
if (auto *HD = B->getHoldingVar())
EmitVarDecl(*HD);
if (auto *DD = dyn_cast<DecompositionDecl>(&VD);
DD && !DD->isDecisionVariable())
EmitDecompositionVarInit(*DD);

return;
}
Expand Down Expand Up @@ -2057,6 +2056,12 @@ void CodeGenFunction::EmitAutoVarInit(const AutoVarEmission &emission) {
/*IsAutoInit=*/false);
}

void CodeGenFunction::EmitDecompositionVarInit(const DecompositionDecl &DD) {
for (auto *B : DD.flat_bindings())
if (auto *HD = B->getHoldingVar())
EmitVarDecl(*HD);
}

/// Emit an expression as an initializer for an object (variable, field, etc.)
/// at the given location. The expression is not necessarily the normal
/// initializer for the object, and the address is not necessarily
Expand Down
38 changes: 35 additions & 3 deletions clang/lib/CodeGen/CGStmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -913,6 +913,11 @@ void CodeGenFunction::EmitIfStmt(const IfStmt &S) {
if (CondConstant)
incrementProfileCounter(&S);
if (Executed) {
if (auto *DD = dyn_cast_if_present<DecompositionDecl>(
S.getConditionVariable())) {
assert(DD->isDecisionVariable());
EmitDecompositionVarInit(*DD);
}
RunCleanupsScope ExecutedScope(*this);
EmitStmt(Executed);
}
Expand Down Expand Up @@ -952,10 +957,16 @@ void CodeGenFunction::EmitIfStmt(const IfStmt &S) {
// there is a 'return' within the body, but this is particularly beneficial
// when one if-stmt is nested within another if-stmt so that all of the MC/DC
// updates are kept linear and consistent.
if (!CGM.getCodeGenOpts().MCDCCoverage)
EmitBranchOnBoolExpr(S.getCond(), ThenBlock, ElseBlock, ThenCount, LH);
else {
if (!CGM.getCodeGenOpts().MCDCCoverage) {
EmitBranchOnBoolExpr(S.getCond(), ThenBlock, ElseBlock, ThenCount, LH,
nullptr, S.getConditionVariable());
} else {
llvm::Value *BoolCondVal = EvaluateExprAsBool(S.getCond());
if (auto *DD =
dyn_cast_if_present<DecompositionDecl>(S.getConditionVariable())) {
assert(DD->isDecisionVariable());
EmitDecompositionVarInit(*DD);
}
Builder.CreateCondBr(BoolCondVal, ThenBlock, ElseBlock);
}

Expand Down Expand Up @@ -1099,6 +1110,11 @@ void CodeGenFunction::EmitWhileStmt(const WhileStmt &S,
// execution of the loop body.
llvm::Value *BoolCondVal = EvaluateExprAsBool(S.getCond());

if (auto *DD =
dyn_cast_if_present<DecompositionDecl>(S.getConditionVariable());
DD && DD->isDecisionVariable())
EmitDecompositionVarInit(*DD);

// while(1) is common, avoid extra exit blocks. Be sure
// to correctly handle break/continue though.
llvm::ConstantInt *C = dyn_cast<llvm::ConstantInt>(BoolCondVal);
Expand Down Expand Up @@ -1332,6 +1348,12 @@ void CodeGenFunction::EmitForStmt(const ForStmt &S,
// C99 6.8.5p2/p4: The first substatement is executed if the expression
// compares unequal to 0. The condition must be a scalar type.
llvm::Value *BoolCondVal = EvaluateExprAsBool(S.getCond());

if (auto *DD =
dyn_cast_if_present<DecompositionDecl>(S.getConditionVariable());
DD && DD->isDecisionVariable())
EmitDecompositionVarInit(*DD);

llvm::MDNode *Weights =
createProfileWeightsForLoop(S.getCond(), getProfileCount(S.getBody()));
if (!Weights && CGM.getCodeGenOpts().OptimizationLevel)
Expand Down Expand Up @@ -2229,6 +2251,11 @@ void CodeGenFunction::EmitSwitchStmt(const SwitchStmt &S) {
if (S.getConditionVariable())
EmitDecl(*S.getConditionVariable());

if (auto *DD =
dyn_cast_if_present<DecompositionDecl>(S.getConditionVariable());
DD && DD->isDecisionVariable())
EmitDecompositionVarInit(*DD);

// At this point, we are no longer "within" a switch instance, so
// we can temporarily enforce this to ensure that any embedded case
// statements are not emitted.
Expand Down Expand Up @@ -2260,6 +2287,11 @@ void CodeGenFunction::EmitSwitchStmt(const SwitchStmt &S) {
EmitDecl(*S.getConditionVariable());
llvm::Value *CondV = EmitScalarExpr(S.getCond());

if (auto *DD =
dyn_cast_if_present<DecompositionDecl>(S.getConditionVariable());
DD && DD->isDecisionVariable())
EmitDecompositionVarInit(*DD);

// Create basic block to hold stuff that comes after switch
// statement. We also need to create a default block now so that
// explicit case ranges tests can have a place to jump to on
Expand Down
Loading