[clang-tools-extra] f5b6e9b - [include-cleaner] Fix crash on unresolved headers
Kadir Cetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Sun Mar 26 07:38:22 PDT 2023
Author: Kadir Cetinkaya
Date: 2023-03-26T16:37:56+02:00
New Revision: f5b6e9b6d355866c2aa4e440438b870885ec0373
URL: https://github.com/llvm/llvm-project/commit/f5b6e9b6d355866c2aa4e440438b870885ec0373
DIFF: https://github.com/llvm/llvm-project/commit/f5b6e9b6d355866c2aa4e440438b870885ec0373.diff
LOG: [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
Added:
Modified:
clang-tools-extra/include-cleaner/lib/Analysis.cpp
clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/include-cleaner/lib/Analysis.cpp b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
index fb0879b7aab63..58402cce0b717 100644
--- 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 a2084d4f37903..fc7d14582f632 100644
--- 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) {
More information about the cfe-commits
mailing list