[nullability] Use `isSupportedPointer()` more widely.
Also:
- Rename to `isSupportedPointerType()` for consistency with
`QualType::isPointerType()`.
- Make the function desugar. We'll want to desugar in most places we use this,
and leaving the desugaring to the caller means the caller might forget.
We do eventually want a version that doesn't desugar, so that we can decide
whether we can rewrite a type or not, but that version should probably deal
in `TypeLoc` instead of `QualType`.
- Add a corresponding AST matcher (called just `isSupportedPointer()` for
consistency with `ast_matchers::isAnyPointer()`)
Use `isSupportedPointerType()` wherever we had been using `isPointerType()` or
`isAnyPointerType()`. (The latter was in widespread use but also includes
Objective-C pointers, which we don't currently actually support.)
PiperOrigin-RevId: 559377210
Change-Id: I69b44ce2ca78cf8e47a1061255ab797bfb68e374
diff --git a/nullability/BUILD b/nullability/BUILD
index ae4b293..0ee6158 100644
--- a/nullability/BUILD
+++ b/nullability/BUILD
@@ -26,6 +26,7 @@
"//nullability/inference:__pkg__",
],
deps = [
+ ":type_nullability",
"@llvm-project//clang:ast",
"@llvm-project//clang:ast_matchers",
],
diff --git a/nullability/inference/collect_evidence.cc b/nullability/inference/collect_evidence.cc
index 90a7a28..a4efd97 100644
--- a/nullability/inference/collect_evidence.cc
+++ b/nullability/inference/collect_evidence.cc
@@ -168,16 +168,12 @@
// For each pointer parameter of the callee, ...
for (; ParamI < CalleeDecl->param_size(); ++ParamI, ++ArgI) {
- if (!CalleeDecl->getParamDecl(ParamI)
- ->getType()
- .getNonReferenceType()
- ->isPointerType())
+ if (!isSupportedPointerType(
+ CalleeDecl->getParamDecl(ParamI)->getType().getNonReferenceType()))
continue;
// the corresponding argument should also be a pointer.
- CHECK(CallExpr->getArg(ArgI)
- ->getType()
- .getNonReferenceType()
- ->isPointerType());
+ CHECK(isSupportedPointerType(
+ CallExpr->getArg(ArgI)->getType().getNonReferenceType()));
dataflow::PointerValue *PV =
getPointerValueFromExpr(CallExpr->getArg(ArgI), Env);
@@ -222,7 +218,7 @@
auto *ReturnStmt = dyn_cast_or_null<clang::ReturnStmt>(CFGStmt->getStmt());
if (!ReturnStmt) return;
auto *ReturnExpr = ReturnStmt->getRetValue();
- if (!ReturnExpr || !ReturnExpr->getType()->isPointerType()) return;
+ if (!ReturnExpr || !isSupportedPointerType(ReturnExpr->getType())) return;
NullabilityKind ReturnNullability =
getNullability(ReturnExpr, Env,
@@ -254,7 +250,7 @@
}
std::optional<Evidence::Kind> evidenceKindFromDeclaredType(QualType T) {
- if (!T.getNonReferenceType()->isPointerType()) return std::nullopt;
+ if (!isSupportedPointerType(T.getNonReferenceType())) return std::nullopt;
auto Nullability = getNullabilityAnnotationsFromType(T);
switch (Nullability.front().concrete()) {
default:
@@ -289,7 +285,7 @@
auto Parameters = Func->parameters();
for (auto I = 0; I < Parameters.size(); ++I) {
auto T = Parameters[I]->getType().getNonReferenceType();
- if (T->isPointerType() && !evidenceKindFromDeclaredType(T)) {
+ if (isSupportedPointerType(T) && !evidenceKindFromDeclaredType(T)) {
InferrableSlots.push_back(
std::make_pair(Analysis.assignNullabilityVariable(
Parameters[I], AnalysisContext.arena()),
diff --git a/nullability/inference/inferrable.cc b/nullability/inference/inferrable.cc
index 2adb12a..e41c6e6 100644
--- a/nullability/inference/inferrable.cc
+++ b/nullability/inference/inferrable.cc
@@ -15,7 +15,7 @@
namespace {
bool isInferrable(QualType T) {
- return isSupportedPointer(T.getNonReferenceType().getCanonicalType());
+ return isSupportedPointerType(T.getNonReferenceType());
}
} // namespace
diff --git a/nullability/pointer_nullability_analysis.cc b/nullability/pointer_nullability_analysis.cc
index 846b90b..344df00 100644
--- a/nullability/pointer_nullability_analysis.cc
+++ b/nullability/pointer_nullability_analysis.cc
@@ -389,7 +389,7 @@
}
}
- if (CallExpr->getType()->isAnyPointerType()) {
+ if (isSupportedPointerType(CallExpr->getType())) {
// Create a pointer so that we can attach nullability to it and have the
// nullability propagate with the pointer.
auto *PointerVal = getPointerValueFromExpr(CallExpr, State.Env);
@@ -677,8 +677,8 @@
computeNullability(ASE, State, [&]() {
auto &BaseNullability = getNullabilityForChild(ASE->getBase(), State);
QualType BaseType = ASE->getBase()->getType();
- CHECK(BaseType->isAnyPointerType() || BaseType->isVectorType());
- return BaseType->isAnyPointerType()
+ CHECK(isSupportedPointerType(BaseType) || BaseType->isVectorType());
+ return isSupportedPointerType(BaseType)
? ArrayRef(BaseNullability).slice(1).vec()
: BaseNullability;
});
@@ -747,7 +747,8 @@
if (!S) return;
auto *E = dyn_cast<Expr>(S->getStmt());
- if (E == nullptr || !E->isPRValue() || !E->getType()->isPointerType()) return;
+ if (E == nullptr || !E->isPRValue() || !isSupportedPointerType(E->getType()))
+ return;
if (Env.getValue(*E) == nullptr)
// `createValue()` always produces a value for pointer types.
@@ -821,7 +822,7 @@
const Environment &Env2,
Value &MergedVal,
Environment &MergedEnv) {
- if (!Type->isAnyPointerType()) {
+ if (!isSupportedPointerType(Type)) {
return false;
}
diff --git a/nullability/pointer_nullability_diagnosis.cc b/nullability/pointer_nullability_diagnosis.cc
index 75d1a2b..b33fdf5 100644
--- a/nullability/pointer_nullability_diagnosis.cc
+++ b/nullability/pointer_nullability_diagnosis.cc
@@ -63,7 +63,7 @@
SmallVector<PointerNullabilityDiagnostic> diagnoseTypeExprCompatibility(
QualType DeclaredType, const Expr *E, const Environment &Env,
ASTContext &Ctx) {
- CHECK(DeclaredType->isAnyPointerType());
+ CHECK(isSupportedPointerType(DeclaredType));
return getNullabilityKind(DeclaredType, Ctx) == NullabilityKind::NonNull
? diagnoseNonnullExpected(E, Env)
: SmallVector<PointerNullabilityDiagnostic>{};
@@ -96,7 +96,7 @@
SmallVector<PointerNullabilityDiagnostic> Diagnostics;
for (unsigned int I = 0; I < Args.size(); ++I) {
auto ParamType = ParamTypes[I].getNonReferenceType();
- if (ParamType->isAnyPointerType())
+ if (isSupportedPointerType(ParamType))
Diagnostics.append(
diagnoseTypeExprCompatibility(ParamType, Args[I], Env, Ctx));
}
@@ -260,12 +260,12 @@
auto ReturnType = cast<FunctionDecl>(State.Env.getDeclCtx())->getReturnType();
// TODO: Handle non-pointer return types.
- if (!ReturnType->isPointerType()) {
+ if (!isSupportedPointerType(ReturnType)) {
return {};
}
auto *ReturnExpr = RS->getRetValue();
- CHECK(ReturnExpr->getType()->isPointerType());
+ CHECK(isSupportedPointerType(ReturnExpr->getType()));
return diagnoseTypeExprCompatibility(ReturnType, ReturnExpr, State.Env,
*Result.Context);
@@ -276,7 +276,7 @@
const TransferStateForDiagnostics<PointerNullabilityLattice> &State) {
CHECK(CI->isAnyMemberInitializer());
auto MemberType = CI->getAnyMember()->getType();
- if (!MemberType->isAnyPointerType()) return {};
+ if (!isSupportedPointerType(MemberType)) return {};
auto *MemberInitExpr = CI->getInit();
return diagnoseTypeExprCompatibility(MemberType, MemberInitExpr, State.Env,
diff --git a/nullability/pointer_nullability_matchers.cc b/nullability/pointer_nullability_matchers.cc
index dda8929..be9ca0f 100644
--- a/nullability/pointer_nullability_matchers.cc
+++ b/nullability/pointer_nullability_matchers.cc
@@ -26,7 +26,6 @@
using ast_matchers::hasType;
using ast_matchers::hasUnaryOperand;
using ast_matchers::implicitCastExpr;
-using ast_matchers::isAnyPointer;
using ast_matchers::isArrow;
using ast_matchers::isMemberInitializer;
using ast_matchers::memberExpr;
@@ -34,7 +33,7 @@
using ast_matchers::unaryOperator;
using ast_matchers::internal::Matcher;
-Matcher<Stmt> isPointerExpr() { return expr(hasType(isAnyPointer())); }
+Matcher<Stmt> isPointerExpr() { return expr(hasType(isSupportedPointer())); }
Matcher<Stmt> isNullPointerLiteral() {
return implicitCastExpr(anyOf(hasCastKind(CK_NullToPointer),
hasCastKind(CK_NullToMemberPointer)));
@@ -51,13 +50,13 @@
return implicitCastExpr(hasCastKind(CK_PointerToBoolean));
}
Matcher<Stmt> isMemberOfPointerType() {
- return memberExpr(hasType(isAnyPointer()));
+ return memberExpr(hasType(isSupportedPointer()));
}
Matcher<Stmt> isPointerArrow() { return memberExpr(isArrow()); }
Matcher<Stmt> isCXXThisExpr() { return cxxThisExpr(); }
Matcher<Stmt> isCallExpr() { return callExpr(); }
Matcher<Stmt> isPointerReturn() {
- return returnStmt(hasReturnValue(hasType(isAnyPointer())));
+ return returnStmt(hasReturnValue(hasType(isSupportedPointer())));
}
Matcher<Stmt> isConstructExpr() { return cxxConstructExpr(); }
Matcher<CXXCtorInitializer> isCtorMemberInitializer() {
diff --git a/nullability/pointer_nullability_matchers.h b/nullability/pointer_nullability_matchers.h
index 353bbe8..ea0ed26 100644
--- a/nullability/pointer_nullability_matchers.h
+++ b/nullability/pointer_nullability_matchers.h
@@ -5,12 +5,18 @@
#ifndef CRUBIT_NULLABILITY_POINTER_NULLABILITY_MATCHERS_H_
#define CRUBIT_NULLABILITY_POINTER_NULLABILITY_MATCHERS_H_
+#include "nullability/type_nullability.h"
#include "clang/ASTMatchers/ASTMatchersInternal.h"
+#include "clang/ASTMatchers/ASTMatchersMacros.h"
namespace clang {
namespace tidy {
namespace nullability {
+AST_MATCHER(QualType, isSupportedPointer) {
+ return isSupportedPointerType(Node);
+}
+
ast_matchers::internal::Matcher<Stmt> isPointerExpr();
ast_matchers::internal::Matcher<Stmt> isMemberOfPointerType();
ast_matchers::internal::Matcher<Stmt> isPointerArrow();
diff --git a/nullability/type_nullability.cc b/nullability/type_nullability.cc
index 81c07fe..af998f3 100644
--- a/nullability/type_nullability.cc
+++ b/nullability/type_nullability.cc
@@ -28,7 +28,7 @@
namespace clang::tidy::nullability {
-bool isSupportedPointer(QualType T) { return isa<PointerType>(T); }
+bool isSupportedPointerType(QualType T) { return T->isPointerType(); }
PointerTypeNullability PointerTypeNullability::createSymbolic(
dataflow::Arena &A) {
diff --git a/nullability/type_nullability.h b/nullability/type_nullability.h
index 1ba0bab..8605452 100644
--- a/nullability/type_nullability.h
+++ b/nullability/type_nullability.h
@@ -43,11 +43,12 @@
namespace clang::tidy::nullability {
/// Is this exactly a pointer type that we track outer nullability for?
+/// This unwraps sugar, i.e. it looks at the canonical type.
/// Does not unwrap sugar, consider isSupportedPointer(T.getCanonicalType()).
///
-/// (For now, only regular PointerTypes, in future we should consider supporting
-/// pointer-to-member, ObjC pointers, etc).
-bool isSupportedPointer(QualType);
+/// (For now, only regular `PointerType`s, in future we should consider
+/// supporting pointer-to-member, ObjC pointers, `unique_ptr`, etc).
+bool isSupportedPointerType(QualType);
/// Describes the nullability contract of a pointer "slot" within a type.
///
diff --git a/nullability/type_nullability_test.cc b/nullability/type_nullability_test.cc
index 33eb23a..e058bf3 100644
--- a/nullability/type_nullability_test.cc
+++ b/nullability/type_nullability_test.cc
@@ -19,7 +19,7 @@
namespace {
using testing::ElementsAre;
-TEST(TypeNullabilityTest, IsSupportedPointer) {
+TEST(TypeNullabilityTest, IsSupportedPointerType) {
TestAST AST(R"cpp(
using NotPointer = int;
using Pointer = NotPointer*;
@@ -44,14 +44,14 @@
EXPECT_TRUE(Lookup.isSingleResult());
return Lookup.find_first<TypeAliasDecl>()->getUnderlyingType();
};
- EXPECT_FALSE(isSupportedPointer(Underlying("NotPointer")));
- EXPECT_TRUE(isSupportedPointer(Underlying("Pointer")));
- EXPECT_TRUE(isSupportedPointer(Underlying("FuncPointer")));
- EXPECT_FALSE(isSupportedPointer(Underlying("SugaredPointer")));
- EXPECT_FALSE(isSupportedPointer(Underlying("PointerDataMember")));
- EXPECT_FALSE(isSupportedPointer(Underlying("PointerMemberFunction")));
- EXPECT_FALSE(isSupportedPointer(Underlying("ObjCPointer")));
- EXPECT_FALSE(isSupportedPointer(Underlying("ContainsPointers")));
+ EXPECT_FALSE(isSupportedPointerType(Underlying("NotPointer")));
+ EXPECT_TRUE(isSupportedPointerType(Underlying("Pointer")));
+ EXPECT_TRUE(isSupportedPointerType(Underlying("FuncPointer")));
+ EXPECT_TRUE(isSupportedPointerType(Underlying("SugaredPointer")));
+ EXPECT_FALSE(isSupportedPointerType(Underlying("PointerDataMember")));
+ EXPECT_FALSE(isSupportedPointerType(Underlying("PointerMemberFunction")));
+ EXPECT_FALSE(isSupportedPointerType(Underlying("ObjCPointer")));
+ EXPECT_FALSE(isSupportedPointerType(Underlying("ContainsPointers")));
}
class GetNullabilityAnnotationsFromTypeTest : public ::testing::Test {
