Check whether targets are valid before collecting evidence in more cases.
Also log more useful information about what invalid Target evidence is being emitted for.
PiperOrigin-RevId: 579311303
Change-Id: Id6ce4c5f34d1c5618cf54c3e645ddb9436fb7ec2
diff --git a/nullability/inference/collect_evidence.cc b/nullability/inference/collect_evidence.cc
index 9caf03c..6d1bd93 100644
--- a/nullability/inference/collect_evidence.cc
+++ b/nullability/inference/collect_evidence.cc
@@ -80,7 +80,10 @@
void operator()(const Decl &Target, Slot S, Evidence::Kind Kind,
SourceLocation Loc) const {
CHECK(isInferenceTarget(Target))
- << "Evidence emitted for a Target which is not an inference target.";
+ << "Evidence emitted for a Target which is not an inference target: "
+ << (dyn_cast<NamedDecl>(&Target)
+ ? dyn_cast<NamedDecl>(&Target)->getQualifiedNameAsString()
+ : "not a named decl");
Evidence E;
E.set_slot(S);
@@ -257,6 +260,9 @@
isa<clang::CXXMethodDecl>(CalleeDecl))
++ArgI;
+ bool CollectEvidenceForCallee = isInferenceTarget(CalleeDecl);
+ bool CollectEvidenceForCaller = isInferenceTarget(*Env.getCurrentFunc());
+
// For each pointer parameter of the callee, ...
for (; ParamI < CalleeDecl.param_size(); ++ParamI, ++ArgI) {
const auto *ParamDecl = CalleeDecl.getParamDecl(ParamI);
@@ -271,33 +277,37 @@
SourceLocation ArgLoc = Expr.getArg(ArgI)->getExprLoc();
- auto ParamNullability = getNullabilityAnnotationsFromTypeAndOverrides(
- ParamType, ParamDecl, Lattice);
+ if (CollectEvidenceForCaller) {
+ auto ParamNullability = getNullabilityAnnotationsFromTypeAndOverrides(
+ ParamType, ParamDecl, Lattice);
- // Collect evidence from the binding of the argument to the parameter's
- // nullability, if known.
- collectEvidenceFromBindingToType(
- ParamNullability, *PV, InferableCallerSlots, InferableSlotsConstraint,
- Env, ArgLoc, Emit);
-
- // Emit evidence of the parameter's nullability. First, calculate that
- // nullability based on InferableSlots for the caller being assigned to
- // Unknown or their previously-inferred value, to reflect the current
- // annotations and not all possible annotations for them.
- NullabilityKind ArgNullability =
- getNullability(*PV, Env, &InferableSlotsConstraint);
- Evidence::Kind ArgEvidenceKind;
- switch (ArgNullability) {
- case NullabilityKind::Nullable:
- ArgEvidenceKind = Evidence::NULLABLE_ARGUMENT;
- break;
- case NullabilityKind::NonNull:
- ArgEvidenceKind = Evidence::NONNULL_ARGUMENT;
- break;
- default:
- ArgEvidenceKind = Evidence::UNKNOWN_ARGUMENT;
+ // Collect evidence from the binding of the argument to the parameter's
+ // nullability, if known.
+ collectEvidenceFromBindingToType(
+ ParamNullability, *PV, InferableCallerSlots, InferableSlotsConstraint,
+ Env, ArgLoc, Emit);
}
- Emit(CalleeDecl, paramSlot(ParamI), ArgEvidenceKind, ArgLoc);
+
+ if (CollectEvidenceForCallee) {
+ // Emit evidence of the parameter's nullability. First, calculate that
+ // nullability based on InferableSlots for the caller being assigned to
+ // Unknown or their previously-inferred value, to reflect the current
+ // annotations and not all possible annotations for them.
+ NullabilityKind ArgNullability =
+ getNullability(*PV, Env, &InferableSlotsConstraint);
+ Evidence::Kind ArgEvidenceKind;
+ switch (ArgNullability) {
+ case NullabilityKind::Nullable:
+ ArgEvidenceKind = Evidence::NULLABLE_ARGUMENT;
+ break;
+ case NullabilityKind::NonNull:
+ ArgEvidenceKind = Evidence::NONNULL_ARGUMENT;
+ break;
+ default:
+ ArgEvidenceKind = Evidence::UNKNOWN_ARGUMENT;
+ }
+ Emit(CalleeDecl, paramSlot(ParamI), ArgEvidenceKind, ArgLoc);
+ }
}
}
@@ -313,7 +323,7 @@
if (!CallExpr || !CallExpr->getCalleeDecl()) return;
auto *CalleeDecl =
dyn_cast_or_null<clang::FunctionDecl>(CallExpr->getCalleeDecl());
- if (!CalleeDecl || !isInferenceTarget(*CalleeDecl)) return;
+ if (!CalleeDecl) return;
collectEvidenceFromArgsAndParams(*CalleeDecl, *CallExpr, InferableCallerSlots,
InferableSlotsConstraint, Lattice, Env,
@@ -380,6 +390,7 @@
llvm::function_ref<EvidenceEmitter> Emit) {
auto CFGStmt = Element.getAs<clang::CFGStmt>();
if (!CFGStmt) return;
+ if (!isInferenceTarget(*Env.getCurrentFunc())) return;
// Initialization of new decl.
if (auto *DeclStmt = dyn_cast_or_null<clang::DeclStmt>(CFGStmt->getStmt())) {
diff --git a/nullability/inference/collect_evidence_test.cc b/nullability/inference/collect_evidence_test.cc
index 4df90f8..bf2696a 100644
--- a/nullability/inference/collect_evidence_test.cc
+++ b/nullability/inference/collect_evidence_test.cc
@@ -585,9 +585,14 @@
TEST(CollectEvidenceFromImplementationTest, NotInferenceTarget) {
static constexpr llvm::StringRef Src = R"cc(
+ void isATarget(Nonnull<int*> a);
template <typename T>
T* target(T* p) {
*p;
+ Nonnull<int*> a = p;
+ isATarget(p);
+ target<T>(nullptr);
+ target<int>(nullptr);
return nullptr;
}
@@ -611,7 +616,11 @@
usr_cache),
usr_cache);
if (Err) ADD_FAILURE() << toString(std::move(Err));
- EXPECT_THAT(Results, IsEmpty());
+ // Doesn't collect any evidence for target from target's body, only collects
+ // some for isATarget.
+ EXPECT_THAT(Results, UnorderedElementsAre(
+ evidence(paramSlot(0), Evidence::UNKNOWN_ARGUMENT,
+ functionNamed("isATarget"))));
}
TEST(CollectEvidenceFromImplementationTest, PropagatesPreviousInferences) {
