Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bazel/llvm.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def _llvm_loader_repository(repository_ctx):
executable = False,
)

LLVM_COMMIT_SHA = "4f39a4ff0ada92870ca1c2dccad382ea04947da8"
LLVM_COMMIT_SHA = "0c2701fe7fa002e1befc5f86c268a7964f96d286"

def llvm_loader_repository_dependencies():
# This *declares* the dependency, but it won't actually be *downloaded* unless it's used.
Expand Down
2 changes: 2 additions & 0 deletions nullability/test/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ cc_test(
srcs = ["check_diagnostics_test.cc"],
deps = [
":check_diagnostics",
"//nullability:pointer_nullability_diagnosis",
"@llvm-project//third-party/unittest:gmock",
"@llvm-project//third-party/unittest:gtest",
"@llvm-project//third-party/unittest:gtest_main",
],
Expand Down
50 changes: 39 additions & 11 deletions nullability/test/check_diagnostics.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <array>
#include <iterator>
#include <memory>
#include <optional>
#include <set>
#include <string>
#include <vector>
Expand Down Expand Up @@ -36,25 +37,26 @@

namespace clang::tidy::nullability {

bool checkDiagnostics(ASTContext &AST, llvm::Annotations AnnotatedCode,
const NullabilityPragmas &Pragmas, bool AllowUntracked) {
static std::optional<std::vector<PointerNullabilityDiagnostic>>
checkAndGetDiagnostics(ASTContext& AST, llvm::Annotations AnnotatedCode,
const NullabilityPragmas& Pragmas, bool AllowUntracked) {
using ast_matchers::BoundNodes;
using ast_matchers::hasAnyName;
using ast_matchers::match;
using ast_matchers::stmt;
using ast_matchers::valueDecl;

SmallVector<BoundNodes, 1> MatchResult =
match(valueDecl(hasAnyName("target", "~target")).bind("target"), AST);
if (MatchResult.empty()) {
ADD_FAILURE() << "didn't find target declaration";
return false;
return std::nullopt;
}

bool Success = true;
// We already checked MatchResult is not empty above, but we skip
// isTemplated() functions. Make sure we didn't skip every MatchResult.
bool FoundMatch = false;
std::vector<PointerNullabilityDiagnostic> AllActualDiagnostics;
for (const ast_matchers::BoundNodes &BN : MatchResult) {
const auto *Target = BN.getNodeAs<ValueDecl>("target");
// Skip templates and only analyze instantiations
Expand All @@ -71,7 +73,7 @@ bool checkDiagnostics(ASTContext &AST, llvm::Annotations AnnotatedCode,
if (llvm::Error Err =
diagnosePointerNullability(Target, Pragmas).moveInto(Diagnostics)) {
ADD_FAILURE() << Err;
return false;
return std::nullopt;
}

// Note: use sorted sets for expected and actual lines to improve
Expand All @@ -88,6 +90,7 @@ bool checkDiagnostics(ASTContext &AST, llvm::Annotations AnnotatedCode,
ActualLines.insert(AST.getSourceManager().getPresumedLineNumber(
Diag.Range.getBegin()));
}
AllActualDiagnostics.push_back(Diag);
}
if (ActualLines != ExpectedLines) {
Success = false;
Expand Down Expand Up @@ -126,14 +129,21 @@ bool checkDiagnostics(ASTContext &AST, llvm::Annotations AnnotatedCode,
if (!FoundMatch) {
ADD_FAILURE() << "didn't find target declaration (after skipping templated "
"functions) -- add an instantiation?";
return false;
return std::nullopt;
}

return Success;
return Success ? std::optional(AllActualDiagnostics) : std::nullopt;
}

static bool checkDiagnostics(llvm::StringRef SourceCode, TestLanguage Lang,
bool AllowUntracked) {
bool checkDiagnostics(ASTContext& AST, llvm::Annotations AnnotatedCode,
const NullabilityPragmas& Pragmas, bool AllowUntracked) {
return checkAndGetDiagnostics(AST, AnnotatedCode, Pragmas, AllowUntracked)
.has_value();
}

static std::optional<std::vector<PointerNullabilityDiagnostic>>
checkAndGetDiagnostics(llvm::StringRef SourceCode, TestLanguage Lang,
bool AllowUntracked) {
llvm::Annotations AnnotatedCode(SourceCode);
clang::TestInputs Inputs(AnnotatedCode.code());
Inputs.Language = Lang;
Expand Down Expand Up @@ -163,15 +173,33 @@ static bool checkDiagnostics(llvm::StringRef SourceCode, TestLanguage Lang,
return std::make_unique<Action>(Pragmas);
};
clang::TestAST AST(Inputs);
return checkDiagnostics(AST.context(), AnnotatedCode, Pragmas,
AllowUntracked);
return checkAndGetDiagnostics(AST.context(), AnnotatedCode, Pragmas,
AllowUntracked);
}

static bool checkDiagnostics(llvm::StringRef SourceCode, TestLanguage Lang,
bool AllowUntracked) {
return checkAndGetDiagnostics(SourceCode, Lang, AllowUntracked).has_value();
}

// Run in C++17 and C++20 mode to cover differences in the AST between modes
// (e.g. C++20 can contain `CXXRewrittenBinaryOperator`).
static constexpr std::array<TestLanguage, 2> CXXLanguagesToTest = {
TestLanguage::Lang_CXX17, TestLanguage::Lang_CXX20};

std::optional<std::vector<PointerNullabilityDiagnostic>> checkAndGetDiagnostics(
llvm::StringRef SourceCode) {
std::vector<PointerNullabilityDiagnostic> AllDiagnostics;
for (TestLanguage Lang : CXXLanguagesToTest) {
const auto Diagnostics =
checkAndGetDiagnostics(SourceCode, Lang, /*AllowUntracked=*/false);
if (!Diagnostics.has_value()) return std::nullopt;
AllDiagnostics.insert(AllDiagnostics.end(), Diagnostics->begin(),
Diagnostics->end());
}
return AllDiagnostics;
}

bool checkDiagnostics(llvm::StringRef SourceCode) {
for (TestLanguage Lang : CXXLanguagesToTest)
if (!checkDiagnostics(SourceCode, Lang, /*AllowUntracked=*/false))
Expand Down
14 changes: 12 additions & 2 deletions nullability/test/check_diagnostics.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
#ifndef CRUBIT_NULLABILITY_TEST_CHECK_DIAGNOSTICS_H_
#define CRUBIT_NULLABILITY_TEST_CHECK_DIAGNOSTICS_H_

#include <optional>
#include <vector>

#include "nullability/pointer_nullability_diagnosis.h"
#include "nullability/pragma.h"
#include "clang/AST/ASTContext.h"
#include "clang/Testing/CommandLineArgs.h"
Expand All @@ -18,8 +22,6 @@ namespace nullability {
/// Runs nullability verification on `SourceCode` and returns whether
/// diagnostics are produced on those lines marked in the source code with
/// `llvm::Annotations` style annotations (and no other lines).
/// TODO(mboehme): So far, we only check the locations of the diagnostics; it
/// would be desirable to check their actual content too.
bool checkDiagnostics(llvm::StringRef SourceCode);

/// Same as `checkDiagnostics`, but allows for untracked errors.
Expand All @@ -31,6 +33,14 @@ bool checkDiagnostics(ASTContext& AST, llvm::Annotations AnnotatedCode,
const NullabilityPragmas& Pragmas = NullabilityPragmas(),
bool AllowUntracked = false);

/// Variation of `checkDiagnostics` which returns the ordered list of all actual
/// diagnostics if their locations match those in the source code annotations,
/// and otherwise returns `std::nullopt` to indicate a mismatch or failure.
/// Note that this iterates over the target C++ versions, so produces repeated
/// diagnostics if they occur in multiple versions.
std::optional<std::vector<PointerNullabilityDiagnostic>> checkAndGetDiagnostics(
llvm::StringRef SourceCode);

} // namespace nullability
} // namespace tidy
} // namespace clang
Expand Down
76 changes: 71 additions & 5 deletions nullability/test/check_diagnostics_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,28 +6,71 @@

#include "nullability/test/check_diagnostics.h"

#include <optional>
#include <vector>

#include "nullability/pointer_nullability_diagnosis.h"
#include "external/llvm-project/third-party/unittest/googlemock/include/gmock/gmock.h"
#include "external/llvm-project/third-party/unittest/googletest/include/gtest/gtest-spi.h"
#include "external/llvm-project/third-party/unittest/googletest/include/gtest/gtest.h"

namespace clang::tidy::nullability {
namespace {

TEST(PointerNullabilityTest, NoDiagnostics) {
using ::testing::AllOf;
using ::testing::ElementsAre;
using ::testing::Field;
using ::testing::IsEmpty;
using ::testing::Optional;

TEST(PointerNullabilityTest, CheckNoDiagnostics) {
EXPECT_TRUE(checkDiagnostics(R"cc(
void target() {}
)cc"));
}

TEST(PointerNullabilityTest, ExpectedDiagnostic) {
TEST(PointerNullabilityTest, GetNoDiagnostics) {
EXPECT_THAT(checkAndGetDiagnostics(R"cc(
void target() {}
)cc"),
Optional(IsEmpty()));
}

TEST(PointerNullabilityTest, CheckExpectedDiagnostic) {
EXPECT_TRUE(checkDiagnostics(R"cc(
void target() {
int *p = nullptr;
int* p = nullptr;
*p; // [[unsafe]]
}
)cc"));
}

TEST(PointerNullabilityTest, UnexpectedDiagnostic) {
TEST(PointerNullabilityTest, GetExpectedDiagnostic) {
EXPECT_THAT(
checkAndGetDiagnostics(R"cc(
void target() {
int* p = nullptr;
*p; // [[unsafe]]
}
)cc"),
Optional(ElementsAre(
AllOf(
Field("Code", &PointerNullabilityDiagnostic::Code,
PointerNullabilityDiagnostic::ErrorCode::ExpectedNonnull),
Field("Ctx", &PointerNullabilityDiagnostic::Ctx,
PointerNullabilityDiagnostic::Context::NullableDereference),
Field("NoteMessage", &PointerNullabilityDiagnostic::NoteMessage,
"")),
AllOf(
Field("Code", &PointerNullabilityDiagnostic::Code,
PointerNullabilityDiagnostic::ErrorCode::ExpectedNonnull),
Field("Ctx", &PointerNullabilityDiagnostic::Ctx,
PointerNullabilityDiagnostic::Context::NullableDereference),
Field("NoteMessage", &PointerNullabilityDiagnostic::NoteMessage,
"")))));
}

TEST(PointerNullabilityTest, CheckUnexpectedDiagnostic) {
bool Result = true;
EXPECT_NONFATAL_FAILURE(Result = checkDiagnostics(R"cc(
void target() {
Expand All @@ -38,7 +81,18 @@ TEST(PointerNullabilityTest, UnexpectedDiagnostic) {
EXPECT_EQ(Result, false);
}

TEST(PointerNullabilityTest, MissingDiagnostic) {
TEST(PointerNullabilityTest, GetUnexpectedDiagnostic) {
std::optional<std::vector<PointerNullabilityDiagnostic>> Result;
EXPECT_NONFATAL_FAILURE(Result = checkAndGetDiagnostics(R"cc(
void target() {
1; // [[unsafe]]
}
)cc"),
"Expected diagnostics but didn't find them");
EXPECT_EQ(Result, std::nullopt);
}

TEST(PointerNullabilityTest, CheckMissingDiagnostic) {
bool Result = true;
EXPECT_NONFATAL_FAILURE(Result = checkDiagnostics(R"cc(
void target() {
Expand All @@ -50,5 +104,17 @@ TEST(PointerNullabilityTest, MissingDiagnostic) {
EXPECT_EQ(Result, false);
}

TEST(PointerNullabilityTest, GetMissingDiagnostic) {
std::optional<std::vector<PointerNullabilityDiagnostic>> Result;
EXPECT_NONFATAL_FAILURE(Result = checkAndGetDiagnostics(R"cc(
void target() {
int* p = nullptr;
*p; // Missing diagnostic
}
)cc"),
"Found diagnostics but didn't expect them");
EXPECT_EQ(Result, std::nullopt);
}

} // namespace
} // namespace clang::tidy::nullability