https://github.com/steakhal created https://github.com/llvm/llvm-project/pull/127406
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_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 #127034. To fix this, I use this time `Expr::getID` for a stable ID for an Expr. Hopefully fixes #126619 Hopefully fixes #126804 >From 4beb086e2fc90524ae9b76ce61db506658da4306 Mon Sep 17 00:00:00 2001 From: Balazs Benics <benicsbal...@gmail.com> Date: Sun, 16 Feb 2025 20:07:02 +0100 Subject: [PATCH] [clang][analysis] Fix flaky clang/test/Analysis/live-stmts.cpp test (2nd attempt) 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_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. To fix this, I I use this time `Expr::getID` for a stable ID for an Expr. Hopefully fixes #126619 Hopefully fixes #126804 --- clang/lib/Analysis/LiveVariables.cpp | 8 ++--- clang/test/Analysis/live-stmts.cpp | 47 +++++++++++++--------------- 2 files changed, 26 insertions(+), 29 deletions(-) diff --git a/clang/lib/Analysis/LiveVariables.cpp b/clang/lib/Analysis/LiveVariables.cpp index af563702b77bf..c7d3451d37cf6 100644 --- a/clang/lib/Analysis/LiveVariables.cpp +++ b/clang/lib/Analysis/LiveVariables.cpp @@ -664,18 +664,18 @@ void LiveVariables::dumpExprLiveness(const SourceManager &M) { } void LiveVariablesImpl::dumpExprLiveness(const SourceManager &M) { - auto ByBeginLoc = [&M](const Expr *L, const Expr *R) { - return M.isBeforeInTranslationUnit(L->getBeginLoc(), R->getBeginLoc()); + 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"; std::vector<const Expr *> LiveExprs; llvm::append_range(LiveExprs, blocksEndToLiveness[B].liveExprs); - llvm::sort(LiveExprs, ByBeginLoc); + 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 9cac815e65de1..ca2ff6da8b133 100644 --- a/clang/test/Analysis/live-stmts.cpp +++ b/clang/test/Analysis/live-stmts.cpp @@ -1,6 +1,3 @@ -// Disabling this flaky test, see https://github.com/llvm/llvm-project/pull/126913#issuecomment-2655850766 -// UNSUPPORTED: true - // RUN: %clang_analyze_cc1 -w -analyzer-checker=debug.DumpLiveExprs %s 2>&1\ // RUN: | FileCheck %s @@ -29,36 +26,36 @@ int testThatDumperWorks(int x, int y, int z) { // CHECK-EMPTY: // CHECK: [ B2 (live expressions at block exit) ] // CHECK-EMPTY: -// CHECK-NEXT: ImplicitCastExpr {{.*}} <IntegralToBoolean> -// CHECK-NEXT: `-ImplicitCastExpr {{.*}} <LValueToRValue> -// CHECK-NEXT: `-DeclRefExpr {{.*}} 'x' 'int' -// CHECK-EMPTY: // CHECK-NEXT: DeclRefExpr {{.*}} 'y' 'int' // CHECK-EMPTY: // CHECK-NEXT: DeclRefExpr {{.*}} 'z' 'int' // CHECK-EMPTY: -// CHECK-EMPTY: -// CHECK: [ B3 (live expressions at block exit) ] -// CHECK-EMPTY: // CHECK-NEXT: ImplicitCastExpr {{.*}} <IntegralToBoolean> // CHECK-NEXT: `-ImplicitCastExpr {{.*}} <LValueToRValue> // CHECK-NEXT: `-DeclRefExpr {{.*}} 'x' 'int' // CHECK-EMPTY: -// CHECK-NEXT: DeclRefExpr {{.*}} 'y' 'int' // CHECK-EMPTY: -// CHECK-NEXT: DeclRefExpr {{.*}} 'z' 'int' +// CHECK: [ B3 (live expressions at block exit) ] // CHECK-EMPTY: +// CHECK-NEXT: DeclRefExpr {{.*}} 'y' 'int' // CHECK-EMPTY: -// CHECK: [ B4 (live expressions at block exit) ] +// CHECK-NEXT: DeclRefExpr {{.*}} 'z' 'int' // CHECK-EMPTY: // 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' // CHECK-EMPTY: // CHECK-NEXT: DeclRefExpr {{.*}} 'z' 'int' // CHECK-EMPTY: +// CHECK-NEXT: ImplicitCastExpr {{.*}} <IntegralToBoolean> +// CHECK-NEXT: `-ImplicitCastExpr {{.*}} <LValueToRValue> +// CHECK-NEXT: `-DeclRefExpr {{.*}} 'x' 'int' +// CHECK-EMPTY: // CHECK-EMPTY: // CHECK: [ B5 (live expressions at block exit) ] // CHECK-EMPTY: @@ -228,15 +225,15 @@ int logicalOpInTernary(bool b) { // CHECK: ImplicitCastExpr {{.*}} '_Bool' <LValueToRValue> // CHECK: `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' // CHECK-EMPTY: +// CHECK: ImplicitCastExpr {{.*}} '_Bool' <LValueToRValue> +// CHECK: `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' +// CHECK-EMPTY: // CHECK: BinaryOperator {{.*}} '_Bool' '||' // CHECK: |-ImplicitCastExpr {{.*}} '_Bool' <LValueToRValue> // CHECK: | `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' // CHECK: `-ImplicitCastExpr {{.*}} '_Bool' <LValueToRValue> // CHECK: `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' // CHECK-EMPTY: -// CHECK: ImplicitCastExpr {{.*}} '_Bool' <LValueToRValue> -// CHECK: `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' -// CHECK-EMPTY: // CHECK: IntegerLiteral {{.*}} 'int' 0 // CHECK-EMPTY: // CHECK: IntegerLiteral {{.*}} 'int' 1 @@ -247,15 +244,15 @@ int logicalOpInTernary(bool b) { // CHECK: ImplicitCastExpr {{.*}} '_Bool' <LValueToRValue> // CHECK: `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' // CHECK-EMPTY: +// CHECK: ImplicitCastExpr {{.*}} '_Bool' <LValueToRValue> +// CHECK: `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' +// CHECK-EMPTY: // CHECK: BinaryOperator {{.*}} '_Bool' '||' // CHECK: |-ImplicitCastExpr {{.*}} '_Bool' <LValueToRValue> // CHECK: | `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' // CHECK: `-ImplicitCastExpr {{.*}} '_Bool' <LValueToRValue> // CHECK: `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' // CHECK-EMPTY: -// CHECK: ImplicitCastExpr {{.*}} '_Bool' <LValueToRValue> -// CHECK: `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' -// CHECK-EMPTY: // CHECK: IntegerLiteral {{.*}} 'int' 0 // CHECK-EMPTY: // CHECK: IntegerLiteral {{.*}} 'int' 1 @@ -266,15 +263,15 @@ int logicalOpInTernary(bool b) { // CHECK: ImplicitCastExpr {{.*}} '_Bool' <LValueToRValue> // CHECK: `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' // CHECK-EMPTY: +// CHECK: ImplicitCastExpr {{.*}} '_Bool' <LValueToRValue> +// CHECK: `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' +// CHECK-EMPTY: // CHECK: BinaryOperator {{.*}} '_Bool' '||' // CHECK: |-ImplicitCastExpr {{.*}} '_Bool' <LValueToRValue> // CHECK: | `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' // CHECK: `-ImplicitCastExpr {{.*}} '_Bool' <LValueToRValue> // CHECK: `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' // CHECK-EMPTY: -// CHECK: ImplicitCastExpr {{.*}} '_Bool' <LValueToRValue> -// CHECK: `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' -// CHECK-EMPTY: // CHECK: IntegerLiteral {{.*}} 'int' 0 // CHECK-EMPTY: // CHECK: IntegerLiteral {{.*}} 'int' 1 @@ -285,15 +282,15 @@ int logicalOpInTernary(bool b) { // CHECK: ImplicitCastExpr {{.*}} '_Bool' <LValueToRValue> // CHECK: `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' // CHECK-EMPTY: +// CHECK: ImplicitCastExpr {{.*}} '_Bool' <LValueToRValue> +// CHECK: `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' +// CHECK-EMPTY: // CHECK: BinaryOperator {{.*}} '_Bool' '||' // CHECK: |-ImplicitCastExpr {{.*}} '_Bool' <LValueToRValue> // CHECK: | `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' // CHECK: `-ImplicitCastExpr {{.*}} '_Bool' <LValueToRValue> // CHECK: `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' // CHECK-EMPTY: -// CHECK: ImplicitCastExpr {{.*}} '_Bool' <LValueToRValue> -// CHECK: `-DeclRefExpr {{.*}} '_Bool' lvalue ParmVar {{.*}} 'b' '_Bool' -// CHECK-EMPTY: // CHECK: IntegerLiteral {{.*}} 'int' 0 // CHECK-EMPTY: // CHECK: IntegerLiteral {{.*}} 'int' 1 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits