[nullability] Analyze functions that have a foward declaration only o…

…nce.

My [previous
change](4b01ed0)
here fixed a bug but had the unwanted effect of causing us to analyze a function
having a forward declaration twice (once for each redeclaration).

PiperOrigin-RevId: 598578542
Change-Id: I60a9120b8bfb0d23cc0b5bdc4f67fbd30c1ffe6c

bazel/llvm.bzl

@@ -53,7 +53,7 @@ def _llvm_loader_repository(repository_ctx):
             executable = False,
         )
 
-LLVM_COMMIT_SHA = "959a430a8d5b7e77b3d88327f835d9f9b8a6842e"
+LLVM_COMMIT_SHA = "c230138011cbf07ad7caf9d256ae9d0c5032a974"
 
 def llvm_loader_repository_dependencies():
     # This *declares* the dependency, but it won't actually be *downloaded* unless it's used.

nullability/pointer_nullability_diagnosis.cc

@@ -441,10 +441,9 @@ diagnosePointerNullability(const FunctionDecl *Func) {
   for (const ParmVarDecl *Parm : Func->parameters())
     checkParmVarDeclWithPointerDefaultArg(Ctx, *Parm, Diags);
 
-  // If `Func` has multiple declarations in the current TU, make sure we use the
-  // one that defines the body of the function. (We retrieve this using the
-  // output parameter of `hasBody()`.)
-  if (!Func->hasBody(Func)) return Diags;
+  // Use `doesThisDeclarationHaveABody()` rather than `hasBody()` to ensure we
+  // analyze forward-declared functions only once.
+  if (!Func->doesThisDeclarationHaveABody()) return Diags;
 
   auto Diagnoser = pointerNullabilityDiagnoser();
   const std::int64_t MaxSATIterations = 2'000'000;

nullability/test/BUILD

@@ -95,6 +95,13 @@ cc_test(
     srcs = ["basic.cc"],
     deps = [
         ":check_diagnostics",
+        "//nullability:pointer_nullability_diagnosis",
+        "@llvm-project//clang:ast",
+        "@llvm-project//clang:basic",
+        "@llvm-project//clang:frontend",
+        "@llvm-project//clang:tooling",
+        "@llvm-project//llvm:TestingSupport",
+        "@llvm-project//third-party/unittest:gmock",
         "@llvm-project//third-party/unittest:gtest",
         "@llvm-project//third-party/unittest:gtest_main",
     ],

nullability/test/basic.cc

@@ -4,12 +4,26 @@
 
 // Tests for basic functionality (simple dereferences without control flow).
 
+#include <memory>
+
+#include "nullability/pointer_nullability_diagnosis.h"
 #include "nullability/test/check_diagnostics.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
+#include "clang/Basic/LLVM.h"
+#include "clang/Frontend/ASTUnit.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Testing/Support/Error.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::IsEmpty;
+using ::testing::SizeIs;
+
 TEST(PointerNullabilityTest, NoPointerOperations) {
   EXPECT_TRUE(checkDiagnostics(R"cc(
     void target() { 1 + 2; }
@@ -262,5 +276,33 @@ TEST(PointerNullabilityTest, ForwardDeclaration) {
   )cc"));
 }
 
+TEST(PointerNullabilityTest, AnalyzeFunctionWithForwardDeclarationOnlyOnce) {
+  std::unique_ptr<ASTUnit> Unit = tooling::buildASTFromCode(R"cc(
+    // Check that we analyze a function with a forward declaration only once
+    // (for the definition), and not for every redeclaration that we encounter.
+    void target();
+    void target() {
+      int *p = nullptr;
+      *p;
+    }
+  )cc");
+
+  ASTContext &Context = Unit->getASTContext();
+  DeclContextLookupResult Result =
+      Context.getTranslationUnitDecl()->lookup(&Context.Idents.get("target"));
+  ASSERT_TRUE(Result.isSingleResult());
+  auto *Target = cast<FunctionDecl>(Result.front());
+  SmallVector<FunctionDecl *> Redecls(Target->redecls());
+  ASSERT_EQ(Redecls.size(), 2);
+
+  EXPECT_TRUE(Redecls[0]->doesThisDeclarationHaveABody());
+  EXPECT_THAT_EXPECTED(diagnosePointerNullability(Redecls[0]),
+                       llvm::HasValue(SizeIs(1)));
+
+  EXPECT_FALSE(Redecls[1]->doesThisDeclarationHaveABody());
+  EXPECT_THAT_EXPECTED(diagnosePointerNullability(Redecls[1]),
+                       llvm::HasValue(IsEmpty()));
+}
+
 }  // namespace
 }  // namespace clang::tidy::nullability