This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf5b6e9b6d355: [include-cleaner] Fix crash on unresolved
headers (authored by kadircet).
Changed prior to commit:
https://reviews.llvm.org/D146916?vs=508415&id=508417#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146916/new/
https://reviews.llvm.org/D146916
Files:
clang-tools-extra/include-cleaner/lib/Analysis.cpp
clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
Index: clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
===================================================================
--- clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ 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 @@
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 @@
)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) {
Index: clang-tools-extra/include-cleaner/lib/Analysis.cpp
===================================================================
--- clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ 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 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))
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
