Change requirements for inference targets and slot counting.
Functions that have no pointers present in their type will no longer be targets. We will still collect evidence from non-target definitions.
Variables and fields that have pointer-containing types but are not simply a raw or smart pointer will now be considered targets. However, these pointer types within a more complex type will not be counted as inferable slots, as we currently attempt no inference for them.
Also properly exclude variable template specializations.
PiperOrigin-RevId: 651445869
Change-Id: I434ce133d1e3a69d0f7ff591ca6cd57d900163c8
diff --git a/nullability/inference/collect_evidence_test.cc b/nullability/inference/collect_evidence_test.cc
index cdd3643..195e3eb 100644
--- a/nullability/inference/collect_evidence_test.cc
+++ b/nullability/inference/collect_evidence_test.cc
@@ -3466,37 +3466,51 @@
TEST(EvidenceSitesTest, Functions) {
TestAST AST(R"cc(
void foo();
- void bar();
- void bar() {}
- void baz() {}
+ int* bar();
+ int* bar() {}
+ void baz(int*) {}
+ void def() {}
struct S {
S() {}
- void member();
+ S(int*) {}
+ void member(int*);
};
- void S::member() {}
+ void S::member(int*) {}
)cc");
auto Sites = EvidenceSites::discover(AST.context());
EXPECT_THAT(
Sites.Declarations,
- UnorderedElementsAre(declNamed("foo"), declNamed("bar"), declNamed("bar"),
- declNamed("baz"), declNamed("S::S"),
- declNamed("S::member"), declNamed("S::member")));
+ UnorderedElementsAre(declNamed("bar"), declNamed("bar"), declNamed("baz"),
+ declNamed("S::S"), declNamed("S::member"),
+ declNamed("S::member")));
EXPECT_THAT(Sites.Definitions,
UnorderedElementsAre(declNamed("bar"), declNamed("baz"),
+ declNamed("def"), declNamed("S::S"),
declNamed("S::S"), declNamed("S::member")));
}
-TEST(EvidenceSitesTest, Lambdas) {
+TEST(EvidenceSitesTest, LambdaNoPtr) {
TestAST AST(R"cc(
- auto Lambda = []() {};
+ auto NoPtrs = []() {};
+ )cc");
+ auto Sites = EvidenceSites::discover(AST.context());
+ EXPECT_THAT(Sites.Declarations, IsEmpty());
+ EXPECT_THAT(Sites.Definitions,
+ UnorderedElementsAre(declNamed("(anonymous class)::operator()"),
+ declNamed("NoPtrs")));
+}
+
+TEST(EvidenceSitesTest, LambdaWithPtr) {
+ TestAST AST(R"cc(
+ auto Ptr = [](int*) {};
)cc");
auto Sites = EvidenceSites::discover(AST.context());
EXPECT_THAT(Sites.Declarations,
UnorderedElementsAre(declNamed("(anonymous class)::operator()")));
EXPECT_THAT(Sites.Definitions,
UnorderedElementsAre(declNamed("(anonymous class)::operator()"),
- declNamed("Lambda")));
+ declNamed("Ptr")));
}
TEST(EvidenceSitesTest, GlobalVariables) {
@@ -3571,60 +3585,56 @@
void foo() {
int* p = nullptr;
static int* q = nullptr;
+ static int* r;
}
)cc");
auto Sites = EvidenceSites::discover(AST.context());
- EXPECT_THAT(Sites.Declarations, UnorderedElementsAre(declNamed("foo")));
+ EXPECT_THAT(Sites.Declarations, IsEmpty());
EXPECT_THAT(Sites.Definitions, UnorderedElementsAre(declNamed("foo")));
}
TEST(EvidenceSitesTest, Templates) {
TestAST AST(R"cc(
template <int I>
- int f() {
+ int f(int*) {
return I;
}
template <>
- int f<1>() {
+ int f<1>(int*) {
return 1;
}
struct S {
template <int I>
- int f() {
+ int f(int*) {
return I;
}
};
template <int I>
struct T {
- int f() { return I; }
+ int f(int*) { return I; }
};
template <int I>
- int v = 1;
+ int* v = nullptr;
- int Unused = f<0>() + f<1>() + S{}.f<0>() + T<0>{}.f() + v<0>;
+ template <>
+ int* v<1> = nullptr;
+
+ int Unused = f<0>(v<0>) + f<1>(v<1>) + S{}.f<0>(nullptr) + T<0>{}.f(nullptr);
)cc");
auto Sites = EvidenceSites::discover(AST.context());
// Relevant declarations are the written ones that are not templates.
- EXPECT_THAT(Sites.Declarations, UnorderedElementsAre(declNamed("f<1>")));
+ EXPECT_THAT(Sites.Declarations,
+ UnorderedElementsAre(declNamed("f<1>"), declNamed("v<1>")));
// Instantiations are relevant definitions, as is the global variable Unused.
EXPECT_THAT(Sites.Definitions,
- UnorderedElementsAre(declNamed("f<0>"), declNamed("f<1>"),
+ UnorderedElementsAre(declNamed("f<0>"), declNamed("v<0>"),
+ declNamed("f<1>"), declNamed("v<1>"),
declNamed("S::f<0>"), declNamed("T<0>::f"),
- declNamed("v<0>"), declNamed("Unused")));
-
- for (auto* Def : Sites.Definitions) {
- std::string Actual;
- llvm::raw_string_ostream OS(Actual);
- if (auto* ND = dyn_cast<NamedDecl>(Def))
- ND->getNameForDiagnostic(
- OS, Def->getDeclContext()->getParentASTContext().getPrintingPolicy(),
- /*Qualified=*/true);
- llvm::errs() << "Actual: " << Actual << "\n";
- }
+ declNamed("Unused")));
}
TEST(EvidenceEmitterTest, NotInferenceTarget) {
diff --git a/nullability/inference/inferable.cc b/nullability/inference/inferable.cc
index 91c268b..ac131e2 100644
--- a/nullability/inference/inferable.cc
+++ b/nullability/inference/inferable.cc
@@ -26,10 +26,10 @@
return Slots;
}
if (const auto* Field = dyn_cast<FieldDecl>(&D)) {
- return isInferenceTarget(*Field) ? 1 : 0;
+ return hasInferable(Field->getType()) ? 1 : 0;
}
if (const auto* Var = dyn_cast<VarDecl>(&D)) {
- return isInferenceTarget(*Var) ? 1 : 0;
+ return hasInferable(Var->getType()) ? 1 : 0;
}
return 0;
}
@@ -57,24 +57,28 @@
// small set of functions.
Func->getBuiltinID() == 0 &&
// Implicit functions cannot be annotated.
- !Func->isImplicit();
+ !Func->isImplicit() &&
+ // Do the most expensive check last.
+ countPointersInType(Func->getType()) > 0;
}
if (const auto* Field = dyn_cast<FieldDecl>(&D)) {
- return hasInferable(Field->getType()) &&
- // See comments above regarding dependent contexts and templates.
- !Field->getDeclContext()->isDependentContext() &&
- !isa<ClassTemplateSpecializationDecl>(Field->getParent());
+ return
+ // See comments above regarding dependent contexts and templates.
+ !Field->getDeclContext()->isDependentContext() &&
+ !isa<ClassTemplateSpecializationDecl>(Field->getParent()) &&
+ // Do the most expensive check last.
+ countPointersInType(Field->getType()) > 0;
}
if (const auto* Var = dyn_cast<VarDecl>(&D)) {
// Include static member variables and global variables, but not static
// local variables. Local variables often do not need annotation in order to
// be verified.
- return hasInferable(Var->getType()) && Var->hasGlobalStorage() &&
- !Var->isStaticLocal() &&
+ return Var->hasGlobalStorage() && !Var->isStaticLocal() &&
// See comments above regarding dependent contexts and templates.
!Var->getDeclContext()->isDependentContext() &&
- !Var->isTemplated() &&
- !isa<ClassTemplateSpecializationDecl>(Var->getDeclContext());
+ !Var->isTemplated() && !Var->getTemplateInstantiationPattern() &&
+ // Do the most expensive check last.
+ countPointersInType(Var->getType()) > 0;
}
return false;
}
diff --git a/nullability/inference/inferable_test.cc b/nullability/inference/inferable_test.cc
index ca920a5..1ab2484 100644
--- a/nullability/inference/inferable_test.cc
+++ b/nullability/inference/inferable_test.cc
@@ -10,6 +10,7 @@
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclTemplate.h"
#include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/Basic/LLVM.h"
@@ -93,6 +94,7 @@
}
void empty() {}
auto Lambda = []() {};
+ auto LambdaWithPtr = [](int*) {};
)cc")
.str());
@@ -102,15 +104,23 @@
EXPECT_FALSE(isInferenceTarget(lookup("StaticLocal", Ctx)));
EXPECT_FALSE(isInferenceTarget(lookup("StdSmartLocal", Ctx)));
EXPECT_FALSE(isInferenceTarget(lookup("CustomSmartLocal", Ctx)));
- EXPECT_TRUE(isInferenceTarget(lookup("empty", Ctx)));
- EXPECT_FALSE(isInferenceTarget(lookup("Lambda", Ctx)));
- EXPECT_TRUE(isInferenceTarget(lookup("operator()", Ctx)));
+ EXPECT_FALSE(isInferenceTarget(lookup("empty", Ctx)));
+ auto &Lambda = lookup<VarDecl>("Lambda", Ctx);
+ auto *LambdaCtx = cast<LambdaExpr>(Lambda.getInit())->getLambdaClass();
+ EXPECT_FALSE(isInferenceTarget(Lambda));
+ EXPECT_FALSE(isInferenceTarget(lookup("operator()", Ctx, LambdaCtx)));
+ auto &LambdaWithPtr = lookup<VarDecl>("LambdaWithPtr", Ctx);
+ auto *LambdaWithPtrCtx =
+ cast<LambdaExpr>(LambdaWithPtr.getInit())->getLambdaClass();
+ EXPECT_FALSE(isInferenceTarget(LambdaWithPtr));
+ EXPECT_TRUE(isInferenceTarget(lookup("operator()", Ctx, LambdaWithPtrCtx)));
}
TEST(IsInferenceTargetTest, ClassAndMembers) {
TestAST AST((SmartPointerHeader + R"cc(
class C {
void method();
+ int* methodWithPtr();
int NonPtrField;
int* PtrField;
static int* StaticField;
@@ -122,7 +132,8 @@
auto &Ctx = AST.context();
EXPECT_FALSE(isInferenceTarget(lookup<CXXRecordDecl>("C", Ctx)));
- EXPECT_TRUE(isInferenceTarget(lookup("method", Ctx)));
+ EXPECT_FALSE(isInferenceTarget(lookup("method", Ctx)));
+ EXPECT_TRUE(isInferenceTarget(lookup("methodWithPtr", Ctx)));
EXPECT_FALSE(isInferenceTarget(lookup("NonPtrField", Ctx)));
EXPECT_TRUE(isInferenceTarget(lookup("PtrField", Ctx)));
EXPECT_TRUE(isInferenceTarget(lookup("StaticField", Ctx)));
@@ -145,11 +156,11 @@
EXPECT_FALSE(isInferenceTarget(FuncTmpl));
EXPECT_FALSE(isInferenceTarget(*FuncTmpl.getTemplatedDecl()));
// The function template specialization is *also* not an inference target.
- const VarDecl &FuncTmplSpec = lookup<VarDecl>("FuncTmplSpec", Ctx);
- EXPECT_FALSE(isInferenceTarget(FuncTmplSpec));
- const ValueDecl &FuncTmpl2 =
- *cast<DeclRefExpr>(FuncTmplSpec.getInit()->IgnoreImplicit())->getDecl();
- EXPECT_FALSE(isInferenceTarget(FuncTmpl2));
+ const ValueDecl &Specialization =
+ *cast<DeclRefExpr>(
+ lookup<VarDecl>("FuncTmplSpec", Ctx).getInit()->IgnoreImplicit())
+ ->getDecl();
+ EXPECT_FALSE(isInferenceTarget(Specialization));
}
TEST(IsInferenceTargetTest, ClassTemplateAndMembers) {
