diff --git a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h --- a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h +++ b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h @@ -17,16 +17,15 @@ #ifndef CLANG_INCLUDE_CLEANER_RECORD_H #define CLANG_INCLUDE_CLEANER_RECORD_H +#include "clang-include-cleaner/Types.h" +#include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/DenseSet.h" #include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Allocator.h" #include "llvm/Support/FileSystem/UniqueID.h" -#include "clang-include-cleaner/Types.h" -#include "llvm/ADT/ArrayRef.h" -#include "llvm/ADT/DenseMap.h" -#include "llvm/ADT/StringMap.h" #include #include @@ -73,6 +72,9 @@ /// Returns true if the given file is a self-contained file. bool isSelfContained(const FileEntry *File) const; + /// Returns true if the given file is marked with the IWYU private pragma. + bool isPrivate(const FileEntry *File) const; + private: class RecordPragma; /// 1-based Line numbers for the #include directives of the main file that @@ -80,7 +82,8 @@ /// export` right after). llvm::DenseSet ShouldKeep; - /// The public header mapping by the IWYU private pragma. + /// The public header mapping by the IWYU private pragma. For private pragmas + // without public mapping an empty StringRef is stored. // // !!NOTE: instead of using a FileEntry* to identify the physical file, we // deliberately use the UniqueID to ensure the result is stable across diff --git a/clang-tools-extra/include-cleaner/lib/Record.cpp b/clang-tools-extra/include-cleaner/lib/Record.cpp --- a/clang-tools-extra/include-cleaner/lib/Record.cpp +++ b/clang-tools-extra/include-cleaner/lib/Record.cpp @@ -53,7 +53,7 @@ SourceRange Range, const MacroArgs *Args) override { if (!Active) return; - recordMacroRef(MacroName, *MD.getMacroInfo(), RefType::Explicit); + recordMacroRef(MacroName, *MD.getMacroInfo()); } void MacroDefined(const Token &MacroName, const MacroDirective *MD) override { @@ -71,7 +71,7 @@ llvm::is_contained(MI->params(), II)) continue; if (const MacroInfo *MI = PP.getMacroInfo(II)) - recordMacroRef(Tok, *MI, RefType::Explicit); + recordMacroRef(Tok, *MI); } } @@ -80,11 +80,11 @@ if (!Active) return; if (const auto *MI = MD.getMacroInfo()) - recordMacroRef(MacroName, *MI, RefType::Explicit); + recordMacroRef(MacroName, *MI); } void Ifdef(SourceLocation Loc, const Token &MacroNameTok, - const MacroDefinition &MD) override { + const MacroDefinition &MD) override { if (!Active) return; if (const auto *MI = MD.getMacroInfo()) @@ -124,13 +124,13 @@ } private: - void recordMacroRef(const Token &Tok, const MacroInfo &MI, RefType RT) { + void recordMacroRef(const Token &Tok, const MacroInfo &MI, + RefType RT = RefType::Explicit) { if (MI.isBuiltinMacro()) return; // __FILE__ is not a reference. - Recorded.MacroReferences.push_back( - SymbolReference{Tok.getLocation(), - Macro{Tok.getIdentifierInfo(), MI.getDefinitionLoc()}, - RT}); + Recorded.MacroReferences.push_back(SymbolReference{ + Tok.getLocation(), + Macro{Tok.getIdentifierInfo(), MI.getDefinitionLoc()}, RT}); } bool Active = false; @@ -233,14 +233,18 @@ if (!Pragma) return false; - if (Pragma->consume_front("private, include ")) { - // We always insert using the spelling from the pragma. - if (auto *FE = SM.getFileEntryForID(SM.getFileID(Range.getBegin()))) - Out->IWYUPublic.insert( - {FE->getLastRef().getUniqueID(), - save(Pragma->startswith("<") || Pragma->startswith("\"") - ? (*Pragma) - : ("\"" + *Pragma + "\"").str())}); + if (Pragma->consume_front("private")) { + auto *FE = SM.getFileEntryForID(SM.getFileID(Range.getBegin())); + if (!FE) + return false; + StringRef PublicHeader; + if (Pragma->consume_front(", include ")) { + // We always insert using the spelling from the pragma. + PublicHeader = save(Pragma->startswith("<") || Pragma->startswith("\"") + ? (*Pragma) + : ("\"" + *Pragma + "\"").str()); + } + Out->IWYUPublic.insert({FE->getLastRef().getUniqueID(), PublicHeader}); return false; } FileID CommentFID = SM.getFileID(Range.getBegin()); @@ -346,6 +350,10 @@ return !NonSelfContainedFiles.contains(FE->getUniqueID()); } +bool PragmaIncludes::isPrivate(const FileEntry *FE) const { + return IWYUPublic.find(FE->getUniqueID()) != IWYUPublic.end(); +} + std::unique_ptr RecordedAST::record() { class Recorder : public ASTConsumer { RecordedAST *Out; diff --git a/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp b/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp --- a/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp +++ b/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp @@ -8,7 +8,6 @@ #include "clang-include-cleaner/Record.h" #include "clang/Basic/SourceLocation.h" -#include "clang/Basic/SourceLocation.h" #include "clang/Frontend/FrontendAction.h" #include "clang/Frontend/FrontendActions.h" #include "clang/Testing/TestAST.h" @@ -257,27 +256,6 @@ EXPECT_THAT(RefOffsets, ElementsAreArray(MainFile.points())); } -TEST_F(RecordPPTest, CapturesIfDefMacroRefs) { - llvm::Annotations MainFile(R"cpp( - #define X 1 - #ifdef $def^X - #define Y 2 - #endif - )cpp"); - - Inputs.Code = MainFile.code(); - auto AST = build(); - ASSERT_THAT(Recorded.MacroReferences, Not(IsEmpty())); - - SourceManager &SM = AST.sourceManager(); - SourceLocation Def = SM.getComposedLoc(SM.getMainFileID(), MainFile.point("def")); - - SymbolReference XRef = Recorded.MacroReferences.front(); - EXPECT_EQ(XRef.RT, RefType::Ambiguous); - EXPECT_EQ("X", XRef.Target.macro().Name->getName()); - EXPECT_EQ(XRef.RefLocation, Def); -} - // Matches an Include* on the specified line; MATCHER_P(line, N, "") { return arg->Line == (unsigned)N; } @@ -368,18 +346,30 @@ Inputs.Code = R"cpp( #include "public.h" )cpp"; - Inputs.ExtraFiles["public.h"] = "#include \"private.h\""; + Inputs.ExtraFiles["public.h"] = R"cpp( + #include "private.h" + #include "private2.h" + )cpp"; Inputs.ExtraFiles["private.h"] = R"cpp( // IWYU pragma: private, include "public2.h" - class Private {}; + )cpp"; + Inputs.ExtraFiles["private2.h"] = R"cpp( + // IWYU pragma: private )cpp"; TestAST Processed = build(); auto PrivateFE = Processed.fileManager().getFile("private.h"); assert(PrivateFE); + EXPECT_TRUE(PI.isPrivate(PrivateFE.get())); EXPECT_EQ(PI.getPublic(PrivateFE.get()), "\"public2.h\""); + auto PublicFE = Processed.fileManager().getFile("public.h"); assert(PublicFE); EXPECT_EQ(PI.getPublic(PublicFE.get()), ""); // no mapping. + EXPECT_FALSE(PI.isPrivate(PublicFE.get())); + + auto Private2FE = Processed.fileManager().getFile("private2.h"); + assert(Private2FE); + EXPECT_TRUE(PI.isPrivate(Private2FE.get())); } TEST_F(PragmaIncludeTest, IWYUExport) {