Consider smart pointers inferable, though we collect no evidence for them yet. This changes the pipeline behavior when using kUnknownExplicit; all unannotated smart pointers will now be considered eligible for an annotation, and in kUnknownExplicit mode, will be annotated Unknown. PiperOrigin-RevId: 634753587 Change-Id: Ic804bc7930a399bc0bcdbf219cb376f81730a118
diff --git a/nullability/inference/collect_evidence_test.cc b/nullability/inference/collect_evidence_test.cc index 65fca77..b3a24d6 100644 --- a/nullability/inference/collect_evidence_test.cc +++ b/nullability/inference/collect_evidence_test.cc
@@ -2435,18 +2435,26 @@ } TEST(EvidenceSitesTest, GlobalVariables) { - TestAST AST(R"cc( + TestAST AST = getInputsWithAnnotationDefinitions(R"cc( +#include <memory> int* x = true ? nullptr : nullptr; int* y; int a; int b = *y; + std::unique_ptr<int> p; + std::unique_ptr<int> q = nullptr; )cc"); auto Sites = EvidenceSites::discover(AST.context()); EXPECT_THAT(Sites.Declarations, - UnorderedElementsAre(declNamed("x"), declNamed("y"))); - EXPECT_THAT(Sites.Definitions, - UnorderedElementsAre(declNamed("x"), declNamed("b"))); + UnorderedElementsAre(declNamed("x"), declNamed("y"), + declNamed("p"), declNamed("q"))); + EXPECT_THAT( + Sites.Definitions, + UnorderedElementsAre( + declNamed("x"), declNamed("b"), + // unique_ptr p has an initializer because of default construction. + declNamed("p"), declNamed("q"))); } TEST(EvidenceSitesTest, StaticMemberVariables) { @@ -2471,15 +2479,19 @@ } TEST(EvidenceSitesTest, NonStaticMemberVariables) { - TestAST AST(R"cc( + TestAST AST = getInputsWithAnnotationDefinitions(R"cc( +#include <memory> struct S { int* a = nullptr; int* b; + std::unique_ptr<int> p = nullptr; + std::unique_ptr<int> q; }; )cc"); auto Sites = EvidenceSites::discover(AST.context()); EXPECT_THAT(Sites.Declarations, - UnorderedElementsAre(declNamed("S::a"), declNamed("S::b"))); + UnorderedElementsAre(declNamed("S::a"), declNamed("S::b"), + declNamed("S::p"), declNamed("S::q"))); EXPECT_THAT(Sites.Definitions, IsEmpty()); }
diff --git a/nullability/inference/eligible_ranges_test.cc b/nullability/inference/eligible_ranges_test.cc index acff441..f878f11 100644 --- a/nullability/inference/eligible_ranges_test.cc +++ b/nullability/inference/eligible_ranges_test.cc
@@ -235,17 +235,15 @@ )"); EXPECT_THAT(getFunctionRanges(Input.code()), Optional(TypeLocRanges( - MainFileName, UnorderedElementsAre( - SlotRange(1, Input.range("one_o")), - SlotRange(-1, Input.range("one_i")), - // TODO(b/330702908) When supported, Slot 2. - SlotRange(-1, Input.range("two_o")), - SlotRange(-1, Input.range("two_i")), - SlotRange(3, Input.range("three_o")), - SlotRange(-1, Input.range("three_i")), - // TODO(b/330702908) When supported, Slot 4. - SlotRange(-1, Input.range("four_o")), - SlotRange(-1, Input.range("four_i")))))); + MainFileName, + UnorderedElementsAre(SlotRange(1, Input.range("one_o")), + SlotRange(-1, Input.range("one_i")), + SlotRange(2, Input.range("two_o")), + SlotRange(-1, Input.range("two_i")), + SlotRange(3, Input.range("three_o")), + SlotRange(-1, Input.range("three_i")), + SlotRange(4, Input.range("four_o")), + SlotRange(-1, Input.range("four_i")))))); } TEST(EligibleRangesTest, NestedPointersInnerAnnotated) { @@ -263,17 +261,15 @@ )"); EXPECT_THAT(getFunctionRanges(Input.code()), Optional(TypeLocRanges( - MainFileName, UnorderedElementsAre( - SlotRange(1, Input.range("one_o")), - SlotRange(-1, Input.range("one_i")), - // TODO(b/330702908) When supported, Slot 2. - SlotRange(-1, Input.range("two_o")), - SlotRange(-1, Input.range("two_i")), - SlotRange(3, Input.range("three_o")), - SlotRange(-1, Input.range("three_i")), - // TODO(b/330702908) When supported, Slot 4. - SlotRange(-1, Input.range("four_o")), - SlotRange(-1, Input.range("four_i")))))); + MainFileName, + UnorderedElementsAre(SlotRange(1, Input.range("one_o")), + SlotRange(-1, Input.range("one_i")), + SlotRange(2, Input.range("two_o")), + SlotRange(-1, Input.range("two_i")), + SlotRange(3, Input.range("three_o")), + SlotRange(-1, Input.range("three_i")), + SlotRange(4, Input.range("four_o")), + SlotRange(-1, Input.range("four_i")))))); } TEST(EligibleRangesTest, RefToPointer) { @@ -329,13 +325,11 @@ void foo($one[[std::unique_ptr<int>]] std_smart, Nonnull<$two[[std::unique_ptr<int>]]> nonnull_std_smart); )"); - EXPECT_THAT( - getFunctionRanges(Input.code()), - Optional(TypeLocRanges( - MainFileName, UnorderedElementsAre( - // TODO(b/330702908) When supported, Slots 1 and 2. - SlotRange(-1, Input.range("one")), - SlotRange(-1, Input.range("two")))))); + EXPECT_THAT(getFunctionRanges(Input.code()), + Optional(TypeLocRanges( + MainFileName, + UnorderedElementsAre(SlotRange(1, Input.range("one")), + SlotRange(2, Input.range("two")))))); } TEST(EligibleRangesTest, UserDefinedSmartPointer) { @@ -348,13 +342,11 @@ void foo($one[[MySmartIntPtr]] user_defined_smart, Nonnull<$two[[MySmartIntPtr]]> nonnull_user_defined_smart); )"); - EXPECT_THAT( - getFunctionRanges(Input.code()), - Optional(TypeLocRanges( - MainFileName, UnorderedElementsAre( - // TODO(b/330702908) When supported, Slots 1 and 2. - SlotRange(-1, Input.range("one")), - SlotRange(-1, Input.range("two")))))); + EXPECT_THAT(getFunctionRanges(Input.code()), + Optional(TypeLocRanges( + MainFileName, + UnorderedElementsAre(SlotRange(1, Input.range("one")), + SlotRange(2, Input.range("two")))))); } TEST(EligibleRangesTest, UserDefinedTemplatedSmartPointer) { @@ -367,13 +359,11 @@ void foo($one[[MySmartPtr<int>]] user_defined_smart, Nonnull<$two[[MySmartPtr<int>]]> nonnull_user_defined_smart); )"); - EXPECT_THAT( - getFunctionRanges(Input.code()), - Optional(TypeLocRanges( - MainFileName, UnorderedElementsAre( - // TODO(b/330702908) When supported, Slots 1 and 2. - SlotRange(-1, Input.range("one")), - SlotRange(-1, Input.range("two")))))); + EXPECT_THAT(getFunctionRanges(Input.code()), + Optional(TypeLocRanges( + MainFileName, + UnorderedElementsAre(SlotRange(1, Input.range("one")), + SlotRange(2, Input.range("two")))))); } TEST(EligibleRangesTest, SimpleAlias) { @@ -643,7 +633,7 @@ EXPECT_THAT(getFunctionRanges(Input.code()), Optional(TypeLocRanges( MainFileName, - UnorderedElementsAre(SlotRange(-1, Input.range("unique_ptr")), + UnorderedElementsAre(SlotRange(1, Input.range("unique_ptr")), SlotRange(-1, Input.range("inner")))))); }
diff --git a/nullability/inference/infer_tu_test.cc b/nullability/inference/infer_tu_test.cc index 0a11b3f..6c14260 100644 --- a/nullability/inference/infer_tu_test.cc +++ b/nullability/inference/infer_tu_test.cc
@@ -522,8 +522,8 @@ )cc"); // TODO(b/304963199): Currently not inferring anything because we don't - // support smart pointers. The expected result is the same as for the - // `ParamsFromCallSite` test. + // collect evidence for smart pointers. The expected result is the same as for + // the `ParamsFromCallSite` test. ASSERT_THAT(infer(), IsEmpty()); } @@ -533,7 +533,8 @@ std::unique_ptr<int> target() { return std::unique_ptr<int>(); } )cc"); // TODO(b/304963199): Currently not inferring anything because we don't - // support smart pointers. The expected result is a nullable return type. + // collect evidence for smart pointers. The expected result is a nullable + // return type. EXPECT_THAT(infer(), IsEmpty()); } @@ -543,7 +544,8 @@ std::unique_ptr<int> target() { return std::make_unique<int>(0); } )cc"); // TODO(b/304963199): Currently not inferring anything because we don't - // support smart pointers. The expected result is a nonnull return type. + // collect evidence for smart pointers. The expected result is a nonnull + // return type. EXPECT_THAT(infer(), IsEmpty()); }
diff --git a/nullability/inference/inferable.cc b/nullability/inference/inferable.cc index ea85898..a9ac46e 100644 --- a/nullability/inference/inferable.cc +++ b/nullability/inference/inferable.cc
@@ -15,7 +15,7 @@ namespace clang::tidy::nullability { bool hasInferable(QualType T) { - return isSupportedRawPointerType(T.getNonReferenceType()); + return isSupportedPointerType(T.getNonReferenceType()); } int countInferableSlots(const Decl& D) {
diff --git a/nullability/inference/inferable_test.cc b/nullability/inference/inferable_test.cc index 0b08e50..4650c66 100644 --- a/nullability/inference/inferable_test.cc +++ b/nullability/inference/inferable_test.cc
@@ -51,45 +51,74 @@ return *Match; } +constexpr llvm::StringRef SmartPointerHeader = R"cc( + namespace std { + template <typename T> + struct unique_ptr { + using pointer = T*; + }; + } // namespace std + + template <typename T> + struct custom_smart_ptr { + using absl_nullability_compatible = void; + using pointer = T*; + }; +)cc"; + TEST(IsInferenceTargetTest, GlobalVariables) { - TestAST AST(R"cc( - int* Pointer; - int NotPointer; - )cc"); + TestAST AST((SmartPointerHeader + R"cc( + int* Pointer; + std::unique_ptr<int> StdSmartPointer; + custom_smart_ptr<int> CustomSmartPointer; + int NotPointer; + )cc") + .str()); auto &Ctx = AST.context(); EXPECT_TRUE(isInferenceTarget(lookup("Pointer", Ctx))); + EXPECT_TRUE(isInferenceTarget(lookup("StdSmartPointer", Ctx))); + EXPECT_TRUE(isInferenceTarget(lookup("CustomSmartPointer", Ctx))); EXPECT_FALSE(isInferenceTarget(lookup("NotPointer", Ctx))); } TEST(IsInferenceTargetTest, Functions) { - TestAST AST(R"cc( - int* func(int*, int**) { - int* Local; - static int* StaticLocal; - } - void empty() {} - auto Lambda = []() {}; - )cc"); + TestAST AST((SmartPointerHeader + R"cc( + int* func(int*, int**, std::unique_ptr<int>, + custom_smart_ptr<int>) { + int* Local; + static int* StaticLocal; + std::unique_ptr<int> StdSmartLocal; + custom_smart_ptr<int> CustomSmartLocal; + } + void empty() {} + auto Lambda = []() {}; + )cc") + .str()); auto &Ctx = AST.context(); EXPECT_TRUE(isInferenceTarget(lookup("func", Ctx))); EXPECT_FALSE(isInferenceTarget(lookup("Local", Ctx))); 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_FALSE(isInferenceTarget(lookup("operator()", Ctx))); } TEST(IsInferenceTargetTest, ClassAndMembers) { - TestAST AST(R"cc( - class C { - void method(); - int NonPtrField; - int* PtrField; - static int* StaticField; - }; - )cc"); + TestAST AST((SmartPointerHeader + R"cc( + class C { + void method(); + int NonPtrField; + int* PtrField; + static int* StaticField; + std::unique_ptr<int> StdSmartField; + custom_smart_ptr<int> CustomSmartField; + }; + )cc") + .str()); auto &Ctx = AST.context(); EXPECT_FALSE(isInferenceTarget(lookup<CXXRecordDecl>("C", Ctx))); @@ -97,6 +126,8 @@ EXPECT_FALSE(isInferenceTarget(lookup("NonPtrField", Ctx))); EXPECT_TRUE(isInferenceTarget(lookup("PtrField", Ctx))); EXPECT_TRUE(isInferenceTarget(lookup("StaticField", Ctx))); + EXPECT_TRUE(isInferenceTarget(lookup("StdSmartField", Ctx))); + EXPECT_TRUE(isInferenceTarget(lookup("CustomSmartField", Ctx))); } TEST(IsInferenceTargetTest, FunctionTemplate) { @@ -164,26 +195,31 @@ } TEST(InferableTest, CountInferableSlots) { - TestAST AST(R"cc( - using Pointer = int *; - template <class T> - struct S; - struct T; + TestAST AST((SmartPointerHeader + R"cc( + using Pointer = int *; + template <class T> + struct S; + struct T; - void f1(int *); - void f2(Pointer); - void f3(int **); - void f4(Pointer *); - void f5(int *&); - void f6(int (*)()); // function pointer + void f1(int *); + void f2(Pointer); + void f3(int **); + void f4(Pointer *); + void f5(int *&); + void f6(int (*)()); // function pointer + void f7(std::unique_ptr<int>); + void f8(custom_smart_ptr<int>); - int *g1(int); - Pointer g2(int); + int *g1(int); + Pointer g2(int); + std::unique_ptr<int> g3(int); + custom_smart_ptr<int> g4(int); - void h1(S<int *>); - void h2(int T::*); // pointer to data member - void h3(int (T::*)()); // pointer to member function - )cc"); + void h1(S<int *>); + void h2(int T::*); // pointer to data member + void h3(int (T::*)()); // pointer to member function + )cc") + .str()); auto &Ctx = AST.context(); // All the 'f's have a single pointer arg. @@ -193,10 +229,14 @@ EXPECT_EQ(1, countInferableSlots(lookup("f4", Ctx))); EXPECT_EQ(1, countInferableSlots(lookup("f5", Ctx))); EXPECT_EQ(1, countInferableSlots(lookup("f6", Ctx))); + EXPECT_EQ(1, countInferableSlots(lookup("f7", Ctx))); + EXPECT_EQ(1, countInferableSlots(lookup("f8", Ctx))); // All the 'g's have a pointer return. EXPECT_EQ(1, countInferableSlots(lookup("g1", Ctx))); EXPECT_EQ(1, countInferableSlots(lookup("g2", Ctx))); + EXPECT_EQ(1, countInferableSlots(lookup("g3", Ctx))); + EXPECT_EQ(1, countInferableSlots(lookup("g4", Ctx))); // The 'h's have types that aren't really pointers. EXPECT_EQ(0, countInferableSlots(lookup("h1", Ctx)));
diff --git a/nullability/type_nullability.h b/nullability/type_nullability.h index 2629b4b..4aa835d 100644 --- a/nullability/type_nullability.h +++ b/nullability/type_nullability.h
@@ -73,8 +73,8 @@ /// Is this exactly a pointer type that we track outer nullability for? /// This unwraps sugar, i.e. it looks at the canonical type. /// -/// (For now, only regular `PointerType`s, in future we should consider -/// supporting pointer-to-member, ObjC pointers, `unique_ptr`, etc). +/// (For now, only regular `PointerType`s and smart pointers, in future we +/// should consider supporting pointer-to-member, ObjC pointers, etc). bool isSupportedPointerType(QualType); /// Is this exactly a raw (non-smart) pointer type that we track outer