llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-analysis Author: None (martinboehme) <details> <summary>Changes</summary> This is to be consistent with `getValue()`, which also uses `ignoreCFGOmittedNodes()`. Before this fix, it was not possible to retrieve a `Value` from a "CFG omitted" node that had previously been set using `setValue()`; see the accompanying test, which fails without the fix. I discovered this issue while running internal integration tests on https://github.com/llvm/llvm-project/pull/78127. --- Full diff: https://github.com/llvm/llvm-project/pull/78245.diff 2 Files Affected: - (modified) clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp (+6-4) - (modified) clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp (+46) ``````````diff diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp index 96fe6df88dbb9f7..05f8101b0068a31 100644 --- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -803,13 +803,15 @@ void Environment::setValue(const StorageLocation &Loc, Value &Val) { } void Environment::setValue(const Expr &E, Value &Val) { + const Expr &CanonE = ignoreCFGOmittedNodes(E); + if (auto *RecordVal = dyn_cast<RecordValue>(&Val)) { - assert(isOriginalRecordConstructor(E) || - &RecordVal->getLoc() == &getResultObjectLocation(E)); + assert(isOriginalRecordConstructor(CanonE) || + &RecordVal->getLoc() == &getResultObjectLocation(CanonE)); } - assert(E.isPRValue()); - ExprToVal[&E] = &Val; + assert(CanonE.isPRValue()); + ExprToVal[&CanonE] = &Val; } Value *Environment::getValue(const StorageLocation &Loc) const { diff --git a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp index 003434a58b1075f..8799d03dfd3c588 100644 --- a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp @@ -58,6 +58,52 @@ TEST_F(EnvironmentTest, FlowCondition) { EXPECT_FALSE(Env.allows(NotX)); } +TEST_F(EnvironmentTest, SetAndGetValueOnCfgOmittedNodes) { + // Check that we can set a value on an expression that is omitted from the CFG + // (see `ignoreCFGOmittedNodes()`), then retrieve that same value from the + // expression. This is a regression test; `setValue()` and `getValue()` + // previously did not use `ignoreCFGOmittedNodes()` consistently. + + using namespace ast_matchers; + + std::string Code = R"cc( + struct S { + int f(); + }; + void target() { + // Method call on a temporary produces an `ExprWithCleanups`. + S().f(); + (1); + } + )cc"; + + auto Unit = + tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++17"}); + auto &Context = Unit->getASTContext(); + + ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U); + + const ExprWithCleanups *WithCleanups = selectFirst<ExprWithCleanups>( + "cleanups", + match(exprWithCleanups(hasType(isInteger())).bind("cleanups"), Context)); + ASSERT_NE(WithCleanups, nullptr); + + const ParenExpr *Paren = selectFirst<ParenExpr>( + "paren", match(parenExpr(hasType(isInteger())).bind("paren"), Context)); + ASSERT_NE(Paren, nullptr); + + Environment Env(DAContext); + IntegerValue *Val1 = + cast<IntegerValue>(Env.createValue(Unit->getASTContext().IntTy)); + Env.setValue(*WithCleanups, *Val1); + EXPECT_EQ(Env.getValue(*WithCleanups), Val1); + + IntegerValue *Val2 = + cast<IntegerValue>(Env.createValue(Unit->getASTContext().IntTy)); + Env.setValue(*Paren, *Val2); + EXPECT_EQ(Env.getValue(*Paren), Val2); +} + TEST_F(EnvironmentTest, CreateValueRecursiveType) { using namespace ast_matchers; `````````` </details> https://github.com/llvm/llvm-project/pull/78245 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
