https://github.com/martinboehme created
https://github.com/llvm/llvm-project/pull/96601
We definitely know that these operations change the value of their operand, so
clear out any value associated with it. We don't create a new value, instead
leaving it to the analysis to do this if desired.
>From 61a143e02eb9417a8a61bc5349522ee8d7dd04c8 Mon Sep 17 00:00:00 2001
From: Martin Braenne <mboe...@google.com>
Date: Tue, 25 Jun 2024 06:45:30 +0000
Subject: [PATCH] [clang][nullability] Improve modeling of `++`/`--` operators.
We definitely know that these operations change the value of their operand, so
clear out any value associated with it. We don't create a new value, instead
leaving it to the analysis to do this if desired.
---
clang/lib/Analysis/FlowSensitive/Transfer.cpp | 17 ++++++----
.../Analysis/FlowSensitive/TransferTest.cpp | 31 ++++++++++++++-----
2 files changed, 34 insertions(+), 14 deletions(-)
diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index 8109ac6a781e7..3c896d373a211 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -391,17 +391,22 @@ class TransferVisitor : public
ConstStmtVisitor<TransferVisitor> {
}
case UO_PreInc:
case UO_PreDec:
- // Propagate the storage location, but don't create a new value; to
- // avoid generating unnecessary values, we leave it to the specific
- // analysis to do this if desired.
+ // Propagate the storage location and clear out any value associated with
+ // it (to represent the fact that the value has definitely changed).
+ // To avoid generating unnecessary values, we leave it to the specific
+ // analysis to create a new value if desired.
propagateStorageLocation(*S->getSubExpr(), *S, Env);
+ if (StorageLocation *Loc = Env.getStorageLocation(*S->getSubExpr()))
+ Env.clearValue(*Loc);
break;
case UO_PostInc:
case UO_PostDec:
- // Propagate the old value, but don't create a new value; to avoid
- // generating unnecessary values, we leave it to the specific analysis
- // to do this if desired.
+ // Propagate the old value, then clear out any value associated with the
+ // storage location (to represent the fact that the value has definitely
+ // changed). See above for rationale.
propagateValue(*S->getSubExpr(), *S, Env);
+ if (StorageLocation *Loc = Env.getStorageLocation(*S->getSubExpr()))
+ Env.clearValue(*Loc);
break;
default:
break;
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index cfbc64c77b0cc..e743eefa5d458 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -3789,36 +3789,51 @@ TEST(TransferTest, AddrOfReference) {
TEST(TransferTest, Preincrement) {
std::string Code = R"(
void target(int I) {
+ (void)0; // [[before]]
int &IRef = ++I;
- // [[p]]
+ // [[after]]
}
)";
runDataflow(
Code,
[](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
ASTContext &ASTCtx) {
- const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+ const Environment &EnvBefore =
+ getEnvironmentAtAnnotation(Results, "before");
+ const Environment &EnvAfter =
+ getEnvironmentAtAnnotation(Results, "after");
- EXPECT_EQ(&getLocForDecl(ASTCtx, Env, "IRef"),
- &getLocForDecl(ASTCtx, Env, "I"));
+ EXPECT_EQ(&getLocForDecl(ASTCtx, EnvAfter, "IRef"),
+ &getLocForDecl(ASTCtx, EnvBefore, "I"));
+
+ const ValueDecl *IDecl = findValueDecl(ASTCtx, "I");
+ EXPECT_NE(EnvBefore.getValue(*IDecl), nullptr);
+ EXPECT_EQ(EnvAfter.getValue(*IDecl), nullptr);
});
}
TEST(TransferTest, Postincrement) {
std::string Code = R"(
void target(int I) {
+ (void)0; // [[before]]
int OldVal = I++;
- // [[p]]
+ // [[after]]
}
)";
runDataflow(
Code,
[](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
ASTContext &ASTCtx) {
- const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+ const Environment &EnvBefore =
+ getEnvironmentAtAnnotation(Results, "before");
+ const Environment &EnvAfter =
+ getEnvironmentAtAnnotation(Results, "after");
+
+ EXPECT_EQ(&getValueForDecl(ASTCtx, EnvBefore, "I"),
+ &getValueForDecl(ASTCtx, EnvAfter, "OldVal"));
- EXPECT_EQ(&getValueForDecl(ASTCtx, Env, "OldVal"),
- &getValueForDecl(ASTCtx, Env, "I"));
+ const ValueDecl *IDecl = findValueDecl(ASTCtx, "I");
+ EXPECT_EQ(EnvAfter.getValue(*IDecl), nullptr);
});
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
