Handle aliases (and all sugar) in type => nullability vector computation
When walking a type looking for nullability slots, we assume an unhandled type
node has no slots. We weren't handling type aliases, so we'd compute an empty
nullability vector for `T` in `using T = int*;`, oops.
Instead of just handling aliases specifically, add fallback handling for all
sugar types to walk the underlying type.
We can remove the uninteresting specific handling for certain sugar types.
The special behavior for AttributedType still works, of course!
Also added unittest covering getNullabilityAnnotationsFromType, which is the
simplest way to test this change.
This doesn't *nearly* cover all interesting cases, but will at least make it
easier to add more later/migrate some away from __assert_nullability/test
subsequent changes to nullability vector computation.
This bug was the root cause of a problem identified with
https://github.com/google/crubit/commit/7a808ea25d183f5f788c0fb3342ffec99d16c829
When handling a cast from nullopt to a pointer type, that change assumes the
nullability vector of the result type is nonempty (it's a pointer type!) and
assigns to the front of it. But when aliases were involved we got an incorrect
empty vector.
PiperOrigin-RevId: 524813522
diff --git a/nullability_verification/BUILD b/nullability_verification/BUILD
index 737aadb..1b9d492 100644
--- a/nullability_verification/BUILD
+++ b/nullability_verification/BUILD
@@ -70,3 +70,16 @@
"@llvm-project//llvm:Support",
],
)
+
+cc_test(
+ name = "pointer_nullability_test",
+ srcs = ["pointer_nullability_test.cc"],
+ deps = [
+ ":pointer_nullability",
+ "@llvm-project//clang:testing",
+ "@llvm-project//llvm:Support",
+ "@llvm-project//third-party/unittest:gmock",
+ "@llvm-project//third-party/unittest:gtest",
+ "@llvm-project//third-party/unittest:gtest_main",
+ ],
+)
diff --git a/nullability_verification/pointer_nullability.cc b/nullability_verification/pointer_nullability.cc
index 35c5096..0df0ed3 100644
--- a/nullability_verification/pointer_nullability.cc
+++ b/nullability_verification/pointer_nullability.cc
@@ -94,16 +94,24 @@
// - the NullabilityKind may be different, as it derives from type sugar
template <class Impl>
class NullabilityWalker : public TypeVisitor<Impl> {
+ using Base = TypeVisitor<Impl>;
Impl& derived() { return *static_cast<Impl*>(this); }
public:
- void Visit(QualType T) { TypeVisitor<Impl>::Visit(T.getTypePtr()); }
+ void Visit(QualType T) { Base::Visit(T.getTypePtr()); }
void Visit(const TemplateArgument& TA) {
if (TA.getKind() == TemplateArgument::Type) Visit(TA.getAsType());
}
- void VisitElaboratedType(const ElaboratedType* ET) {
- Visit(ET->getNamedType());
+ void VisitType(const Type* T) {
+ // For sugar not explicitly handled below, desugar and continue.
+ // (We need to walk the full structure of the canonical type.)
+ if (auto* Desugar =
+ T->getLocallyUnqualifiedSingleStepDesugaredType().getTypePtr();
+ Desugar != T)
+ return Base::Visit(Desugar);
+
+ Base::VisitType(T);
}
void VisitFunctionProtoType(const FunctionProtoType* FPT) {
@@ -143,8 +151,6 @@
derived().report(PT, NullabilityKind::Unspecified);
Visit(PT->getPointeeType());
}
-
- void VisitParenType(const ParenType* PT) { Visit(PT->getInnerType()); }
};
} // namespace
diff --git a/nullability_verification/pointer_nullability_test.cc b/nullability_verification/pointer_nullability_test.cc
new file mode 100644
index 0000000..bf9f096
--- /dev/null
+++ b/nullability_verification/pointer_nullability_test.cc
@@ -0,0 +1,54 @@
+// Part of the Crubit project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#include "nullability_verification/pointer_nullability.h"
+
+#include "clang/Testing/TestAST.h"
+#include "llvm/ADT/StringRef.h"
+#include "third_party/llvm/llvm-project/third-party/unittest/googlemock/include/gmock/gmock.h"
+#include "third_party/llvm/llvm-project/third-party/unittest/googletest/include/gtest/gtest.h"
+
+namespace clang::tidy::nullability {
+namespace {
+using testing::ElementsAre;
+
+class GetNullabilityAnnotationsFromTypeTest : public ::testing::Test {
+ protected:
+ // C++ declarations prepended before parsing type in nullVec().
+ std::string Preamble;
+
+ // Parses `Type` and returns getNullabilityAnnotationsFromType().
+ std::vector<NullabilityKind> nullVec(llvm::StringRef Type) {
+ clang::TestAST AST((Preamble + "\nusing Target = " + Type + ";").str());
+ auto Target = AST.context().getTranslationUnitDecl()->lookup(
+ &AST.context().Idents.get("Target"));
+ assert(Target.isSingleResult());
+ QualType TargetType =
+ AST.context().getTypedefType(Target.find_first<TypeAliasDecl>());
+ return getNullabilityAnnotationsFromType(TargetType);
+ }
+};
+
+TEST_F(GetNullabilityAnnotationsFromTypeTest, Pointers) {
+ EXPECT_THAT(nullVec("int"), ElementsAre());
+ EXPECT_THAT(nullVec("int *"), ElementsAre(NullabilityKind::Unspecified));
+ EXPECT_THAT(nullVec("int **"), ElementsAre(NullabilityKind::Unspecified,
+ NullabilityKind::Unspecified));
+ EXPECT_THAT(nullVec("int *_Nullable*_Nonnull"),
+ ElementsAre(NullabilityKind::NonNull, NullabilityKind::Nullable));
+}
+
+TEST_F(GetNullabilityAnnotationsFromTypeTest, Sugar) {
+ Preamble = "using X = int* _Nonnull;";
+
+ EXPECT_THAT(nullVec("X"), ElementsAre(NullabilityKind::NonNull));
+ EXPECT_THAT(nullVec("X*"), ElementsAre(NullabilityKind::Unspecified,
+ NullabilityKind::NonNull));
+
+ EXPECT_THAT(nullVec("X(*)"), ElementsAre(NullabilityKind::Unspecified,
+ NullabilityKind::NonNull));
+}
+
+} // namespace
+} // namespace clang::tidy::nullability
diff --git a/nullability_verification/test/BUILD b/nullability_verification/test/BUILD
index 08f3302..ec242aa 100644
--- a/nullability_verification/test/BUILD
+++ b/nullability_verification/test/BUILD
@@ -29,6 +29,16 @@
)
cc_test(
+ name = "aliases",
+ srcs = ["aliases.cc"],
+ deps = [
+ ":check_diagnostics",
+ "@llvm-project//third-party/unittest:gtest",
+ "@llvm-project//third-party/unittest:gtest_main",
+ ],
+)
+
+cc_test(
name = "basic",
srcs = ["basic.cc"],
deps = [
diff --git a/nullability_verification/test/aliases.cc b/nullability_verification/test/aliases.cc
new file mode 100644
index 0000000..0f52df8
--- /dev/null
+++ b/nullability_verification/test/aliases.cc
@@ -0,0 +1,30 @@
+// Part of the Crubit project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+// Tests for nullability information hidden behind aliases.
+
+#include "nullability_verification/test/check_diagnostics.h"
+#include "third_party/llvm/llvm-project/third-party/unittest/googletest/include/gtest/gtest.h"
+
+namespace clang::tidy::nullability {
+namespace {
+
+TEST(PointerNullabilityTest, Aliases) {
+ EXPECT_TRUE(checkDiagnostics(R"cc(
+ template <typename T>
+ struct Factory {
+ T get();
+ };
+ using NeverNull = int* _Nonnull;
+ using MaybeNull = int* _Nullable;
+
+ void target(Factory<NeverNull> never, Factory<MaybeNull> maybe) {
+ *never.get();
+ *maybe.get(); // [[unsafe]]
+ }
+ )cc"));
+}
+
+} // namespace
+} // namespace clang::tidy::nullability
\ No newline at end of file
