llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Artem Dergachev (haoNoQ) <details> <summary>Changes</summary> Simularly to #<!-- -->178943, we need to make sure the old value does not stick around. And for correctness purposes it's better to conjure a fresh value than to leave it completely unknown. --- Full diff: https://github.com/llvm/llvm-project/pull/179058.diff 2 Files Affected: - (modified) clang/lib/Analysis/FlowSensitive/Transfer.cpp (+12-13) - (modified) clang/unittests/Analysis/FlowSensitive/TransferTest.cpp (+2-4) ``````````diff diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp index 51cc1f9bc26ab..a22e6284df5bc 100644 --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -155,21 +155,18 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> { const Expr *RHS = S->getRHS(); assert(RHS != nullptr); - // Do compound assignments up-front, as there are so many of them and we - // don't want to list all of them in the switch statement below. - // To avoid generating unnecessary values, we don't create a new value but - // instead leave it to the specific analysis to do this if desired. - if (S->isCompoundAssignmentOp()) - propagateStorageLocation(*S->getLHS(), *S, Env); - - switch (S->getOpcode()) { - case BO_Assign: { + // Do assignments and compound assignments up-front, as there are + // so many of them and we don't want to list all of them in + // the switch statement below. + if (S->isAssignmentOp()) { auto *LHSLoc = Env.getStorageLocation(*LHS); if (LHSLoc == nullptr) - break; + return; - auto *RHSVal = Env.getValue(*RHS); - if (RHSVal == nullptr) + // Compound assignments involve arithmetic we don't model yet. + Value *RHSVal = + S->isCompoundAssignmentOp() ? nullptr : Env.getValue(*RHS); + if (!RHSVal) RHSVal = Env.createValue(LHS->getType()); // Assign a value to the storage location of the left-hand side. @@ -177,8 +174,10 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> { // Assign a storage location for the whole expression. Env.setStorageLocation(*S, *LHSLoc); - break; + return; } + + switch (S->getOpcode()) { case BO_LAnd: case BO_LOr: { BoolValue &LHSVal = getLogicOperatorSubExprValue(*LHS); diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp index e528ca2221ad1..c0bcb220c3d76 100644 --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -990,8 +990,7 @@ TEST(TransferTest, BinaryOperatorAssignUnknown) { ASSERT_TRUE(isa_and_nonnull<IntegerValue>(FooAtCVal)); EXPECT_NE(FooAtAVal, FooAtBVal); - // FIXME: Should be NE too. - EXPECT_EQ(FooAtBVal, FooAtCVal); + EXPECT_NE(FooAtBVal, FooAtCVal); // Check that the storage location is correctly propagated. auto MatchResult = match(binaryOperator().bind("bo"), ASTCtx); @@ -1006,8 +1005,7 @@ TEST(TransferTest, BinaryOperatorAssignUnknown) { const Environment &EnvR = getEnvironmentAtAnnotation(Results, "r"); EXPECT_FALSE(EnvQ.proves(EnvQ.arena().makeLiteral(false))); - // FIXME: Should be FALSE too. - EXPECT_TRUE(EnvR.proves(EnvR.arena().makeLiteral(false))); + EXPECT_FALSE(EnvR.proves(EnvR.arena().makeLiteral(false))); }); } `````````` </details> https://github.com/llvm/llvm-project/pull/179058 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
