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