Don't match non-member binary operator* as if it was a unary operator* for smart pointers
There might not be a receiver arg that is a pointer (failing an assert).
PiperOrigin-RevId: 698390811
Change-Id: I6a05a546d247299383ce701e2ee5114d3f00c763
diff --git a/nullability/pointer_nullability_analysis.cc b/nullability/pointer_nullability_analysis.cc
index c2e2793..3cf7713 100644
--- a/nullability/pointer_nullability_analysis.cc
+++ b/nullability/pointer_nullability_analysis.cc
@@ -665,10 +665,8 @@
// here as well to get the same behavior:
// https://github.com/llvm/llvm-project/blob/a58c3d3ac7c6b2fd9710ab2189d7971ef37e714f/clang/include/clang/ASTMatchers/ASTMatchers.h#L4563
const Expr *Receiver = OpCall->getArg(0)->IgnoreImpCasts();
- if (Receiver->isPRValue()) {
- assert(Receiver->getType()->isPointerType());
+ if (Receiver->isPRValue() && Receiver->getType()->isPointerType())
return Receiver->getType()->getPointeeType();
- }
return Receiver->getType();
}
@@ -1810,7 +1808,7 @@
transferValue_PointerAddOrSubAssign)
.CaseOfCFGStmt<CXXConstructExpr>(isSmartPointerConstructor(),
transferValue_SmartPointerConstructor)
- .CaseOfCFGStmt<CXXOperatorCallExpr>(isSmartPointerOperatorCall("="),
+ .CaseOfCFGStmt<CXXOperatorCallExpr>(isSmartPointerOperatorCall("=", 2),
transferValue_SmartPointerAssignment)
.CaseOfCFGStmt<CXXMemberCallExpr>(isSmartPointerMethodCall("release"),
transferValue_SmartPointerReleaseCall)
@@ -1827,10 +1825,10 @@
isSmartPointerBoolConversionCall(),
transferValue_SmartPointerBoolConversionCall)
.CaseOfCFGStmt<CXXOperatorCallExpr>(
- isSmartPointerOperatorCall("*"),
+ isSmartPointerOperatorCall("*", 1),
transferValue_SmartPointerOperatorStar)
.CaseOfCFGStmt<CXXOperatorCallExpr>(
- isSmartPointerOperatorCall("->"),
+ isSmartPointerOperatorCall("->", 1),
transferValue_SmartPointerOperatorArrow)
.CaseOfCFGStmt<CallExpr>(isSmartPointerFactoryCall(),
transferValue_SmartPointerFactoryCall)
diff --git a/nullability/pointer_nullability_diagnosis.cc b/nullability/pointer_nullability_diagnosis.cc
index 19ce474..58025d6 100644
--- a/nullability/pointer_nullability_diagnosis.cc
+++ b/nullability/pointer_nullability_diagnosis.cc
@@ -553,7 +553,7 @@
isSmartPointerMethodCall("reset"),
unless(hasArgument(0, hasType(isNullPtrType()))),
onImplicitObjectArgument(expr().bind("e"))),
- cxxOperatorCallExpr(isSmartPointerOperatorCall("="),
+ cxxOperatorCallExpr(isSmartPointerOperatorCall("=", 2),
hasArgument(0, expr().bind("e")))))),
*Func->getBody(), Func->getASTContext())) {
AllowedExprs.insert(normalize(Node.getNodeAs<Expr>("e")));
@@ -670,12 +670,12 @@
SmallVector<PointerNullabilityDiagnostic>>()
// `*`
.CaseOfCFGStmt<UnaryOperator>(isPointerDereference(), diagnoseDereference)
- .CaseOfCFGStmt<CXXOperatorCallExpr>(isSmartPointerOperatorCall("*"),
+ .CaseOfCFGStmt<CXXOperatorCallExpr>(isSmartPointerOperatorCall("*", 1),
diagnoseSmartPointerDereference)
// `[]`
.CaseOfCFGStmt<ArraySubscriptExpr>(isPointerSubscript(),
diagnoseSubscript)
- .CaseOfCFGStmt<CXXOperatorCallExpr>(isSmartPointerOperatorCall("[]"),
+ .CaseOfCFGStmt<CXXOperatorCallExpr>(isSmartPointerOperatorCall("[]", 2),
diagnoseSmartPointerDereference)
// `->`. Covers raw and smart pointers, because smart-pointer
// `operator->` doesn't dereference. It just returns a pointer from which
@@ -685,7 +685,7 @@
.CaseOfCFGStmt<BinaryOperator>(
binaryOperator(hasOperatorName("="), hasLHS(isPointerExpr())),
diagnoseAssignment)
- .CaseOfCFGStmt<CXXOperatorCallExpr>(isSmartPointerOperatorCall("="),
+ .CaseOfCFGStmt<CXXOperatorCallExpr>(isSmartPointerOperatorCall("=", 2),
diagnoseSmartPointerAssignment)
.CaseOfCFGStmt<CXXMemberCallExpr>(isSmartPointerMethodCall("reset"),
diagnoseSmartPointerReset)
diff --git a/nullability/pointer_nullability_matchers.cc b/nullability/pointer_nullability_matchers.cc
index c12ae0a..0646ac0 100644
--- a/nullability/pointer_nullability_matchers.cc
+++ b/nullability/pointer_nullability_matchers.cc
@@ -9,7 +9,7 @@
#include "clang/AST/Stmt.h"
#include "clang/AST/Type.h"
#include "clang/ASTMatchers/ASTMatchers.h"
-#include "clang/ASTMatchers/ASTMatchersMacros.h"
+#include "clang/ASTMatchers/ASTMatchersInternal.h"
namespace clang::tidy::nullability {
@@ -142,9 +142,9 @@
return cxxConstructExpr(hasType(isSupportedSmartPointer()));
}
-Matcher<Stmt> isSmartPointerOperatorCall(llvm::StringRef Name) {
+Matcher<Stmt> isSmartPointerOperatorCall(llvm::StringRef Name, int NumArgs) {
return cxxOperatorCallExpr(
- hasOverloadedOperatorName(Name),
+ hasOverloadedOperatorName(Name), argumentCountIs(NumArgs),
hasArgument(0, hasType(isSupportedSmartPointer())));
}
diff --git a/nullability/pointer_nullability_matchers.h b/nullability/pointer_nullability_matchers.h
index 83f0e80..cf6353c 100644
--- a/nullability/pointer_nullability_matchers.h
+++ b/nullability/pointer_nullability_matchers.h
@@ -57,7 +57,7 @@
ast_matchers::internal::Matcher<Stmt> isSmartPointerArrowMemberExpr();
ast_matchers::internal::Matcher<Stmt> isSmartPointerConstructor();
ast_matchers::internal::Matcher<Stmt> isSmartPointerOperatorCall(
- llvm::StringRef Name);
+ llvm::StringRef Name, int NumArgs);
ast_matchers::internal::Matcher<Stmt> isSmartPointerMethodCall(
llvm::StringRef Name);
ast_matchers::internal::Matcher<Stmt> isSmartPointerFreeSwapCall();
diff --git a/nullability/test/smart_pointers.cc b/nullability/test/smart_pointers.cc
index 588cfcf..98004a0 100644
--- a/nullability/test/smart_pointers.cc
+++ b/nullability/test/smart_pointers.cc
@@ -755,3 +755,45 @@
}
} // namespace unusual_smart_pointer_type
+
+// Check non-member operator overloading (check if we have assumptions
+// about presence of receiver arg that may be a pointer type, etc.).
+namespace free_standing_operator_calls {
+
+struct A {
+ int X;
+};
+
+static Nonnull<std::unique_ptr<A>> operator*(std::unique_ptr<A> L,
+ std::unique_ptr<A> R) {
+ return std::make_unique<A>(L->X * R->X);
+}
+
+TEST void nonMemberBinaryOperatorStar() {
+ auto P1 = std::make_unique<A>(1);
+ auto P2 = std::make_unique<A>(2);
+ nonnull(std::move(P1) * std::move(P2));
+}
+
+template <class T>
+struct _Nullable MySmartPtr {
+ using pointer = T *;
+
+ T *get() const;
+ T *Ptr;
+};
+
+// A bit unusual, but one can define a non-member operator*.
+template <typename T>
+static T &operator*(MySmartPtr<T> P) {
+ return *P.Ptr;
+}
+
+TEST void nonMemberUnaryOperatorStar() {
+ int X = 42;
+ Nonnull<int *> PtrToX = &X;
+ MySmartPtr<int *> NonNull = {&PtrToX};
+ provable(NonNull.get() == &*NonNull);
+}
+
+} // namespace free_standing_operator_calls
diff --git a/nullability/test/smart_pointers_diagnosis.cc b/nullability/test/smart_pointers_diagnosis.cc
index c48f7f9..d1b0c50 100644
--- a/nullability/test/smart_pointers_diagnosis.cc
+++ b/nullability/test/smart_pointers_diagnosis.cc
@@ -463,6 +463,26 @@
)cc"));
}
+TEST(SmartPointerTest, DereferenceViaNonMemberStar) {
+ EXPECT_TRUE(checkDiagnostics(R"cc(
+#include <memory>
+#include <utility>
+ template <class T>
+ struct _Nullable MySmartPtr {
+ using pointer = T*;
+ };
+
+ // A bit unusual, but one can define a non-member operator*.
+ template <typename T>
+ static T& operator*(MySmartPtr<T> P);
+
+ void target() {
+ MySmartPtr<int> Null;
+ *Null; // [[unsafe]]
+ }
+ )cc"));
+}
+
TEST(SmartPointerTest, ArrowOperatorReturnsPointerThatNeedsNullState) {
EXPECT_TRUE(checkDiagnostics(R"cc(
// Take by reference to avoid an implicit cast of the argument to rvalue.
