[nullability] Create `Value`s for pointer prvalues if the framework doesn't.
This fixes a bunch of false positives in unit tests.
PiperOrigin-RevId: 549603184
Change-Id: I0fb05db361eccc768f0c5455249f601075628199
diff --git a/nullability/pointer_nullability_analysis.cc b/nullability/pointer_nullability_analysis.cc
index cb8258a..6786603 100644
--- a/nullability/pointer_nullability_analysis.cc
+++ b/nullability/pointer_nullability_analysis.cc
@@ -23,6 +23,7 @@
#include "clang/AST/Type.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
#include "clang/Analysis/FlowSensitive/Arena.h"
#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
@@ -729,6 +730,21 @@
transferFlowSensitiveNullCheckImplicitCastPtrToBool)
.Build();
}
+
+// Ensure all prvalue expressions of pointer type have a `PointerValue`
+// associated with them so we can track nullability through them.
+void ensurePointerHasValue(const CFGElement &Elt, Environment &Env) {
+ auto S = Elt.getAs<CFGStmt>();
+ if (!S) return;
+
+ auto *E = dyn_cast<Expr>(S->getStmt());
+ if (E == nullptr || !E->isPRValue() || !E->getType()->isPointerType()) return;
+
+ if (Env.getValueStrict(*E) == nullptr)
+ // `createValue()` always produces a value for pointer types.
+ Env.setValueStrict(*E, *Env.createValue(E->getType()));
+}
+
} // namespace
PointerNullabilityAnalysis::PointerNullabilityAnalysis(ASTContext &Context)
@@ -748,6 +764,8 @@
PointerNullabilityLattice &Lattice,
Environment &Env) {
TransferState<PointerNullabilityLattice> State(Lattice, Env);
+
+ ensurePointerHasValue(Elt, Env);
NonFlowSensitiveTransferer(Elt, getASTContext(), State);
FlowSensitiveTransferer(Elt, getASTContext(), State);
}
diff --git a/nullability/test/casts.cc b/nullability/test/casts.cc
index d70da1d..cea0263 100644
--- a/nullability/test/casts.cc
+++ b/nullability/test/casts.cc
@@ -265,7 +265,7 @@
void target(Derived *_Nullable x, Derived *_Nonnull y) {
*static_cast<Base *>(x); // [[unsafe]]
- *static_cast<Base *>(y); // [[unsafe]] TODO: Fix false positive.
+ *static_cast<Base *>(y);
}
)cc"));
@@ -278,12 +278,11 @@
void target(Struct3Arg<1, int *_Nullable, int *> &p) {
*((const Struct3Arg<1, int *, int *> &)p).arg1; // [[unsafe]]
- *((const Struct3Arg<1, int *, int *> &)p) // [[unsafe]]
- .arg2; // TODO: Fix false positive.
- *(int *)p.arg1; // [[unsafe]]
- *(int *)p.arg2; // [[unsafe]] TODO: Fix false positive.
+ *((const Struct3Arg<1, int *, int *> &)p).arg2;
+ *(int *)p.arg1; // [[unsafe]]
+ *(int *)p.arg2;
*(float *)p.arg1; // [[unsafe]]
- *(char *)p.arg2; // [[unsafe]] TODO: Fix false positive.
+ *(char *)p.arg2;
}
)cc"));
@@ -295,7 +294,7 @@
};
void target(Struct2Arg<const int *, const int *_Nullable> &p) {
- *const_cast<int *>(p.arg0); // [[unsafe]] TODO: Fix false positive.
+ *const_cast<int *>(p.arg0);
*const_cast<int *>(p.arg1); // [[unsafe]]
}
)cc"));
diff --git a/nullability/test/fields.cc b/nullability/test/fields.cc
index 697d8d5..1404582 100644
--- a/nullability/test/fields.cc
+++ b/nullability/test/fields.cc
@@ -80,7 +80,6 @@
)cc"));
}
-// TODO: fix false positives due to unsupported PointerValues in the framework.
TEST(PointerNullabilityTest, ChainedFieldDeref) {
EXPECT_TRUE(checkDiagnostics(R"cc(
struct S {
@@ -89,14 +88,14 @@
S *unknown;
};
void target(S &s) {
- *(*s.nonnull).nonnull; // [[unsafe]] TODO: fix false positive
+ *(*s.nonnull).nonnull;
*(*s.nonnull).nullable; // [[unsafe]]
- *(*s.nonnull).unknown; // [[unsafe]] TODO: fix false positive
+ *(*s.nonnull).unknown;
- s.nonnull->nonnull->nonnull; // [[unsafe]] TODO: fix false positive
- s.nonnull->nonnull->nullable; // [[unsafe]] TODO: fix false positive
+ s.nonnull->nonnull->nonnull;
+ s.nonnull->nonnull->nullable;
s.nonnull->nullable->nonnull; // [[unsafe]]
- s.nonnull->unknown->nonnull; // [[unsafe]] TODO: fix false positive
+ s.nonnull->unknown->nonnull;
*&s;
}
diff --git a/nullability/test/pointer_arithmetic.cc b/nullability/test/pointer_arithmetic.cc
index c6ea24e..a873906 100644
--- a/nullability/test/pointer_arithmetic.cc
+++ b/nullability/test/pointer_arithmetic.cc
@@ -10,7 +10,6 @@
namespace clang::tidy::nullability {
namespace {
-// TODO: fix false positives due to unsupported PointerValues in the framework.
TEST(PointerNullabilityTest, PointerArithmetic) {
EXPECT_TRUE(checkDiagnostics(R"cc(
void target(int *_Nullable p, int *_Nonnull q, int *r) {
@@ -20,17 +19,17 @@
*p--; // [[unsafe]]
*+p; // [[unsafe]]
- *++q; // [[unsafe]] TODO: fix false positive
- *q++; // [[unsafe]] TODO: fix false positive
- *--q; // [[unsafe]] TODO: fix false positive
- *q--; // [[unsafe]] TODO: fix false positive
- *+q; // [[unsafe]] TODO: fix false positive
+ *++q;
+ *q++;
+ *--q;
+ *q--;
+ *+q;
- *++r; // [[unsafe]] TODO: fix false positive
- *r++; // [[unsafe]] TODO: fix false positive
- *--r; // [[unsafe]] TODO: fix false positive
- *r--; // [[unsafe]] TODO: fix false positive
- *+r; // [[unsafe]] TODO: fix false positive
+ *++r;
+ *r++;
+ *--r;
+ *r--;
+ *+r;
}
)cc"));
}
diff --git a/nullability/test/templates.cc b/nullability/test/templates.cc
index f994eb3..980021b 100644
--- a/nullability/test/templates.cc
+++ b/nullability/test/templates.cc
@@ -261,7 +261,7 @@
)cc"));
}
-// TODO: Fix false positives and false negatives.
+// TODO: Fix false negatives.
TEST(PointerNullabilityTest,
ClassTemplateInstantiationWithStructsAsParameters) {
EXPECT_TRUE(checkDiagnostics(R"cc(
@@ -290,9 +290,9 @@
*p.arg0.getNullable(); // [[unsafe]]
*p.arg0.getNonnull();
- *p.getT0().unknown; // [[unsafe]] TODO: fix false positive.
+ *p.getT0().unknown;
*p.getT0().nullable; // [[unsafe]]
- *p.getT0().nonnull; // [[unsafe]] TODO: fix false positive.
+ *p.getT0().nonnull;
*p.getT0().getUnknown();
*p.getT0().getNullable(); // [[unsafe]]
@@ -372,15 +372,15 @@
*p.arg3.getNullableUInt(); // [[unsafe]]
*p.arg3.getNullableBool(); // [[unsafe]]
- *p.getT0().unknownChar; // [[unsafe]] TODO: fix false positive.
- *p.getT1().nullableChar; // [[unsafe]]
- *p.getT2().nonnullChar; // [[unsafe]] TODO: fix false positive.
- *p.getT3().unknownLongLong; // [[unsafe]] TODO: fix false positive.
- *p.getT3().nullableDouble; // [[unsafe]]
- *p.getT3().nonnullFloat; // [[unsafe]] TODO: fix false positive.
- *p.getT3().unknownShort; // [[unsafe]] TODO: fix false positive.
- *p.getT3().nullableUInt; // [[unsafe]]
- *p.getT3().nullableBool; // [[unsafe]]
+ *p.getT0().unknownChar;
+ *p.getT1().nullableChar; // [[unsafe]]
+ *p.getT2().nonnullChar;
+ *p.getT3().unknownLongLong;
+ *p.getT3().nullableDouble; // [[unsafe]]
+ *p.getT3().nonnullFloat;
+ *p.getT3().unknownShort;
+ *p.getT3().nullableUInt; // [[unsafe]]
+ *p.getT3().nullableBool; // [[unsafe]]
*p.getT0().getUnknownChar();
*p.getT1().getNullableChar(); // [[unsafe]]
@@ -460,15 +460,15 @@
*p.arg3.getNonnullConstChar();
*p.arg3.getConstNonnullConstChar();
- *p.getT1().constUnknownChar; // [[unsafe]] TODO: fix false positive.
- *p.getT1().unknownConstChar; // [[unsafe]] TODO: fix false positive.
- *p.getT1().constUnknownConstChar; // [[unsafe]] TODO: fix false positive.
- *p.getT2().constNullableChar; // [[unsafe]]
- *p.getT2().nullableConstChar; // [[unsafe]]
+ *p.getT1().constUnknownChar;
+ *p.getT1().unknownConstChar;
+ *p.getT1().constUnknownConstChar;
+ *p.getT2().constNullableChar; // [[unsafe]]
+ *p.getT2().nullableConstChar; // [[unsafe]]
*p.getT2().constNullableConstChar; // [[unsafe]]
- *p.getT3().constNonnullChar; // [[unsafe]] TODO: fix false positive.
- *p.getT3().nonnullConstChar; // [[unsafe]] TODO: fix false positive.
- *p.getT3().constNonnullConstChar; // [[unsafe]] TODO: fix false positive.
+ *p.getT3().constNonnullChar;
+ *p.getT3().nonnullConstChar;
+ *p.getT3().constNonnullConstChar;
*p.getT1().getConstUnknownChar();
*p.getT1().getUnknownConstChar();
@@ -562,7 +562,6 @@
)cc"));
}
-// TODO: Fix false positives.
TEST(PointerNullabilityTest,
ClassTemplateInstantiationWithTemplateStructsAsParameters) {
// Class template with another class template as parameter
@@ -623,9 +622,9 @@
int, int *_Nullable, int *_Nonnull, int>
p) {
*p.arg0.arg0.arg0.arg0; // [[unsafe]]
- *p.arg0.arg0.arg0.arg1; // [[unsafe]] TODO: fix false positive.
+ *p.arg0.arg0.arg0.arg1;
*p.arg0.arg0.arg0.arg2; // [[unsafe]]
- *p.arg0.arg0.arg0.arg3; // [[unsafe]] TODO: fix false positive.
+ *p.arg0.arg0.arg0.arg3;
*p.arg0.arg0.arg0.arg4; // [[unsafe]]
*p.arg0.arg0.arg4; // [[unsafe]]
*p.arg0.arg2; // [[unsafe]]
@@ -659,9 +658,9 @@
int, int *_Nullable, 2, int *_Nonnull, int>
p) {
*p.arg1.arg1.arg1.arg1; // [[unsafe]]
- *p.arg1.arg1.arg1.arg2; // [[unsafe]] TODO: fix false positive.
+ *p.arg1.arg1.arg1.arg2;
*p.arg1.arg1.arg1.arg3; // [[unsafe]]
- *p.arg1.arg1.arg1.arg5; // [[unsafe]] TODO: fix false positive.
+ *p.arg1.arg1.arg1.arg5;
*p.arg1.arg1.arg1.arg6; // [[unsafe]]
*p.arg1.arg1.arg6; // [[unsafe]]
*p.arg1.arg3; // [[unsafe]]
@@ -1079,13 +1078,7 @@
void target() {
*returnSecond<StructUnknownNullable, int *_Nullable>(); // [[unsafe]]
*returnSecond<int *_Nonnull, StructUnknownNullable *>();
- // TODO: The following line is a false positive. We correctly compute the
- // nullability of the expression, as confirmed by the call to
- // `assert_nullability`. However, the dataflow framework currently does
- // not model pointer values for this expression, which results in a (in
- // this case incorrect) nullptr value.
- *returnSecond<int *_Nonnull, StructUnknownNullable>() // [[unsafe]]
- .var0; // TODO: Fix false positive.
+ *returnSecond<int *_Nonnull, StructUnknownNullable>().var0;
__assert_nullability<NK_unspecified>(
returnSecond<int *_Nonnull, StructUnknownNullable>().var0);
*returnSecond<int *_Nonnull, StructUnknownNullable>().var1; // [[unsafe]]
