-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[Analyzer][CFG] Correctly handle rebuilt default arg and default init expression #117437
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
Conversation
@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. |
75b4765
to
6af171c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Thanks for the review! @cor3ntin Do you have any comments on the changes of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like more time to think about this change and discuss it with @AaronBallman / @erichkeane
I'm having some doubts too. This seems like the sort of thing we usually trace by knowing the original template, and seeing if we aren't that. So find the thing that owns this expression, and walk over to see if the primary template matches this one (though that isn't a perfect simile here). |
When the it was used, is it possible for us to rebuild all default-arg and default-init? |
friendly ping~ |
2 similar comments
friendly ping~ |
friendly ping~ |
6af171c
to
392001e
Compare
friendly ping~ |
I don't have a great solution for this unfortuantely, but Second, whether or not it was rebuilt should be a property of that expression perhaps? The init itself should know whether it needs to be built again or not. I honestly don't think I understand the requirement well enough, but I DO think this looks/smells incorrect here. It seems to me the static analyzer should be responsible for this perhaps, and not the AST. But again, it isn't clear to me that is the case. |
I think @yronglin Can you modify bool hasRebuiltInit() const { return getExpr() != getParam()->getDefaultArg(); }` |
@cor3ntin @erichkeane Many thanks for the review!
Sure, let me give a try.
Yeah, this function was only used in static analyzer now, maybe we can move it into static analyzer's code. |
… expression Signed-off-by: yronglin <[email protected]>
392001e
to
b5117ef
Compare
I've made changes in Sema, if the rewritten-expr same as |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm WAY happier with this. I don't have the ability to review the SA stuff well enough to give an approval, but the rest of the stuff is good. 1 Nit, else LGTM.
Signed-off-by: yronglin <[email protected]>
Thanks for the review! There are no functional changes in Static Analyzer after @steakhal's +1 (just renamed a function). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much better!
Thanks a lot for working on this
We're hitting an assert after this:
See https://crbug.com/394015869 for stack trace and reproducer. I'll revert for now to unbreak the build, and start a creduce to see if we can get a smaller repro. |
creduce'd reproducer:
|
Thanks for reporting this bug, I'll take a look. |
…ult init expression (llvm#117437)" This caused assertion failures: clang/lib/Analysis/CFG.cpp:822: void (anonymous namespace)::CFGBuilder::appendStmt(CFGBlock *, const Stmt *): Assertion `!isa<Expr>(S) || cast<Expr>(S)->IgnoreParens() == S' failed. See comment on the PR. This reverts commit 44aa618.
…ault init expression" (#127338) This PR reapply #117437. The issue has been fixed by the 2nd commit, we need to ignore parens in CXXDefaultArgExpr when build CFG, because CXXDefaultArgExpr::getExpr stripped off the top level FullExpr and ConstantExpr, ParenExpr may occurres in the top level. --------- Signed-off-by: yronglin <[email protected]>
…arg and default init expression" (#127338) This PR reapply llvm/llvm-project#117437. The issue has been fixed by the 2nd commit, we need to ignore parens in CXXDefaultArgExpr when build CFG, because CXXDefaultArgExpr::getExpr stripped off the top level FullExpr and ConstantExpr, ParenExpr may occurres in the top level. --------- Signed-off-by: yronglin <[email protected]>
…ault init expression" (llvm#127338) This PR reapply llvm#117437. The issue has been fixed by the 2nd commit, we need to ignore parens in CXXDefaultArgExpr when build CFG, because CXXDefaultArgExpr::getExpr stripped off the top level FullExpr and ConstantExpr, ParenExpr may occurres in the top level. --------- Signed-off-by: yronglin <[email protected]>
Clang currently support extending lifetime of object bound to reference members of aggregates, that are created from default member initializer. This PR address this change and updaye CFG and ExprEngine.
This PR reapply #91879.
Fixes #93725.