[clang][analysis] Fix flaky clang/test/Analysis/live-stmts.cpp test (2nd attempt) (#127406) #139591
Conversation
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Balazs Benics (steakhal) ChangesIn my previous attempt (#126913) of fixing the flaky case was on a good track when I used the begin locations as a stable ordering. However, I forgot to consider the case when the begin locations are the same among the Exprs. In an To fix this, I use this time Hopefully fixes #126619 (cherry picked from commit f378e52) Full diff: https://github.com/llvm/llvm-project/pull/139591.diff 2 Files Affected:
diff --git a/clang/lib/Analysis/LiveVariables.cpp b/clang/lib/Analysis/LiveVariables.cpp
index 481932ee59c8e..5fb5ee767a683 100644
--- a/clang/lib/Analysis/LiveVariables.cpp
+++ b/clang/lib/Analysis/LiveVariables.cpp
@@ -662,12 +662,19 @@ void LiveVariables::dumpExprLiveness(const SourceManager &M) {
}
void LiveVariablesImpl::dumpExprLiveness(const SourceManager &M) {
+ const ASTContext &Ctx = analysisContext.getASTContext();
+ auto ByIDs = [&Ctx](const Expr *L, const Expr *R) {
+ return L->getID(Ctx) < R->getID(Ctx);
+ };
+
// Don't iterate over blockEndsToLiveness directly because it's not sorted.
for (const CFGBlock *B : *analysisContext.getCFG()) {
-
llvm::errs() << "\n[ B" << B->getBlockID()
<< " (live expressions at block exit) ]\n";
- for (const Expr *E : blocksEndToLiveness[B].liveExprs) {
+ std::vector<const Expr *> LiveExprs;
+ llvm::append_range(LiveExprs, blocksEndToLiveness[B].liveExprs);
+ llvm::sort(LiveExprs, ByIDs);
+ for (const Expr *E : LiveExprs) {
llvm::errs() << "\n";
E->dump();
}
diff --git a/clang/test/Analysis/live-stmts.cpp b/clang/test/Analysis/live-stmts.cpp
index c60f522588e39..ca2ff6da8b133 100644
--- a/clang/test/Analysis/live-stmts.cpp
+++ b/clang/test/Analysis/live-stmts.cpp
@@ -44,6 +44,8 @@ int testThatDumperWorks(int x, int y, int z) {
// CHECK-NEXT: ImplicitCastExpr {{.*}} <IntegralToBoolean>
// CHECK-NEXT: `-ImplicitCastExpr {{.*}} <LValueToRValue>
// CHECK-NEXT: `-DeclRefExpr {{.*}} 'x' 'int'
+// CHECK-EMPTY:
+// CHECK-EMPTY:
// CHECK: [ B4 (live expressions at block exit) ]
// CHECK-EMPTY:
// CHECK-NEXT: DeclRefExpr {{.*}} 'y' 'int'
|
|
@llvm/pr-subscribers-clang-analysis Author: Balazs Benics (steakhal) ChangesIn my previous attempt (#126913) of fixing the flaky case was on a good track when I used the begin locations as a stable ordering. However, I forgot to consider the case when the begin locations are the same among the Exprs. In an To fix this, I use this time Hopefully fixes #126619 (cherry picked from commit f378e52) Full diff: https://github.com/llvm/llvm-project/pull/139591.diff 2 Files Affected:
diff --git a/clang/lib/Analysis/LiveVariables.cpp b/clang/lib/Analysis/LiveVariables.cpp
index 481932ee59c8e..5fb5ee767a683 100644
--- a/clang/lib/Analysis/LiveVariables.cpp
+++ b/clang/lib/Analysis/LiveVariables.cpp
@@ -662,12 +662,19 @@ void LiveVariables::dumpExprLiveness(const SourceManager &M) {
}
void LiveVariablesImpl::dumpExprLiveness(const SourceManager &M) {
+ const ASTContext &Ctx = analysisContext.getASTContext();
+ auto ByIDs = [&Ctx](const Expr *L, const Expr *R) {
+ return L->getID(Ctx) < R->getID(Ctx);
+ };
+
// Don't iterate over blockEndsToLiveness directly because it's not sorted.
for (const CFGBlock *B : *analysisContext.getCFG()) {
-
llvm::errs() << "\n[ B" << B->getBlockID()
<< " (live expressions at block exit) ]\n";
- for (const Expr *E : blocksEndToLiveness[B].liveExprs) {
+ std::vector<const Expr *> LiveExprs;
+ llvm::append_range(LiveExprs, blocksEndToLiveness[B].liveExprs);
+ llvm::sort(LiveExprs, ByIDs);
+ for (const Expr *E : LiveExprs) {
llvm::errs() << "\n";
E->dump();
}
diff --git a/clang/test/Analysis/live-stmts.cpp b/clang/test/Analysis/live-stmts.cpp
index c60f522588e39..ca2ff6da8b133 100644
--- a/clang/test/Analysis/live-stmts.cpp
+++ b/clang/test/Analysis/live-stmts.cpp
@@ -44,6 +44,8 @@ int testThatDumperWorks(int x, int y, int z) {
// CHECK-NEXT: ImplicitCastExpr {{.*}} <IntegralToBoolean>
// CHECK-NEXT: `-ImplicitCastExpr {{.*}} <LValueToRValue>
// CHECK-NEXT: `-DeclRefExpr {{.*}} 'x' 'int'
+// CHECK-EMPTY:
+// CHECK-EMPTY:
// CHECK: [ B4 (live expressions at block exit) ]
// CHECK-EMPTY:
// CHECK-NEXT: DeclRefExpr {{.*}} 'y' 'int'
|
|
We decided to backport in #127406 (comment) |
github-project-automation
bot
moved this from Needs Triage
to Needs Merge
in LLVM Release Status
…2nd attempt) (llvm#127406) In my previous attempt (llvm#126913) of fixing the flaky case was on a good track when I used the begin locations as a stable ordering. However, I forgot to consider the case when the begin locations are the same among the Exprs. In an `EXPENSIVE_CHECKS` build, arrays are randomly shuffled prior to sorting them. This exposed the flaky behavior much more often basically breaking the "stability" of the vector - as it should. Because of this, I had to revert the previous fix attempt in llvm#127034. To fix this, I use this time `Expr::getID` for a stable ID for an Expr. Hopefully fixes llvm#126619 Hopefully fixes llvm#126804 (cherry picked from commit f378e52)
tstellar
force-pushed
the
bb/backport-flaky-fix
branch
from
55ae802 to
1c03684
Compare
github-project-automation
bot
moved this from Needs Merge
to Done
in LLVM Release Status
|
@steakhal (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR. |
In my previous attempt (#126913) of fixing the flaky case was on a good track when I used the begin locations as a stable ordering. However, I forgot to consider the case when the begin locations are the same among the Exprs.
In an
EXPENSIVE_CHECKSbuild, arrays are randomly shuffled prior to sorting them. This exposed the flaky behavior much more often basically breaking the "stability" of the vector - as it should. Because of this, I had to revert the previous fix attempt in #127034.To fix this, I use this time
Expr::getIDfor a stable ID for an Expr.Hopefully fixes #126619
Hopefully fixes #126804
(cherry picked from commit f378e52)