Propagate nullability for unique pointers created with `absl::WrapUnique`. PiperOrigin-RevId: 820389260 Change-Id: Iccf6b684ffd279324d79c379cc271939475f40c6
diff --git a/nullability/pointer_nullability_matchers.cc b/nullability/pointer_nullability_matchers.cc index 12489da..f132a11 100644 --- a/nullability/pointer_nullability_matchers.cc +++ b/nullability/pointer_nullability_matchers.cc
@@ -181,6 +181,12 @@ "allocate_shared_for_overwrite")))); } +Matcher<Stmt> isWrapUniqueCall() { + return callExpr( + hasType(isSupportedSmartPointer()), + callee(functionDecl(hasName("WrapUnique"), isInAbslNamespace()))); +} + Matcher<Stmt> isSmartPointerComparisonOpCall() { return cxxOperatorCallExpr( hasAnyOverloadedOperatorName("==", "!="), argumentCountIs(2),
diff --git a/nullability/pointer_nullability_matchers.h b/nullability/pointer_nullability_matchers.h index d60c970..cdbfcf3 100644 --- a/nullability/pointer_nullability_matchers.h +++ b/nullability/pointer_nullability_matchers.h
@@ -6,6 +6,7 @@ #define CRUBIT_NULLABILITY_POINTER_NULLABILITY_MATCHERS_H_ #include "nullability/type_nullability.h" +#include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/Expr.h" #include "clang/AST/ExprCXX.h" @@ -84,6 +85,7 @@ ast_matchers::internal::Matcher<Stmt> isSmartPointerFreeSwapCall(); ast_matchers::internal::Matcher<Stmt> isSmartPointerBoolConversionCall(); ast_matchers::internal::Matcher<Stmt> isSmartPointerFactoryCall(); +ast_matchers::internal::Matcher<Stmt> isWrapUniqueCall(); ast_matchers::internal::Matcher<Stmt> isSmartPointerComparisonOpCall(); ast_matchers::internal::Matcher<Stmt> isSharedPtrCastCall(); ast_matchers::internal::Matcher<Stmt> isWeakPtrLockCall(); @@ -97,6 +99,37 @@ Builder); } +// Checks if the given declaration is within the `absl` namespace. +// Traverses the parent namespaces up to the top-level namespace. +// For example, `absl::nested::f()` is considered within the `absl` namespace. +// The logic is similar to `isDeclaredInAbseilOrUtil()` in the value transferer: +// https://github.com/google/crubit/blob/55767d191778d2a421a229d3fe446a65912c9865/nullability/value_transferer.cc#L882 +// This is unlike `Decl::isInStdNamespace()` +// (https://clang.llvm.org/doxygen/classclang_1_1Decl.html#a066b012f94431b5bba21d19715a274f4), +// which only traverses up inline namespaces +// (https://en.cppreference.com/w/cpp/language/namespace.html#Inline_namespaces) +// and "transparent contexts" such as those induced by unscoped enums +// (https://clang.llvm.org/doxygen/classclang_1_1DeclContext.html#a1d3b0ef59e3e789890485aa141c4712e). +AST_MATCHER(Decl, isInAbslNamespace) { + const DeclContext* DC = Node.getDeclContext(); + if (DC == nullptr || DC->isTranslationUnit()) { + return false; + } + + // Traverse the parent namespaces up to the top-level namespace. + while (DC->getParent() != nullptr && !DC->getParent()->isTranslationUnit()) { + DC = DC->getParent(); + } + + if (!DC->isNamespace()) { + return false; + } + + const NamespaceDecl* ND = cast<NamespaceDecl>(DC); + const IdentifierInfo* II = ND->getIdentifier(); + return II != nullptr && II->isStr("absl"); +} + } // namespace nullability } // namespace tidy } // namespace clang
diff --git a/nullability/pointer_nullability_matchers_test.cc b/nullability/pointer_nullability_matchers_test.cc index e68f8f3..a76ffc6 100644 --- a/nullability/pointer_nullability_matchers_test.cc +++ b/nullability/pointer_nullability_matchers_test.cc
@@ -5,6 +5,7 @@ #include "nullability/pointer_nullability_matchers.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Testing/TestAST.h" #include "llvm/ADT/StringRef.h" #include "external/llvm-project/third-party/unittest/googletest/include/gtest/gtest.h" @@ -12,7 +13,20 @@ namespace clang::tidy::nullability { namespace { +using ast_matchers::callee; +using ast_matchers::callExpr; +using ast_matchers::declRefExpr; +using ast_matchers::enumConstantDecl; +using ast_matchers::functionDecl; +using ast_matchers::hasName; using ast_matchers::match; +using ast_matchers::to; + +template <typename MatcherT> +bool matches(llvm::StringRef test_input, MatcherT Matcher) { + TestAST InputAST(test_input.str()); + return !match(Matcher, InputAST.context()).empty(); +} template <typename MatcherT> bool matches(llvm::StringRef base_input, llvm::StringRef test_input, @@ -155,5 +169,199 @@ isSmartPointerBoolConversionCall())); } +TEST(AbslNamespaceTest, MatchesAbslFunctionCall) { + llvm::StringRef Input(R"cc( + namespace absl { + void f() {} + } // namespace absl + )cc"); + auto AbslFunctionCall = + callExpr(callee(functionDecl(isInAbslNamespace(), hasName("f")))); + EXPECT_TRUE(matches(Input, "void target(){ absl::f(); }", AbslFunctionCall)); + EXPECT_TRUE(matches(Input, "using namespace absl; void target(){ f(); }", + AbslFunctionCall)); + EXPECT_TRUE( + matches(Input, "using absl::f; void target(){ f(); }", AbslFunctionCall)); +} + +TEST(AbslNamespaceTest, DoesNotMatchFunctionCallWithNoNamespace) { + llvm::StringRef Input(R"cc( + void f() {} + void target() { f(); } + )cc"); + EXPECT_FALSE(matches(Input, isInAbslNamespace())); +} + +TEST(AbslNamespaceTest, DoesNotMatchFunctionCallWithDifferentNamespace) { + llvm::StringRef Input(R"cc( + namespace util { + void f() {} + } // namespace util + )cc"); + EXPECT_FALSE( + matches(Input, "void target(){ util::f(); }", isInAbslNamespace())); + EXPECT_FALSE(matches(Input, "using namespace util; void target(){ f(); }", + isInAbslNamespace())); + EXPECT_FALSE(matches(Input, "using util::f; void target(){ f(); }", + isInAbslNamespace())); +} + +TEST(AbslNamespaceTest, DoesNotMatchEnumWithAbslType) { + llvm::StringRef Input(R"cc( + enum absl { + kValue, + }; + )cc"); + EXPECT_FALSE( + matches(Input, "void target() { kValue; }", isInAbslNamespace())); + EXPECT_FALSE( + matches(Input, "void target() { absl::kValue; }", isInAbslNamespace())); +} + +TEST(AbslNamespaceTest, DoesNotMatchEnumClassWithQualifiedAbslType) { + llvm::StringRef Input(R"cc( + enum class absl { + kValue, + }; + void target() { absl::kValue; } + )cc"); + EXPECT_FALSE(matches(Input, isInAbslNamespace())); +} + +TEST(AbslNamespaceTest, MatchesEnumInAbslNamespace) { + llvm::StringRef Input(R"cc( + namespace absl { + enum Type { + kValue, + }; + } // namespace absl + )cc"); + EXPECT_TRUE(matches(Input, "void target() { absl::kValue; }", + declRefExpr(to(enumConstantDecl(isInAbslNamespace(), + hasName("kValue")))))); + EXPECT_TRUE(matches(Input, "void target() { absl::Type::kValue; }", + declRefExpr(to(enumConstantDecl(isInAbslNamespace(), + hasName("kValue")))))); +} + +TEST(AbslNamespaceTest, MatchesEnumClassInAbslNamespace) { + llvm::StringRef Input(R"cc( + namespace absl { + enum class Type { + kValue, + }; + void target() { absl::Type::kValue; } + } // namespace absl + )cc"); + EXPECT_TRUE(matches(Input, declRefExpr(to(enumConstantDecl( + isInAbslNamespace(), hasName("kValue")))))); +} + +TEST(AbslNamespaceTest, MatchesFunctionCallWithNestedNamespace) { + llvm::StringRef Input(R"cc( + namespace absl { + namespace nested { + void f() {} + } // namespace nested + } // namespace absl + )cc"); + auto AbslFunctionCall = + callExpr(callee(functionDecl(isInAbslNamespace(), hasName("f")))); + EXPECT_TRUE( + matches(Input, "void target(){ absl::nested::f(); }", AbslFunctionCall)); + EXPECT_TRUE(matches(Input, + "using namespace absl; void target(){ nested::f(); }", + AbslFunctionCall)); + EXPECT_TRUE(matches(Input, + "using namespace absl::nested; void target(){ f(); }", + AbslFunctionCall)); + EXPECT_TRUE(matches(Input, "using absl::nested::f; void target(){ f(); }", + AbslFunctionCall)); +} + +TEST(AbslNamespaceTest, MatchesFunctionCallWithInlineNamespace) { + llvm::StringRef Input(R"cc( + namespace absl { + inline namespace latest { + void f() {} + } // namespace latest + } // namespace absl + )cc"); + auto AbslFunctionCall = + callExpr(callee(functionDecl(isInAbslNamespace(), hasName("f")))); + EXPECT_TRUE(matches(Input, "void target(){ absl::f(); }", AbslFunctionCall)); + EXPECT_TRUE( + matches(Input, "void target(){ absl::latest::f(); }", AbslFunctionCall)); + EXPECT_TRUE(matches(Input, "using namespace absl; void target(){ f(); }", + AbslFunctionCall)); + EXPECT_TRUE(matches(Input, + "using namespace absl; void target(){ latest::f(); }", + AbslFunctionCall)); + EXPECT_TRUE(matches(Input, + "using namespace absl::latest; void target(){ f(); }", + AbslFunctionCall)); + EXPECT_TRUE( + matches(Input, "using absl::f; void target(){ f(); }", AbslFunctionCall)); + EXPECT_TRUE(matches(Input, "using absl::latest::f; void target(){ f(); }", + AbslFunctionCall)); +} + +TEST(AbslNamespaceTest, MatchesFunctionCallWithNestedInlineNamespace) { + llvm::StringRef Input(R"cc( + namespace absl { + inline namespace latest { + inline namespace nested { + void f() {} + } // namespace nested + } // namespace latest + } // namespace absl + )cc"); + auto AbslFunctionCall = + callExpr(callee(functionDecl(isInAbslNamespace(), hasName("f")))); + EXPECT_TRUE(matches(Input, "void target(){ absl::f(); }", AbslFunctionCall)); + EXPECT_TRUE( + matches(Input, "void target(){ absl::latest::f(); }", AbslFunctionCall)); + EXPECT_TRUE(matches(Input, "void target(){ absl::latest::nested::f(); }", + AbslFunctionCall)); +} + +TEST(AbslNamespaceTest, DoesNotMatchFunctionCallWithNestedAbslNamespace) { + llvm::StringRef Input(R"cc( + namespace util { + namespace absl { + void f() {} + } // namespace absl + } // namespace util + )cc"); + EXPECT_FALSE( + matches(Input, "void target(){ util::absl::f(); }", isInAbslNamespace())); + EXPECT_FALSE(matches(Input, + "using namespace util; void target(){ absl::f(); }", + isInAbslNamespace())); + EXPECT_FALSE(matches(Input, + "using namespace util::absl; void target(){ f(); }", + isInAbslNamespace())); +} + +TEST(AbslNamespaceTest, DoesNotMatchFunctionCallWithInlineAbslNamespace) { + llvm::StringRef Input(R"cc( + namespace util { + inline namespace absl { + void f() {} + } // namespace absl + } // namespace util + )cc"); + EXPECT_FALSE( + matches(Input, "void target(){ util::f(); }", isInAbslNamespace())); + EXPECT_FALSE( + matches(Input, "void target(){ util::absl::f(); }", isInAbslNamespace())); + EXPECT_FALSE(matches(Input, + "using namespace util; void target(){ absl::f(); }", + isInAbslNamespace())); + EXPECT_FALSE(matches(Input, + "using namespace util::absl; void target(){ f(); }", + isInAbslNamespace())); +} + } // namespace } // namespace clang::tidy::nullability
diff --git a/nullability/test/smart_pointers.cc b/nullability/test/smart_pointers.cc index 32d288d..5320889 100644 --- a/nullability/test/smart_pointers.cc +++ b/nullability/test/smart_pointers.cc
@@ -414,6 +414,43 @@ nonnull(std::make_unique_for_overwrite<int[]>(5)); } +namespace absl { +template <typename T> +std::unique_ptr<T> WrapUnique(T* P) { + return std::unique_ptr<T>(P); +} +} // namespace absl + +TEST void abslWrapUnique() { + nonnull(absl::WrapUnique(makeNonnullRaw())); + nullable(absl::WrapUnique(makeNullableRaw())); + unknown(absl::WrapUnique(makeUnknownRaw())); +} + +template <typename T> +std::unique_ptr<T> WrapUnique(T* P) { + return std::unique_ptr<T>(P); +} + +TEST void customWrapUnique() { + unknown(WrapUnique(makeNonnullRaw())); + unknown(WrapUnique(makeNullableRaw())); + unknown(WrapUnique(makeUnknownRaw())); +} + +namespace util { +template <typename T> +std::unique_ptr<T> WrapUnique(T* P) { + return std::unique_ptr<T>(P); +} +} // namespace util + +TEST void nonAbslNamespaceWrapUnique() { + unknown(util::WrapUnique(makeNonnullRaw())); + unknown(util::WrapUnique(makeNullableRaw())); + unknown(util::WrapUnique(makeUnknownRaw())); +} + TEST void makeShared() { nonnull(std::make_shared<int>()); nonnull(std::make_shared<int>(42));
diff --git a/nullability/value_transferer.cc b/nullability/value_transferer.cc index 3457c4a..1775eb8 100644 --- a/nullability/value_transferer.cc +++ b/nullability/value_transferer.cc
@@ -552,6 +552,21 @@ } } +static void transferWrapUniqueCall( + const CallExpr* CE, const MatchFinder::MatchResult& Result, + TransferState<PointerNullabilityLattice>& State) { + if (CE->getNumArgs() != 1) { + return; + } + const Expr* Arg = CE->getArg(0); + RecordStorageLocation& Loc = State.Env.getResultObjectLocation(*CE); + if (isSupportedRawPointerType(Arg->getType()) && + isPointerTypeConvertible(Arg->getType(), + underlyingRawPointerTypeFromSmartPointer(Loc))) { + setSmartPointerValue(Loc, getRawPointerValue(Arg, State.Env), State.Env); + } +} + static void transferSmartPointerComparisonOpCall( const CXXOperatorCallExpr* OpCall, const MatchFinder::MatchResult& Result, TransferState<PointerNullabilityLattice>& State) { @@ -1409,6 +1424,7 @@ transferSmartPointerOperatorArrow) .CaseOfCFGStmt<CallExpr>(isSmartPointerFactoryCall(), transferSmartPointerFactoryCall) + .CaseOfCFGStmt<CallExpr>(isWrapUniqueCall(), transferWrapUniqueCall) .CaseOfCFGStmt<CXXOperatorCallExpr>(isSmartPointerComparisonOpCall(), transferSmartPointerComparisonOpCall) .CaseOfCFGStmt<CallExpr>(isSharedPtrCastCall(), transferSharedPtrCastCall)