[include-cleaner] Fix crash on unresolved headers

Make sure unresolved headers are not analyzed as part of unused
includes.

Also introduces a testing fixture for analyze tests

Differential Revision: https://reviews.llvm.org/D146916

diff --git a/clang-tools-extra/include-cleaner/lib/Analysis.cpp b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
index fb0879b7aab63ffc1c8903ef4f95917b5f46eecf..58402cce0b71740730c090829466d793da6fe4e2 100644 (file)
--- a/clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -10,7 +10,6 @@
 #include "AnalysisInternal.h"
 #include "clang-include-cleaner/Record.h"
 #include "clang-include-cleaner/Types.h"
-#include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/Basic/SourceManager.h"
@@ -19,9 +18,13 @@
 #include "clang/Tooling/Core/Replacement.h"
 #include "clang/Tooling/Inclusions/StandardLibrary.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSet.h"
+#include "llvm/Support/Error.h"
+#include <string>
 
 namespace clang::include_cleaner {
 
@@ -91,7 +94,7 @@ AnalysisResults analyze(llvm::ArrayRef<Decl *> ASTRoots,
 
   AnalysisResults Results;
   for (const Include &I : Inc.all()) {
-    if (Used.contains(&I))
+    if (Used.contains(&I) || !I.Resolved)
       continue;
     if (PI) {
       if (PI->shouldKeep(I.Line))

diff --git a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
index a2084d4f379030e804922347c010cfac211b0343..fc7d14582f63243eb5155df40410736115c21e98 100644 (file)
--- a/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -8,22 +8,28 @@
 
 #include "clang-include-cleaner/Analysis.h"
 #include "AnalysisInternal.h"
+#include "TypesInternal.h"
 #include "clang-include-cleaner/Record.h"
 #include "clang-include-cleaner/Types.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/Basic/FileManager.h"
+#include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Testing/TestAST.h"
 #include "clang/Tooling/Inclusions/StandardLibrary.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Testing/Annotations/Annotations.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include <cstddef>
+#include <memory>
+#include <string>
 #include <vector>
 
 namespace clang::include_cleaner {
@@ -178,8 +184,31 @@ TEST_F(WalkUsedTest, MacroRefs) {
           Pair(Code.point("2"), UnorderedElementsAre(HdrFile))));
 }
 
-TEST(Analyze, Basic) {
+class AnalyzeTest : public testing::Test {
+protected:
   TestInputs Inputs;
+  PragmaIncludes PI;
+  RecordedPP PP;
+  AnalyzeTest() {
+    Inputs.MakeAction = [this] {
+      struct Hook : public SyntaxOnlyAction {
+      public:
+        Hook(RecordedPP &PP, PragmaIncludes &PI) : PP(PP), PI(PI) {}
+        bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
+          CI.getPreprocessor().addPPCallbacks(PP.record(CI.getPreprocessor()));
+          PI.record(CI);
+          return true;
+        }
+
+        RecordedPP &PP;
+        PragmaIncludes &PI;
+      };
+      return std::make_unique<Hook>(PP, PI);
+    };
+  }
+};
+
+TEST_F(AnalyzeTest, Basic) {
   Inputs.Code = R"cpp(
 #include "a.h"
 #include "b.h"
@@ -194,53 +223,41 @@ int x = a + c;
   )cpp");
   Inputs.ExtraFiles["c.h"] = guard("int c;");
   Inputs.ExtraFiles["keep.h"] = guard("");
+  TestAST AST(Inputs);
+  auto Decls = AST.context().getTranslationUnitDecl()->decls();
+  auto Results =
+      analyze(std::vector<Decl *>{Decls.begin(), Decls.end()},
+              PP.MacroReferences, PP.Includes, &PI, AST.sourceManager(),
+              AST.preprocessor().getHeaderSearchInfo());
 
-  RecordedPP PP;
-  PragmaIncludes PI;
-  Inputs.MakeAction = [&PP, &PI] {
-    struct Hook : public SyntaxOnlyAction {
-    public:
-      Hook(RecordedPP &PP, PragmaIncludes &PI) : PP(PP), PI(PI) {}
-      bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
-        CI.getPreprocessor().addPPCallbacks(PP.record(CI.getPreprocessor()));
-        PI.record(CI);
-        return true;
-      }
-
-      RecordedPP &PP;
-      PragmaIncludes &PI;
-    };
-    return std::make_unique<Hook>(PP, PI);
-  };
-
-  {
-    TestAST AST(Inputs);
-    auto Decls = AST.context().getTranslationUnitDecl()->decls();
-    auto Results =
-        analyze(std::vector<Decl *>{Decls.begin(), Decls.end()},
-                PP.MacroReferences, PP.Includes, &PI, AST.sourceManager(),
-                AST.preprocessor().getHeaderSearchInfo());
+  const Include *B = PP.Includes.atLine(3);
+  ASSERT_EQ(B->Spelled, "b.h");
+  EXPECT_THAT(Results.Missing, ElementsAre("\"c.h\""));
+  EXPECT_THAT(Results.Unused, ElementsAre(B));
+}
 
-    const Include *B = PP.Includes.atLine(3);
-    ASSERT_EQ(B->Spelled, "b.h");
-    EXPECT_THAT(Results.Missing, ElementsAre("\"c.h\""));
-    EXPECT_THAT(Results.Unused, ElementsAre(B));
-  }
+TEST_F(AnalyzeTest, PrivateUsedInPublic) {
+  // Check that umbrella header uses private include.
+  Inputs.Code = R"cpp(#include "private.h")cpp";
+  Inputs.ExtraFiles["private.h"] =
+      guard("// IWYU pragma: private, include \"public.h\"");
+  Inputs.FileName = "public.h";
+  TestAST AST(Inputs);
+  EXPECT_FALSE(PP.Includes.all().empty());
+  auto Results = analyze({}, {}, PP.Includes, &PI, AST.sourceManager(),
+                         AST.preprocessor().getHeaderSearchInfo());
+  EXPECT_THAT(Results.Unused, testing::IsEmpty());
+}
 
+TEST_F(AnalyzeTest, NoCrashWhenUnresolved) {
   // Check that umbrella header uses private include.
-  {
-    Inputs.Code = R"cpp(#include "private.h")cpp";
-    Inputs.ExtraFiles["private.h"] =
-        guard("// IWYU pragma: private, include \"public.h\"");
-    Inputs.FileName = "public.h";
-    PP.Includes = {};
-    PI = {};
-    TestAST AST(Inputs);
-    EXPECT_FALSE(PP.Includes.all().empty());
-    auto Results = analyze({}, {}, PP.Includes, &PI, AST.sourceManager(),
-                           AST.preprocessor().getHeaderSearchInfo());
-    EXPECT_THAT(Results.Unused, testing::IsEmpty());
-  }
+  Inputs.Code = R"cpp(#include "not_found.h")cpp";
+  Inputs.ErrorOK = true;
+  TestAST AST(Inputs);
+  EXPECT_FALSE(PP.Includes.all().empty());
+  auto Results = analyze({}, {}, PP.Includes, &PI, AST.sourceManager(),
+                         AST.preprocessor().getHeaderSearchInfo());
+  EXPECT_THAT(Results.Unused, testing::IsEmpty());
 }
 
 TEST(FixIncludes, Basic) {