VitaNuo updated this revision to Diff 478145.
VitaNuo marked an inline comment as done.
VitaNuo added a comment.
Address the remaining comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138678/new/
https://reviews.llvm.org/D138678
Files:
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
clang-tools-extra/include-cleaner/lib/Record.cpp
clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
Index: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
===================================================================
--- clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
+++ 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) {
Index: clang-tools-extra/include-cleaner/lib/Record.cpp
===================================================================
--- clang-tools-extra/include-cleaner/lib/Record.cpp
+++ 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<ASTConsumer> RecordedAST::record() {
class Recorder : public ASTConsumer {
RecordedAST *Out;
Index: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
===================================================================
--- clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
+++ 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 <memory>
#include <vector>
@@ -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</*LineNumber*/ unsigned> 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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
