[Analyzer][CFG] Correctly handle rebuilt default arg and default init expression #117437
+235
−96
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
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-static-analyzer-1 @llvm/pr-subscribers-clang-modules Author: None (yronglin) ChangesClang now support the following:
But CFG and ExprEngine need to be updated to address this change. This PR reapply #91879.
Patch is 30.04 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/117437.diff 14 Files Affected:
diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h
index 696a574833dad2..99680537a3f777 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -1277,7 +1277,8 @@ class CXXDefaultArgExpr final
DeclContext *UsedContext;
CXXDefaultArgExpr(StmtClass SC, SourceLocation Loc, ParmVarDecl *Param,
- Expr *RewrittenExpr, DeclContext *UsedContext)
+ DeclContext *UsedContext, Expr *RewrittenExpr,
+ bool HasRebuiltInit)
: Expr(SC,
Param->hasUnparsedDefaultArg()
? Param->getType().getNonReferenceType()
@@ -1287,25 +1288,27 @@ class CXXDefaultArgExpr final
Param(Param), UsedContext(UsedContext) {
CXXDefaultArgExprBits.Loc = Loc;
CXXDefaultArgExprBits.HasRewrittenInit = RewrittenExpr != nullptr;
+ CXXDefaultArgExprBits.HasRebuiltInit = HasRebuiltInit;
if (RewrittenExpr)
*getTrailingObjects<Expr *>() = RewrittenExpr;
setDependence(computeDependence(this));
}
- CXXDefaultArgExpr(EmptyShell Empty, bool HasRewrittenInit)
+ CXXDefaultArgExpr(EmptyShell Empty, bool HasRewrittenInit, bool HasRebuiltInit)
: Expr(CXXDefaultArgExprClass, Empty) {
CXXDefaultArgExprBits.HasRewrittenInit = HasRewrittenInit;
+ CXXDefaultArgExprBits.HasRebuiltInit = HasRebuiltInit;
}
public:
static CXXDefaultArgExpr *CreateEmpty(const ASTContext &C,
- bool HasRewrittenInit);
+ bool HasRewrittenInit, bool HasRebuiltInit);
// \p Param is the parameter whose default argument is used by this
// expression.
static CXXDefaultArgExpr *Create(const ASTContext &C, SourceLocation Loc,
- ParmVarDecl *Param, Expr *RewrittenExpr,
- DeclContext *UsedContext);
+ ParmVarDecl *Param, DeclContext *UsedContext,
+ Expr *RewrittenExpr, bool HasRebuiltInit);
// Retrieve the parameter that the argument was created from.
const ParmVarDecl *getParam() const { return Param; }
ParmVarDecl *getParam() { return Param; }
@@ -1314,6 +1317,10 @@ class CXXDefaultArgExpr final
return CXXDefaultArgExprBits.HasRewrittenInit;
}
+ bool hasRebuiltInit() const {
+ return CXXDefaultArgExprBits.HasRebuiltInit;
+ }
+
// Retrieve the argument to the function call.
Expr *getExpr();
const Expr *getExpr() const {
@@ -1385,26 +1392,31 @@ class CXXDefaultInitExpr final
CXXDefaultInitExpr(const ASTContext &Ctx, SourceLocation Loc,
FieldDecl *Field, QualType Ty, DeclContext *UsedContext,
- Expr *RewrittenInitExpr);
+ Expr *RewrittenInitExpr, bool HasRebuiltInit);
- CXXDefaultInitExpr(EmptyShell Empty, bool HasRewrittenInit)
+ CXXDefaultInitExpr(EmptyShell Empty, bool HasRewrittenInit, bool HasRebuiltInit)
: Expr(CXXDefaultInitExprClass, Empty) {
CXXDefaultInitExprBits.HasRewrittenInit = HasRewrittenInit;
+ CXXDefaultInitExprBits.HasRebuiltInit = HasRebuiltInit;
}
public:
static CXXDefaultInitExpr *CreateEmpty(const ASTContext &C,
- bool HasRewrittenInit);
+ bool HasRewrittenInit, bool HasRebuiltInit);
/// \p Field is the non-static data member whose default initializer is used
/// by this expression.
static CXXDefaultInitExpr *Create(const ASTContext &Ctx, SourceLocation Loc,
FieldDecl *Field, DeclContext *UsedContext,
- Expr *RewrittenInitExpr);
+ Expr *RewrittenInitExpr, bool HasRebuiltInit);
bool hasRewrittenInit() const {
return CXXDefaultInitExprBits.HasRewrittenInit;
}
+ bool hasRebuiltInit() const {
+ return CXXDefaultInitExprBits.HasRebuiltInit;
+ }
+
/// Get the field whose initializer will be used.
FieldDecl *getField() { return Field; }
const FieldDecl *getField() const { return Field; }
diff --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h
index 83fafbabb1d460..1b2dbcbaa09219 100644
--- a/clang/include/clang/AST/Stmt.h
+++ b/clang/include/clang/AST/Stmt.h
@@ -839,6 +839,10 @@ class alignas(void *) Stmt {
LLVM_PREFERRED_TYPE(bool)
unsigned HasRewrittenInit : 1;
+ /// Whether this CXXDefaultArgExpr rebuild its argument and stores a copy.
+ LLVM_PREFERRED_TYPE(bool)
+ unsigned HasRebuiltInit : 1;
+
/// The location where the default argument expression was used.
SourceLocation Loc;
};
@@ -850,11 +854,16 @@ class alignas(void *) Stmt {
LLVM_PREFERRED_TYPE(ExprBitfields)
unsigned : NumExprBits;
- /// Whether this CXXDefaultInitExprBitfields rewrote its argument and stores
- /// a copy.
+ /// Whether this CXXDefaultInitExpr rewrote its argument and stores
+ /// a copy, unlike HasRebuiltInit, when this flag is true, the argument may
+ /// be partial rebuilt.
LLVM_PREFERRED_TYPE(bool)
unsigned HasRewrittenInit : 1;
+ /// Whether this CXXDefaultInitExpr fully rebuild its argument and stores a copy.
+ LLVM_PREFERRED_TYPE(bool)
+ unsigned HasRebuiltInit : 1;
+
/// The location where the default initializer expression was used.
SourceLocation Loc;
};
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index baed1416635432..e2126f5fd29a47 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -8154,8 +8154,8 @@ ExpectedStmt ASTNodeImporter::VisitCXXDefaultArgExpr(CXXDefaultArgExpr *E) {
RewrittenInit = ExprOrErr.get();
}
return CXXDefaultArgExpr::Create(Importer.getToContext(), *ToUsedLocOrErr,
- *ToParamOrErr, RewrittenInit,
- *UsedContextOrErr);
+ *ToParamOrErr, *UsedContextOrErr,
+ RewrittenInit, E->hasRebuiltInit());
}
ExpectedStmt
@@ -8863,7 +8863,8 @@ ExpectedStmt ASTNodeImporter::VisitCXXDefaultInitExpr(CXXDefaultInitExpr *E) {
}
return CXXDefaultInitExpr::Create(Importer.getToContext(), *ToBeginLocOrErr,
- ToField, *UsedContextOrErr, RewrittenInit);
+ ToField, *UsedContextOrErr, RewrittenInit,
+ E->hasRebuiltInit());
}
ExpectedStmt ASTNodeImporter::VisitCXXNamedCastExpr(CXXNamedCastExpr *E) {
diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index 0ce129de85f03f..6b6507d4b5ebeb 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -1009,21 +1009,23 @@ const IdentifierInfo *UserDefinedLiteral::getUDSuffix() const {
}
CXXDefaultArgExpr *CXXDefaultArgExpr::CreateEmpty(const ASTContext &C,
- bool HasRewrittenInit) {
+ bool HasRewrittenInit,
+ bool HasRebuiltInit) {
size_t Size = totalSizeToAlloc<Expr *>(HasRewrittenInit);
auto *Mem = C.Allocate(Size, alignof(CXXDefaultArgExpr));
- return new (Mem) CXXDefaultArgExpr(EmptyShell(), HasRewrittenInit);
+ return new (Mem)
+ CXXDefaultArgExpr(EmptyShell(), HasRewrittenInit, HasRebuiltInit);
}
-CXXDefaultArgExpr *CXXDefaultArgExpr::Create(const ASTContext &C,
- SourceLocation Loc,
- ParmVarDecl *Param,
- Expr *RewrittenExpr,
- DeclContext *UsedContext) {
+CXXDefaultArgExpr *
+CXXDefaultArgExpr::Create(const ASTContext &C, SourceLocation Loc,
+ ParmVarDecl *Param, DeclContext *UsedContext,
+ Expr *RewrittenExpr, bool HasRebuiltInit) {
size_t Size = totalSizeToAlloc<Expr *>(RewrittenExpr != nullptr);
auto *Mem = C.Allocate(Size, alignof(CXXDefaultArgExpr));
- return new (Mem) CXXDefaultArgExpr(CXXDefaultArgExprClass, Loc, Param,
- RewrittenExpr, UsedContext);
+ return new (Mem)
+ CXXDefaultArgExpr(CXXDefaultArgExprClass, Loc, Param, UsedContext,
+ RewrittenExpr, HasRebuiltInit);
}
Expr *CXXDefaultArgExpr::getExpr() {
@@ -1044,7 +1046,7 @@ Expr *CXXDefaultArgExpr::getAdjustedRewrittenExpr() {
CXXDefaultInitExpr::CXXDefaultInitExpr(const ASTContext &Ctx,
SourceLocation Loc, FieldDecl *Field,
QualType Ty, DeclContext *UsedContext,
- Expr *RewrittenInitExpr)
+ Expr *RewrittenInitExpr, bool hasRebuiltInit)
: Expr(CXXDefaultInitExprClass, Ty.getNonLValueExprType(Ctx),
Ty->isLValueReferenceType() ? VK_LValue
: Ty->isRValueReferenceType() ? VK_XValue
@@ -1053,6 +1055,7 @@ CXXDefaultInitExpr::CXXDefaultInitExpr(const ASTContext &Ctx,
Field(Field), UsedContext(UsedContext) {
CXXDefaultInitExprBits.Loc = Loc;
CXXDefaultInitExprBits.HasRewrittenInit = RewrittenInitExpr != nullptr;
+ CXXDefaultInitExprBits.HasRebuiltInit = hasRebuiltInit;
if (CXXDefaultInitExprBits.HasRewrittenInit)
*getTrailingObjects<Expr *>() = RewrittenInitExpr;
@@ -1063,22 +1066,24 @@ CXXDefaultInitExpr::CXXDefaultInitExpr(const ASTContext &Ctx,
}
CXXDefaultInitExpr *CXXDefaultInitExpr::CreateEmpty(const ASTContext &C,
- bool HasRewrittenInit) {
+ bool HasRewrittenInit,
+ bool HasRebuiltInit) {
size_t Size = totalSizeToAlloc<Expr *>(HasRewrittenInit);
auto *Mem = C.Allocate(Size, alignof(CXXDefaultInitExpr));
- return new (Mem) CXXDefaultInitExpr(EmptyShell(), HasRewrittenInit);
+ return new (Mem)
+ CXXDefaultInitExpr(EmptyShell(), HasRewrittenInit, HasRebuiltInit);
}
-CXXDefaultInitExpr *CXXDefaultInitExpr::Create(const ASTContext &Ctx,
- SourceLocation Loc,
- FieldDecl *Field,
- DeclContext *UsedContext,
- Expr *RewrittenInitExpr) {
+CXXDefaultInitExpr *
+CXXDefaultInitExpr::Create(const ASTContext &Ctx, SourceLocation Loc,
+ FieldDecl *Field, DeclContext *UsedContext,
+ Expr *RewrittenInitExpr, bool HasRebuiltInit) {
size_t Size = totalSizeToAlloc<Expr *>(RewrittenInitExpr != nullptr);
auto *Mem = Ctx.Allocate(Size, alignof(CXXDefaultInitExpr));
- return new (Mem) CXXDefaultInitExpr(Ctx, Loc, Field, Field->getType(),
- UsedContext, RewrittenInitExpr);
+ return new (Mem)
+ CXXDefaultInitExpr(Ctx, Loc, Field, Field->getType(), UsedContext,
+ RewrittenInitExpr, HasRebuiltInit);
}
Expr *CXXDefaultInitExpr::getExpr() {
diff --git a/clang/lib/AST/ParentMap.cpp b/clang/lib/AST/ParentMap.cpp
index fd749b02b758c9..61eb2ec0e1cb94 100644
--- a/clang/lib/AST/ParentMap.cpp
+++ b/clang/lib/AST/ParentMap.cpp
@@ -13,6 +13,7 @@
#include "clang/AST/ParentMap.h"
#include "clang/AST/Decl.h"
#include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
#include "clang/AST/StmtObjC.h"
#include "llvm/ADT/DenseMap.h"
@@ -96,6 +97,22 @@ static void BuildParentMap(MapTy& M, Stmt* S,
BuildParentMap(M, SubStmt, OVMode);
}
break;
+ case Stmt::CXXDefaultArgExprClass:
+ if (auto *Arg = dyn_cast<CXXDefaultArgExpr>(S)) {
+ if (Arg->hasRebuiltInit()) {
+ M[Arg->getExpr()] = S;
+ BuildParentMap(M, Arg->getExpr(), OVMode);
+ }
+ }
+ break;
+ case Stmt::CXXDefaultInitExprClass:
+ if (auto *Init = dyn_cast<CXXDefaultInitExpr>(S)) {
+ if (Init->hasRebuiltInit()) {
+ M[Init->getExpr()] = S;
+ BuildParentMap(M, Init->getExpr(), OVMode);
+ }
+ }
+ break;
default:
for (Stmt *SubStmt : S->children()) {
if (SubStmt) {
diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index 304bbb2b422c61..82c283335b0ff4 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -556,6 +556,10 @@ class CFGBuilder {
private:
// Visitors to walk an AST and construct the CFG.
+ CFGBlock *VisitCXXDefaultArgExpr(CXXDefaultArgExpr *Default,
+ AddStmtChoice asc);
+ CFGBlock *VisitCXXDefaultInitExpr(CXXDefaultInitExpr *Default,
+ AddStmtChoice asc);
CFGBlock *VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc);
CFGBlock *VisitAddrLabelExpr(AddrLabelExpr *A, AddStmtChoice asc);
CFGBlock *VisitAttributedStmt(AttributedStmt *A, AddStmtChoice asc);
@@ -2261,16 +2265,10 @@ CFGBlock *CFGBuilder::Visit(Stmt * S, AddStmtChoice asc,
asc, ExternallyDestructed);
case Stmt::CXXDefaultArgExprClass:
+ return VisitCXXDefaultArgExpr(cast<CXXDefaultArgExpr>(S), asc);
+
case Stmt::CXXDefaultInitExprClass:
- // FIXME: The expression inside a CXXDefaultArgExpr is owned by the
- // called function's declaration, not by the caller. If we simply add
- // this expression to the CFG, we could end up with the same Expr
- // appearing multiple times (PR13385).
- //
- // It's likewise possible for multiple CXXDefaultInitExprs for the same
- // expression to be used in the same function (through aggregate
- // initialization).
- return VisitStmt(S, asc);
+ return VisitCXXDefaultInitExpr(cast<CXXDefaultInitExpr>(S), asc);
case Stmt::CXXBindTemporaryExprClass:
return VisitCXXBindTemporaryExpr(cast<CXXBindTemporaryExpr>(S), asc);
@@ -2440,6 +2438,40 @@ CFGBlock *CFGBuilder::VisitChildren(Stmt *S) {
return B;
}
+CFGBlock *CFGBuilder::VisitCXXDefaultArgExpr(CXXDefaultArgExpr *Arg,
+ AddStmtChoice asc) {
+ if (Arg->hasRebuiltInit()) {
+ if (asc.alwaysAdd(*this, Arg)) {
+ autoCreateBlock();
+ appendStmt(Block, Arg);
+ }
+ return VisitStmt(Arg->getExpr(), asc);
+ }
+
+ // We can't add the default argument if it's not rewritten because the
+ // expression inside a CXXDefaultArgExpr is owned by the called function's
+ // declaration, not by the caller, we could end up with the same expression
+ // appearing multiple times.
+ return VisitStmt(Arg, asc);
+}
+
+CFGBlock *CFGBuilder::VisitCXXDefaultInitExpr(CXXDefaultInitExpr *Init,
+ AddStmtChoice asc) {
+ if (Init->hasRebuiltInit()) {
+ if (asc.alwaysAdd(*this, Init)) {
+ autoCreateBlock();
+ appendStmt(Block, Init);
+ }
+ return VisitStmt(Init->getExpr(), asc);
+ }
+
+ // We can't add the default initializer if it's not rewritten because multiple
+ // CXXDefaultInitExprs for the same sub-expression to be used in the same
+ // function (through aggregate initialization). we could end up with the same
+ // expression appearing multiple times.
+ return VisitStmt(Init, asc);
+}
+
CFGBlock *CFGBuilder::VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc) {
if (asc.alwaysAdd(*this, ILE)) {
autoCreateBlock();
diff --git a/clang/lib/Analysis/ReachableCode.cpp b/clang/lib/Analysis/ReachableCode.cpp
index dd81c8e0a3d543..9f0527317eca95 100644
--- a/clang/lib/Analysis/ReachableCode.cpp
+++ b/clang/lib/Analysis/ReachableCode.cpp
@@ -454,11 +454,10 @@ bool DeadCodeScan::isDeadCodeRoot(const clang::CFGBlock *Block) {
return isDeadRoot;
}
-// Check if the given `DeadStmt` is a coroutine statement and is a substmt of
-// the coroutine statement. `Block` is the CFGBlock containing the `DeadStmt`.
-static bool isInCoroutineStmt(const Stmt *DeadStmt, const CFGBlock *Block) {
+template <class... Ts>
+static bool isDeadStmtIn(const Stmt *DeadStmt, const CFGBlock *Block) {
// The coroutine statement, co_return, co_await, or co_yield.
- const Stmt *CoroStmt = nullptr;
+ const Stmt *TargetStmt = nullptr;
// Find the first coroutine statement after the DeadStmt in the block.
bool AfterDeadStmt = false;
for (CFGBlock::const_iterator I = Block->begin(), E = Block->end(); I != E;
@@ -467,32 +466,29 @@ static bool isInCoroutineStmt(const Stmt *DeadStmt, const CFGBlock *Block) {
const Stmt *S = CS->getStmt();
if (S == DeadStmt)
AfterDeadStmt = true;
- if (AfterDeadStmt &&
- // For simplicity, we only check simple coroutine statements.
- (llvm::isa<CoreturnStmt>(S) || llvm::isa<CoroutineSuspendExpr>(S))) {
- CoroStmt = S;
+ if (AfterDeadStmt && llvm::isa<Ts...>(S)) {
+ TargetStmt = S;
break;
}
}
- if (!CoroStmt)
+ if (!TargetStmt)
return false;
struct Checker : DynamicRecursiveASTVisitor {
const Stmt *DeadStmt;
- bool CoroutineSubStmt = false;
+ bool IsSubStmtOfTargetStmt = false;
Checker(const Stmt *S) : DeadStmt(S) {
- // Statements captured in the CFG can be implicit.
ShouldVisitImplicitCode = true;
}
bool VisitStmt(Stmt *S) override {
if (S == DeadStmt)
- CoroutineSubStmt = true;
+ IsSubStmtOfTargetStmt = true;
return true;
}
};
Checker checker(DeadStmt);
- checker.TraverseStmt(const_cast<Stmt *>(CoroStmt));
- return checker.CoroutineSubStmt;
+ checker.TraverseStmt(const_cast<Stmt *>(TargetStmt));
+ return checker.IsSubStmtOfTargetStmt;
}
static bool isValidDeadStmt(const Stmt *S, const clang::CFGBlock *Block) {
@@ -503,7 +499,12 @@ static bool isValidDeadStmt(const Stmt *S, const clang::CFGBlock *Block) {
// Coroutine statements are never considered dead statements, because removing
// them may change the function semantic if it is the only coroutine statement
// of the coroutine.
- return !isInCoroutineStmt(S, Block);
+ //
+ // If the dead stmt is a sub-stmt of CXXDefaultInitExpr and CXXDefaultArgExpr,
+ // we would rather expect to find CXXDefaultInitExpr and CXXDefaultArgExpr as
+ // a valid dead stmt.
+ return !isDeadStmtIn<CoreturnStmt, CoroutineSuspendExpr, CXXDefaultArgExpr,
+ CXXDefaultInitExpr>(S, Block);
}
const Stmt *DeadCodeScan::findDeadCode(const clang::CFGBlock *Block) {
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 6c7472ce92703b..baea4ffef1799e 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -5489,6 +5489,7 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
bool NestedDefaultChecking = isCheckingDefaultArgumentOrInitializer();
bool NeedRebuild = needsRebuildOfDefaultArgOrInit();
+ bool HasRebuiltInit = false;
std::optional<ExpressionEvaluationContextRecord::InitializationContext>
InitializationContext =
OutermostDeclarationWithDelayedImmediateInvocations();
@@ -5546,6 +5547,7 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
if (Res.isInvalid())
return ExprError();
Init = Res.get();
+ HasRebuiltInit = true;
}
}
@@ -5555,7 +5557,8 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
return ExprError();
return CXXDefaultArgExpr::Create(Context, InitializationContext->Loc, Param,
- Init, InitializationContext->Context);
+ InitializationContext->Context, Init,
+ HasRebuiltInit);
}
static FieldDecl *FindFieldDeclInstantiationPattern(const ASTContext &Ctx,
@@ -5597,6 +5600,7 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
bool NestedDefaultChecking = isCheckingDefaultArgumentOrInitializer();
bool NeedRebuild = needsRebuildOfDefaultArgOrInit();
+ bool HasRebuiltInit = false;
EnterExpressionEvaluationContext EvalContext(
*this, ExpressionEvaluationContext::PotentiallyEvaluated, Field);
@@ -5658,6 +5662,7 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocat...
[truncated]
|
|
@llvm/pr-subscribers-clang Author: None (yronglin) ChangesClang now support the following:
But CFG and ExprEngine need to be updated to address this change. This PR reapply #91879.
Patch is 30.04 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/117437.diff 14 Files Affected:
diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h
index 696a574833dad2..99680537a3f777 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -1277,7 +1277,8 @@ class CXXDefaultArgExpr final
DeclContext *UsedContext;
CXXDefaultArgExpr(StmtClass SC, SourceLocation Loc, ParmVarDecl *Param,
- Expr *RewrittenExpr, DeclContext *UsedContext)
+ DeclContext *UsedContext, Expr *RewrittenExpr,
+ bool HasRebuiltInit)
: Expr(SC,
Param->hasUnparsedDefaultArg()
? Param->getType().getNonReferenceType()
@@ -1287,25 +1288,27 @@ class CXXDefaultArgExpr final
Param(Param), UsedContext(UsedContext) {
CXXDefaultArgExprBits.Loc = Loc;
CXXDefaultArgExprBits.HasRewrittenInit = RewrittenExpr != nullptr;
+ CXXDefaultArgExprBits.HasRebuiltInit = HasRebuiltInit;
if (RewrittenExpr)
*getTrailingObjects<Expr *>() = RewrittenExpr;
setDependence(computeDependence(this));
}
- CXXDefaultArgExpr(EmptyShell Empty, bool HasRewrittenInit)
+ CXXDefaultArgExpr(EmptyShell Empty, bool HasRewrittenInit, bool HasRebuiltInit)
: Expr(CXXDefaultArgExprClass, Empty) {
CXXDefaultArgExprBits.HasRewrittenInit = HasRewrittenInit;
+ CXXDefaultArgExprBits.HasRebuiltInit = HasRebuiltInit;
}
public:
static CXXDefaultArgExpr *CreateEmpty(const ASTContext &C,
- bool HasRewrittenInit);
+ bool HasRewrittenInit, bool HasRebuiltInit);
// \p Param is the parameter whose default argument is used by this
// expression.
static CXXDefaultArgExpr *Create(const ASTContext &C, SourceLocation Loc,
- ParmVarDecl *Param, Expr *RewrittenExpr,
- DeclContext *UsedContext);
+ ParmVarDecl *Param, DeclContext *UsedContext,
+ Expr *RewrittenExpr, bool HasRebuiltInit);
// Retrieve the parameter that the argument was created from.
const ParmVarDecl *getParam() const { return Param; }
ParmVarDecl *getParam() { return Param; }
@@ -1314,6 +1317,10 @@ class CXXDefaultArgExpr final
return CXXDefaultArgExprBits.HasRewrittenInit;
}
+ bool hasRebuiltInit() const {
+ return CXXDefaultArgExprBits.HasRebuiltInit;
+ }
+
// Retrieve the argument to the function call.
Expr *getExpr();
const Expr *getExpr() const {
@@ -1385,26 +1392,31 @@ class CXXDefaultInitExpr final
CXXDefaultInitExpr(const ASTContext &Ctx, SourceLocation Loc,
FieldDecl *Field, QualType Ty, DeclContext *UsedContext,
- Expr *RewrittenInitExpr);
+ Expr *RewrittenInitExpr, bool HasRebuiltInit);
- CXXDefaultInitExpr(EmptyShell Empty, bool HasRewrittenInit)
+ CXXDefaultInitExpr(EmptyShell Empty, bool HasRewrittenInit, bool HasRebuiltInit)
: Expr(CXXDefaultInitExprClass, Empty) {
CXXDefaultInitExprBits.HasRewrittenInit = HasRewrittenInit;
+ CXXDefaultInitExprBits.HasRebuiltInit = HasRebuiltInit;
}
public:
static CXXDefaultInitExpr *CreateEmpty(const ASTContext &C,
- bool HasRewrittenInit);
+ bool HasRewrittenInit, bool HasRebuiltInit);
/// \p Field is the non-static data member whose default initializer is used
/// by this expression.
static CXXDefaultInitExpr *Create(const ASTContext &Ctx, SourceLocation Loc,
FieldDecl *Field, DeclContext *UsedContext,
- Expr *RewrittenInitExpr);
+ Expr *RewrittenInitExpr, bool HasRebuiltInit);
bool hasRewrittenInit() const {
return CXXDefaultInitExprBits.HasRewrittenInit;
}
+ bool hasRebuiltInit() const {
+ return CXXDefaultInitExprBits.HasRebuiltInit;
+ }
+
/// Get the field whose initializer will be used.
FieldDecl *getField() { return Field; }
const FieldDecl *getField() const { return Field; }
diff --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h
index 83fafbabb1d460..1b2dbcbaa09219 100644
--- a/clang/include/clang/AST/Stmt.h
+++ b/clang/include/clang/AST/Stmt.h
@@ -839,6 +839,10 @@ class alignas(void *) Stmt {
LLVM_PREFERRED_TYPE(bool)
unsigned HasRewrittenInit : 1;
+ /// Whether this CXXDefaultArgExpr rebuild its argument and stores a copy.
+ LLVM_PREFERRED_TYPE(bool)
+ unsigned HasRebuiltInit : 1;
+
/// The location where the default argument expression was used.
SourceLocation Loc;
};
@@ -850,11 +854,16 @@ class alignas(void *) Stmt {
LLVM_PREFERRED_TYPE(ExprBitfields)
unsigned : NumExprBits;
- /// Whether this CXXDefaultInitExprBitfields rewrote its argument and stores
- /// a copy.
+ /// Whether this CXXDefaultInitExpr rewrote its argument and stores
+ /// a copy, unlike HasRebuiltInit, when this flag is true, the argument may
+ /// be partial rebuilt.
LLVM_PREFERRED_TYPE(bool)
unsigned HasRewrittenInit : 1;
+ /// Whether this CXXDefaultInitExpr fully rebuild its argument and stores a copy.
+ LLVM_PREFERRED_TYPE(bool)
+ unsigned HasRebuiltInit : 1;
+
/// The location where the default initializer expression was used.
SourceLocation Loc;
};
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index baed1416635432..e2126f5fd29a47 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -8154,8 +8154,8 @@ ExpectedStmt ASTNodeImporter::VisitCXXDefaultArgExpr(CXXDefaultArgExpr *E) {
RewrittenInit = ExprOrErr.get();
}
return CXXDefaultArgExpr::Create(Importer.getToContext(), *ToUsedLocOrErr,
- *ToParamOrErr, RewrittenInit,
- *UsedContextOrErr);
+ *ToParamOrErr, *UsedContextOrErr,
+ RewrittenInit, E->hasRebuiltInit());
}
ExpectedStmt
@@ -8863,7 +8863,8 @@ ExpectedStmt ASTNodeImporter::VisitCXXDefaultInitExpr(CXXDefaultInitExpr *E) {
}
return CXXDefaultInitExpr::Create(Importer.getToContext(), *ToBeginLocOrErr,
- ToField, *UsedContextOrErr, RewrittenInit);
+ ToField, *UsedContextOrErr, RewrittenInit,
+ E->hasRebuiltInit());
}
ExpectedStmt ASTNodeImporter::VisitCXXNamedCastExpr(CXXNamedCastExpr *E) {
diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index 0ce129de85f03f..6b6507d4b5ebeb 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -1009,21 +1009,23 @@ const IdentifierInfo *UserDefinedLiteral::getUDSuffix() const {
}
CXXDefaultArgExpr *CXXDefaultArgExpr::CreateEmpty(const ASTContext &C,
- bool HasRewrittenInit) {
+ bool HasRewrittenInit,
+ bool HasRebuiltInit) {
size_t Size = totalSizeToAlloc<Expr *>(HasRewrittenInit);
auto *Mem = C.Allocate(Size, alignof(CXXDefaultArgExpr));
- return new (Mem) CXXDefaultArgExpr(EmptyShell(), HasRewrittenInit);
+ return new (Mem)
+ CXXDefaultArgExpr(EmptyShell(), HasRewrittenInit, HasRebuiltInit);
}
-CXXDefaultArgExpr *CXXDefaultArgExpr::Create(const ASTContext &C,
- SourceLocation Loc,
- ParmVarDecl *Param,
- Expr *RewrittenExpr,
- DeclContext *UsedContext) {
+CXXDefaultArgExpr *
+CXXDefaultArgExpr::Create(const ASTContext &C, SourceLocation Loc,
+ ParmVarDecl *Param, DeclContext *UsedContext,
+ Expr *RewrittenExpr, bool HasRebuiltInit) {
size_t Size = totalSizeToAlloc<Expr *>(RewrittenExpr != nullptr);
auto *Mem = C.Allocate(Size, alignof(CXXDefaultArgExpr));
- return new (Mem) CXXDefaultArgExpr(CXXDefaultArgExprClass, Loc, Param,
- RewrittenExpr, UsedContext);
+ return new (Mem)
+ CXXDefaultArgExpr(CXXDefaultArgExprClass, Loc, Param, UsedContext,
+ RewrittenExpr, HasRebuiltInit);
}
Expr *CXXDefaultArgExpr::getExpr() {
@@ -1044,7 +1046,7 @@ Expr *CXXDefaultArgExpr::getAdjustedRewrittenExpr() {
CXXDefaultInitExpr::CXXDefaultInitExpr(const ASTContext &Ctx,
SourceLocation Loc, FieldDecl *Field,
QualType Ty, DeclContext *UsedContext,
- Expr *RewrittenInitExpr)
+ Expr *RewrittenInitExpr, bool hasRebuiltInit)
: Expr(CXXDefaultInitExprClass, Ty.getNonLValueExprType(Ctx),
Ty->isLValueReferenceType() ? VK_LValue
: Ty->isRValueReferenceType() ? VK_XValue
@@ -1053,6 +1055,7 @@ CXXDefaultInitExpr::CXXDefaultInitExpr(const ASTContext &Ctx,
Field(Field), UsedContext(UsedContext) {
CXXDefaultInitExprBits.Loc = Loc;
CXXDefaultInitExprBits.HasRewrittenInit = RewrittenInitExpr != nullptr;
+ CXXDefaultInitExprBits.HasRebuiltInit = hasRebuiltInit;
if (CXXDefaultInitExprBits.HasRewrittenInit)
*getTrailingObjects<Expr *>() = RewrittenInitExpr;
@@ -1063,22 +1066,24 @@ CXXDefaultInitExpr::CXXDefaultInitExpr(const ASTContext &Ctx,
}
CXXDefaultInitExpr *CXXDefaultInitExpr::CreateEmpty(const ASTContext &C,
- bool HasRewrittenInit) {
+ bool HasRewrittenInit,
+ bool HasRebuiltInit) {
size_t Size = totalSizeToAlloc<Expr *>(HasRewrittenInit);
auto *Mem = C.Allocate(Size, alignof(CXXDefaultInitExpr));
- return new (Mem) CXXDefaultInitExpr(EmptyShell(), HasRewrittenInit);
+ return new (Mem)
+ CXXDefaultInitExpr(EmptyShell(), HasRewrittenInit, HasRebuiltInit);
}
-CXXDefaultInitExpr *CXXDefaultInitExpr::Create(const ASTContext &Ctx,
- SourceLocation Loc,
- FieldDecl *Field,
- DeclContext *UsedContext,
- Expr *RewrittenInitExpr) {
+CXXDefaultInitExpr *
+CXXDefaultInitExpr::Create(const ASTContext &Ctx, SourceLocation Loc,
+ FieldDecl *Field, DeclContext *UsedContext,
+ Expr *RewrittenInitExpr, bool HasRebuiltInit) {
size_t Size = totalSizeToAlloc<Expr *>(RewrittenInitExpr != nullptr);
auto *Mem = Ctx.Allocate(Size, alignof(CXXDefaultInitExpr));
- return new (Mem) CXXDefaultInitExpr(Ctx, Loc, Field, Field->getType(),
- UsedContext, RewrittenInitExpr);
+ return new (Mem)
+ CXXDefaultInitExpr(Ctx, Loc, Field, Field->getType(), UsedContext,
+ RewrittenInitExpr, HasRebuiltInit);
}
Expr *CXXDefaultInitExpr::getExpr() {
diff --git a/clang/lib/AST/ParentMap.cpp b/clang/lib/AST/ParentMap.cpp
index fd749b02b758c9..61eb2ec0e1cb94 100644
--- a/clang/lib/AST/ParentMap.cpp
+++ b/clang/lib/AST/ParentMap.cpp
@@ -13,6 +13,7 @@
#include "clang/AST/ParentMap.h"
#include "clang/AST/Decl.h"
#include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
#include "clang/AST/StmtObjC.h"
#include "llvm/ADT/DenseMap.h"
@@ -96,6 +97,22 @@ static void BuildParentMap(MapTy& M, Stmt* S,
BuildParentMap(M, SubStmt, OVMode);
}
break;
+ case Stmt::CXXDefaultArgExprClass:
+ if (auto *Arg = dyn_cast<CXXDefaultArgExpr>(S)) {
+ if (Arg->hasRebuiltInit()) {
+ M[Arg->getExpr()] = S;
+ BuildParentMap(M, Arg->getExpr(), OVMode);
+ }
+ }
+ break;
+ case Stmt::CXXDefaultInitExprClass:
+ if (auto *Init = dyn_cast<CXXDefaultInitExpr>(S)) {
+ if (Init->hasRebuiltInit()) {
+ M[Init->getExpr()] = S;
+ BuildParentMap(M, Init->getExpr(), OVMode);
+ }
+ }
+ break;
default:
for (Stmt *SubStmt : S->children()) {
if (SubStmt) {
diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index 304bbb2b422c61..82c283335b0ff4 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -556,6 +556,10 @@ class CFGBuilder {
private:
// Visitors to walk an AST and construct the CFG.
+ CFGBlock *VisitCXXDefaultArgExpr(CXXDefaultArgExpr *Default,
+ AddStmtChoice asc);
+ CFGBlock *VisitCXXDefaultInitExpr(CXXDefaultInitExpr *Default,
+ AddStmtChoice asc);
CFGBlock *VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc);
CFGBlock *VisitAddrLabelExpr(AddrLabelExpr *A, AddStmtChoice asc);
CFGBlock *VisitAttributedStmt(AttributedStmt *A, AddStmtChoice asc);
@@ -2261,16 +2265,10 @@ CFGBlock *CFGBuilder::Visit(Stmt * S, AddStmtChoice asc,
asc, ExternallyDestructed);
case Stmt::CXXDefaultArgExprClass:
+ return VisitCXXDefaultArgExpr(cast<CXXDefaultArgExpr>(S), asc);
+
case Stmt::CXXDefaultInitExprClass:
- // FIXME: The expression inside a CXXDefaultArgExpr is owned by the
- // called function's declaration, not by the caller. If we simply add
- // this expression to the CFG, we could end up with the same Expr
- // appearing multiple times (PR13385).
- //
- // It's likewise possible for multiple CXXDefaultInitExprs for the same
- // expression to be used in the same function (through aggregate
- // initialization).
- return VisitStmt(S, asc);
+ return VisitCXXDefaultInitExpr(cast<CXXDefaultInitExpr>(S), asc);
case Stmt::CXXBindTemporaryExprClass:
return VisitCXXBindTemporaryExpr(cast<CXXBindTemporaryExpr>(S), asc);
@@ -2440,6 +2438,40 @@ CFGBlock *CFGBuilder::VisitChildren(Stmt *S) {
return B;
}
+CFGBlock *CFGBuilder::VisitCXXDefaultArgExpr(CXXDefaultArgExpr *Arg,
+ AddStmtChoice asc) {
+ if (Arg->hasRebuiltInit()) {
+ if (asc.alwaysAdd(*this, Arg)) {
+ autoCreateBlock();
+ appendStmt(Block, Arg);
+ }
+ return VisitStmt(Arg->getExpr(), asc);
+ }
+
+ // We can't add the default argument if it's not rewritten because the
+ // expression inside a CXXDefaultArgExpr is owned by the called function's
+ // declaration, not by the caller, we could end up with the same expression
+ // appearing multiple times.
+ return VisitStmt(Arg, asc);
+}
+
+CFGBlock *CFGBuilder::VisitCXXDefaultInitExpr(CXXDefaultInitExpr *Init,
+ AddStmtChoice asc) {
+ if (Init->hasRebuiltInit()) {
+ if (asc.alwaysAdd(*this, Init)) {
+ autoCreateBlock();
+ appendStmt(Block, Init);
+ }
+ return VisitStmt(Init->getExpr(), asc);
+ }
+
+ // We can't add the default initializer if it's not rewritten because multiple
+ // CXXDefaultInitExprs for the same sub-expression to be used in the same
+ // function (through aggregate initialization). we could end up with the same
+ // expression appearing multiple times.
+ return VisitStmt(Init, asc);
+}
+
CFGBlock *CFGBuilder::VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc) {
if (asc.alwaysAdd(*this, ILE)) {
autoCreateBlock();
diff --git a/clang/lib/Analysis/ReachableCode.cpp b/clang/lib/Analysis/ReachableCode.cpp
index dd81c8e0a3d543..9f0527317eca95 100644
--- a/clang/lib/Analysis/ReachableCode.cpp
+++ b/clang/lib/Analysis/ReachableCode.cpp
@@ -454,11 +454,10 @@ bool DeadCodeScan::isDeadCodeRoot(const clang::CFGBlock *Block) {
return isDeadRoot;
}
-// Check if the given `DeadStmt` is a coroutine statement and is a substmt of
-// the coroutine statement. `Block` is the CFGBlock containing the `DeadStmt`.
-static bool isInCoroutineStmt(const Stmt *DeadStmt, const CFGBlock *Block) {
+template <class... Ts>
+static bool isDeadStmtIn(const Stmt *DeadStmt, const CFGBlock *Block) {
// The coroutine statement, co_return, co_await, or co_yield.
- const Stmt *CoroStmt = nullptr;
+ const Stmt *TargetStmt = nullptr;
// Find the first coroutine statement after the DeadStmt in the block.
bool AfterDeadStmt = false;
for (CFGBlock::const_iterator I = Block->begin(), E = Block->end(); I != E;
@@ -467,32 +466,29 @@ static bool isInCoroutineStmt(const Stmt *DeadStmt, const CFGBlock *Block) {
const Stmt *S = CS->getStmt();
if (S == DeadStmt)
AfterDeadStmt = true;
- if (AfterDeadStmt &&
- // For simplicity, we only check simple coroutine statements.
- (llvm::isa<CoreturnStmt>(S) || llvm::isa<CoroutineSuspendExpr>(S))) {
- CoroStmt = S;
+ if (AfterDeadStmt && llvm::isa<Ts...>(S)) {
+ TargetStmt = S;
break;
}
}
- if (!CoroStmt)
+ if (!TargetStmt)
return false;
struct Checker : DynamicRecursiveASTVisitor {
const Stmt *DeadStmt;
- bool CoroutineSubStmt = false;
+ bool IsSubStmtOfTargetStmt = false;
Checker(const Stmt *S) : DeadStmt(S) {
- // Statements captured in the CFG can be implicit.
ShouldVisitImplicitCode = true;
}
bool VisitStmt(Stmt *S) override {
if (S == DeadStmt)
- CoroutineSubStmt = true;
+ IsSubStmtOfTargetStmt = true;
return true;
}
};
Checker checker(DeadStmt);
- checker.TraverseStmt(const_cast<Stmt *>(CoroStmt));
- return checker.CoroutineSubStmt;
+ checker.TraverseStmt(const_cast<Stmt *>(TargetStmt));
+ return checker.IsSubStmtOfTargetStmt;
}
static bool isValidDeadStmt(const Stmt *S, const clang::CFGBlock *Block) {
@@ -503,7 +499,12 @@ static bool isValidDeadStmt(const Stmt *S, const clang::CFGBlock *Block) {
// Coroutine statements are never considered dead statements, because removing
// them may change the function semantic if it is the only coroutine statement
// of the coroutine.
- return !isInCoroutineStmt(S, Block);
+ //
+ // If the dead stmt is a sub-stmt of CXXDefaultInitExpr and CXXDefaultArgExpr,
+ // we would rather expect to find CXXDefaultInitExpr and CXXDefaultArgExpr as
+ // a valid dead stmt.
+ return !isDeadStmtIn<CoreturnStmt, CoroutineSuspendExpr, CXXDefaultArgExpr,
+ CXXDefaultInitExpr>(S, Block);
}
const Stmt *DeadCodeScan::findDeadCode(const clang::CFGBlock *Block) {
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 6c7472ce92703b..baea4ffef1799e 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -5489,6 +5489,7 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
bool NestedDefaultChecking = isCheckingDefaultArgumentOrInitializer();
bool NeedRebuild = needsRebuildOfDefaultArgOrInit();
+ bool HasRebuiltInit = false;
std::optional<ExpressionEvaluationContextRecord::InitializationContext>
InitializationContext =
OutermostDeclarationWithDelayedImmediateInvocations();
@@ -5546,6 +5547,7 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
if (Res.isInvalid())
return ExprError();
Init = Res.get();
+ HasRebuiltInit = true;
}
}
@@ -5555,7 +5557,8 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
return ExprError();
return CXXDefaultArgExpr::Create(Context, InitializationContext->Loc, Param,
- Init, InitializationContext->Context);
+ InitializationContext->Context, Init,
+ HasRebuiltInit);
}
static FieldDecl *FindFieldDeclInstantiationPattern(const ASTContext &Ctx,
@@ -5597,6 +5600,7 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
bool NestedDefaultChecking = isCheckingDefaultArgumentOrInitializer();
bool NeedRebuild = needsRebuildOfDefaultArgOrInit();
+ bool HasRebuiltInit = false;
EnterExpressionEvaluationContext EvalContext(
*this, ExpressionEvaluationContext::PotentiallyEvaluated, Field);
@@ -5658,6 +5662,7 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocat...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
… expression Signed-off-by: yronglin <[email protected]>
yronglin
force-pushed
the
check_default_init_in_cfg
branch
from
75b4765
to
6af171c
Compare
|
Thanks for the review! @cor3ntin Do you have any comments on the changes of |
cor3ntin
reviewed
Nov 27, 2024
Comment on lines
+857
to
+859
| /// Whether this CXXDefaultInitExpr rewrote its argument and stores | ||
| /// a copy, unlike HasRebuiltInit, when this flag is true, the argument may | ||
| /// be partial rebuilt. |
There was a problem hiding this comment.
Do we really need both? Can you explain why? What do you mean by partially?
There was a problem hiding this comment.
For example:
struct A {};
struct B {
A a = A{};
};
B c{}, d{};
|-CXXRecordDecl 0x13f1289d8 <line:2:1, line:4:1> line:2:8 referenced struct B definition
| |-DefinitionData pass_in_registers aggregate standard_layout trivially_copyable literal has_constexpr_non_copy_move_ctor can_const_default_init
| | |-DefaultConstructor exists non_trivial constexpr needs_implicit defaulted_is_constexpr
| | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
| | |-MoveConstructor exists simple trivial needs_implicit
| | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
| | |-MoveAssignment exists simple trivial needs_implicit
| | `-Destructor simple irrelevant trivial constexpr needs_implicit
| |-CXXRecordDecl 0x13f128b10 <col:1, col:8> col:8 implicit struct B
| `-FieldDecl 0x13f149808 <line:3:5, col:13> col:7 a 'A'
| `-CXXFunctionalCastExpr 0x13f149aa0 <col:11, col:13> 'A' functional cast to A <NoOp>
| `-InitListExpr 0x13f1498f0 <col:12, col:13> 'A'
|-VarDecl 0x13f149b18 <line:6:1, col:5> col:3 c 'B' listinit
| `-InitListExpr 0x13f149c10 <col:4, col:5> 'B'
| `-CXXDefaultInitExpr 0x13f149c58 <col:5> 'A' has rewritten init
| `-CXXFunctionalCastExpr 0x13f149aa0 <line:3:11, col:13> 'A' functional cast to A <NoOp>
| `-InitListExpr 0x13f1498f0 <col:12, col:13> 'A'
`-VarDecl 0x13f149ea0 <line:6:1, col:10> col:8 d 'B' listinit
`-InitListExpr 0x13f149f48 <col:9, col:10> 'B'
`-CXXDefaultInitExpr 0x13f149f90 <col:10> 'A' has rewritten init
`-CXXFunctionalCastExpr 0x13f149aa0 <line:3:11, col:13> 'A' functional cast to A <NoOp>
`-InitListExpr 0x13f1498f0 <col:12, col:13> 'A'
The sub-expr in CXXDefaultInitExpr will appears multiple times in the AST. We can't add the default argument if it's not rewritten because we could end up with the same expression appearing multiple times.
Comment on lines
+857
to
+859
| /// Whether this CXXDefaultInitExpr rewrote its argument and stores | ||
| /// a copy, unlike HasRebuiltInit, when this flag is true, the argument may | ||
| /// be partial rebuilt. |
There was a problem hiding this comment.
Suggested change
| /// Whether this CXXDefaultInitExpr rewrote its argument and stores | |
| /// a copy, unlike HasRebuiltInit, when this flag is true, the argument may | |
| /// be partial rebuilt. | |
| /// Whether this CXXDefaultInitExpr rewrote its argument and stores | |
| /// a copy, unlike HasRebuiltInit, when this flag is true, the argument may | |
| /// be partially rebuilt. |
cor3ntin
requested changes
Nov 27, 2024
There was a problem hiding this comment.
I'd like more time to think about this change and discuss it with @AaronBallman / @erichkeane
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.
Clang now support the following:
But CFG and ExprEngine need to be updated to address this change.
This PR reapply #91879.
HasRebuiltInitflag inCXXDefaultInitExprandCXXDefaultArgExpr.-Wunreachable-codeprints incorrect source range if a constructor has a default value set to a builtin #93725.