[clang][nullability] Don't return null fields from getReferencedDecls(). (#94983)

The patch includes a repro for a case where we were returning a null
`FieldDecl`
when calling `getReferencedDecls()` on the `InitListExpr` for a union.

Also, I noticed while working on this that `RecordInitListHelper` has a
bug
where it doesn't work correctly for empty unions. This patch also
includes a
repro and fix for this bug.

Author: martinboehme

clang/docs/tools/clang-formatted-files.txt

@@ -622,6 +622,7 @@ clang/tools/libclang/CXCursor.h
 clang/tools/scan-build-py/tests/functional/src/include/clean-one.h
 clang/unittests/Analysis/CFGBuildResult.h
 clang/unittests/Analysis/MacroExpansionContextTest.cpp
+clang/unittests/Analysis/FlowSensitive/ASTOpsTest.cpp
 clang/unittests/Analysis/FlowSensitive/CNFFormula.cpp
 clang/unittests/Analysis/FlowSensitive/DataflowAnalysisContextTest.cpp
 clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp

clang/lib/Analysis/FlowSensitive/ASTOps.cpp

@@ -100,7 +100,8 @@ getFieldsForInitListExpr(const InitListT *InitList) {
   std::vector<const FieldDecl *> Fields;
 
   if (InitList->getType()->isUnionType()) {
-    Fields.push_back(InitList->getInitializedFieldInUnion());
+    if (const FieldDecl *Field = InitList->getInitializedFieldInUnion())
+      Fields.push_back(Field);
     return Fields;
   }
 
@@ -137,9 +138,11 @@ RecordInitListHelper::RecordInitListHelper(
   // it doesn't do this -- so we create an `ImplicitValueInitExpr` ourselves.
   SmallVector<Expr *> InitsForUnion;
   if (Ty->isUnionType() && Inits.empty()) {
-    assert(Fields.size() == 1);
-    ImplicitValueInitForUnion.emplace(Fields.front()->getType());
-    InitsForUnion.push_back(&*ImplicitValueInitForUnion);
+    assert(Fields.size() <= 1);
+    if (!Fields.empty()) {
+      ImplicitValueInitForUnion.emplace(Fields.front()->getType());
+      InitsForUnion.push_back(&*ImplicitValueInitForUnion);
+    }
     Inits = InitsForUnion;
   }
 

clang/unittests/Analysis/FlowSensitive/ASTOpsTest.cpp

@@ -0,0 +1,88 @@
+//===- unittests/Analysis/FlowSensitive/ASTOpsTest.cpp --------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Analysis/FlowSensitive/ASTOps.h"
+#include "TestingSupport.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include <memory>
+
+namespace {
+
+using namespace clang;
+using namespace dataflow;
+
+using ast_matchers::cxxRecordDecl;
+using ast_matchers::hasName;
+using ast_matchers::hasType;
+using ast_matchers::initListExpr;
+using ast_matchers::match;
+using ast_matchers::selectFirst;
+using test::findValueDecl;
+using testing::IsEmpty;
+using testing::UnorderedElementsAre;
+
+TEST(ASTOpsTest, RecordInitListHelperOnEmptyUnionInitList) {
+  // This is a regression test: The `RecordInitListHelper` used to assert-fail
+  // when called for the `InitListExpr` of an empty union.
+  std::string Code = R"cc(
+    struct S {
+      S() : UField{} {};
+
+      union U {} UField;
+    };
+  )cc";
+  std::unique_ptr<ASTUnit> Unit =
+      tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++17"});
+  auto &ASTCtx = Unit->getASTContext();
+
+  ASSERT_EQ(ASTCtx.getDiagnostics().getClient()->getNumErrors(), 0U);
+
+  auto *InitList = selectFirst<InitListExpr>(
+      "init",
+      match(initListExpr(hasType(cxxRecordDecl(hasName("U")))).bind("init"),
+            ASTCtx));
+  ASSERT_NE(InitList, nullptr);
+
+  RecordInitListHelper Helper(InitList);
+  EXPECT_THAT(Helper.base_inits(), IsEmpty());
+  EXPECT_THAT(Helper.field_inits(), IsEmpty());
+}
+
+TEST(ASTOpsTest, ReferencedDeclsOnUnionInitList) {
+  // This is a regression test: `getReferencedDecls()` used to return a null
+  // `FieldDecl` in this case (in addition to the correct non-null `FieldDecl`)
+  // because `getInitializedFieldInUnion()` returns null for the syntactic form
+  // of the `InitListExpr`.
+  std::string Code = R"cc(
+    struct S {
+      S() : UField{0} {};
+
+      union U {
+        int I;
+      } UField;
+    };
+  )cc";
+  std::unique_ptr<ASTUnit> Unit =
+      tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++17"});
+  auto &ASTCtx = Unit->getASTContext();
+
+  ASSERT_EQ(ASTCtx.getDiagnostics().getClient()->getNumErrors(), 0U);
+
+  auto *InitList = selectFirst<InitListExpr>(
+      "init",
+      match(initListExpr(hasType(cxxRecordDecl(hasName("U")))).bind("init"),
+            ASTCtx));
+  ASSERT_NE(InitList, nullptr);
+  auto *IDecl = cast<FieldDecl>(findValueDecl(ASTCtx, "I"));
+
+  EXPECT_THAT(getReferencedDecls(*InitList).Fields,
+              UnorderedElementsAre(IDecl));
+}
+
+} // namespace

clang/unittests/Analysis/FlowSensitive/CMakeLists.txt

@@ -5,6 +5,7 @@ set(LLVM_LINK_COMPONENTS
 
 add_clang_unittest(ClangAnalysisFlowSensitiveTests
   ArenaTest.cpp
+  ASTOpsTest.cpp
   CFGMatchSwitchTest.cpp
   ChromiumCheckModelTest.cpp
   DataflowAnalysisContextTest.cpp

llvm/utils/gn/secondary/clang/unittests/Analysis/FlowSensitive/BUILD.gn

@@ -18,6 +18,7 @@ unittest("ClangAnalysisFlowSensitiveTests") {
     "//llvm/lib/Testing/Support",
   ]
   sources = [
+    "ASTOpsTest.cpp",
     "ArenaTest.cpp",
     "CFGMatchSwitchTest.cpp",
     "ChromiumCheckModelTest.cpp",