llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: Daniil Dudkin (unterumarmung) <details> <summary>Changes</summary> --- Full diff: https://github.com/llvm/llvm-project/pull/189366.diff 3 Files Affected: - (modified) clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h (+3-2) - (modified) clang-tools-extra/include-cleaner/lib/Record.cpp (+18-8) - (modified) clang-tools-extra/include-cleaner/unittests/RecordTest.cpp (+69-2) ``````````diff 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 index 2dcb5ea2555c5..98c2b63489f41 100644 --- a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h +++ b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h @@ -132,7 +132,7 @@ struct RecordedAST { std::vector<Decl *> Roots; }; -/// Recorded main-file preprocessor events relevant to include-cleaner. +/// Recorded preprocessor events relevant to include-cleaner. /// /// This doesn't include facts that we record globally for the whole TU, even /// when they occur in the main file (e.g. IWYU pragmas). @@ -140,7 +140,8 @@ struct RecordedPP { /// The callback (when installed into clang) tracks macros/includes in this. std::unique_ptr<PPCallbacks> record(const Preprocessor &PP); - /// Describes where macros were used in the main file. + /// Describes where macros were used in the main file or in files directly + /// included by the main file. std::vector<SymbolReference> MacroReferences; /// The include directives seen in the main file. diff --git a/clang-tools-extra/include-cleaner/lib/Record.cpp b/clang-tools-extra/include-cleaner/lib/Record.cpp index 0284d6842e2d2..a19fcf525919d 100644 --- a/clang-tools-extra/include-cleaner/lib/Record.cpp +++ b/clang-tools-extra/include-cleaner/lib/Record.cpp @@ -84,13 +84,13 @@ class PPRecorder : public PPCallbacks { void MacroExpands(const Token &MacroName, const MacroDefinition &MD, SourceRange Range, const MacroArgs *Args) override { - if (!Active) + if (!shouldRecordMacroRef(MacroName.getLocation())) return; recordMacroRef(MacroName, *MD.getMacroInfo()); } void MacroDefined(const Token &MacroName, const MacroDirective *MD) override { - if (!Active) + if (!shouldRecordMacroRef(MacroName.getLocation())) return; const auto *MI = MD->getMacroInfo(); @@ -110,7 +110,7 @@ class PPRecorder : public PPCallbacks { void MacroUndefined(const Token &MacroName, const MacroDefinition &MD, const MacroDirective *) override { - if (!Active) + if (!shouldRecordMacroRef(MacroName.getLocation())) return; if (const auto *MI = MD.getMacroInfo()) recordMacroRef(MacroName, *MI); @@ -118,7 +118,7 @@ class PPRecorder : public PPCallbacks { void Ifdef(SourceLocation Loc, const Token &MacroNameTok, const MacroDefinition &MD) override { - if (!Active) + if (!shouldRecordMacroRef(MacroNameTok.getLocation())) return; if (const auto *MI = MD.getMacroInfo()) recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous); @@ -126,7 +126,7 @@ class PPRecorder : public PPCallbacks { void Ifndef(SourceLocation Loc, const Token &MacroNameTok, const MacroDefinition &MD) override { - if (!Active) + if (!shouldRecordMacroRef(MacroNameTok.getLocation())) return; if (const auto *MI = MD.getMacroInfo()) recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous); @@ -136,14 +136,14 @@ class PPRecorder : public PPCallbacks { using PPCallbacks::Elifndef; void Elifdef(SourceLocation Loc, const Token &MacroNameTok, const MacroDefinition &MD) override { - if (!Active) + if (!shouldRecordMacroRef(MacroNameTok.getLocation())) return; if (const auto *MI = MD.getMacroInfo()) recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous); } void Elifndef(SourceLocation Loc, const Token &MacroNameTok, const MacroDefinition &MD) override { - if (!Active) + if (!shouldRecordMacroRef(MacroNameTok.getLocation())) return; if (const auto *MI = MD.getMacroInfo()) recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous); @@ -151,13 +151,23 @@ class PPRecorder : public PPCallbacks { void Defined(const Token &MacroNameTok, const MacroDefinition &MD, SourceRange Range) override { - if (!Active) + if (!shouldRecordMacroRef(MacroNameTok.getLocation())) return; if (const auto *MI = MD.getMacroInfo()) recordMacroRef(MacroNameTok, *MI, RefType::Ambiguous); } private: + bool shouldRecordMacroRef(SourceLocation Loc) const { + const SourceLocation ExpandedLoc = SM.getExpansionLoc(Loc); + const FileID FID = SM.getFileID(ExpandedLoc); + if (FID == SM.getMainFileID()) + return true; + const SourceLocation IncludeLoc = SM.getIncludeLoc(FID); + return IncludeLoc.isValid() && + SM.getFileID(IncludeLoc) == SM.getMainFileID(); + } + void recordMacroRef(const Token &Tok, const MacroInfo &MI, RefType RT = RefType::Explicit) { if (MI.isBuiltinMacro()) diff --git a/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp b/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp index cbf7bae23b365..c4b785f763750 100644 --- a/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp +++ b/clang-tools-extra/include-cleaner/unittests/RecordTest.cpp @@ -209,8 +209,8 @@ TEST_F(RecordPPTest, CapturesMacroRefs) { #define $def^X 1 // Refs, but not in main file. - #define Y X - int one = X; + #define Y $hdr^X + int one = $hdr^X; )cpp"); llvm::Annotations MainFile(R"cpp( #define EARLY X // not a ref, no definition @@ -244,11 +244,15 @@ TEST_F(RecordPPTest, CapturesMacroRefs) { std::vector<unsigned> RefOffsets; std::vector<unsigned> ExpOffsets; // Expansion locs of refs in macro locs. + std::vector<unsigned> HeaderOffsets; for (const auto &Ref : Recorded.MacroReferences) { if (Ref.Target == OrigX) { auto [FID, Off] = SM.getDecomposedLoc(Ref.RefLocation); if (FID == SM.getMainFileID()) { RefOffsets.push_back(Off); + } else if (FID == SM.translateFile(*AST.fileManager().getOptionalFileRef( + "header.h"))) { + HeaderOffsets.push_back(Off); } else if (Ref.RefLocation.isMacroID() && SM.isWrittenInMainFile(SM.getExpansionLoc(Ref.RefLocation))) { ExpOffsets.push_back( @@ -260,6 +264,7 @@ TEST_F(RecordPPTest, CapturesMacroRefs) { } EXPECT_THAT(RefOffsets, ElementsAreArray(MainFile.points())); EXPECT_THAT(ExpOffsets, ElementsAreArray(MainFile.points("exp"))); + EXPECT_THAT(HeaderOffsets, ElementsAreArray(Header.points("hdr"))); } TEST_F(RecordPPTest, CapturesConditionalMacroRefs) { @@ -300,6 +305,68 @@ TEST_F(RecordPPTest, CapturesConditionalMacroRefs) { EXPECT_THAT(RefOffsets, ElementsAreArray(MainFile.points())); } +TEST_F(RecordPPTest, CapturesConditionalMacroRefsInDirectIncludes) { + llvm::Annotations Header(R"cpp( + #ifdef ^X + #endif + + #if defined(^X) + #endif + + #ifndef ^X + #endif + + #ifdef Y + #elifdef ^X + #endif + + #ifndef ^X + #elifndef ^X + #endif + )cpp"); + + Inputs.Code = R"cpp( + #define X 1 + #include "header.h" + )cpp"; + Inputs.ExtraFiles["header.h"] = Header.code(); + Inputs.ExtraArgs.push_back("-std=c++2b"); + auto AST = build(); + + std::vector<unsigned> RefOffsets; + SourceManager &SM = AST.sourceManager(); + FileID HeaderID = + SM.translateFile(*AST.fileManager().getOptionalFileRef("header.h")); + for (const auto &Ref : Recorded.MacroReferences) { + auto [FID, Off] = SM.getDecomposedLoc(Ref.RefLocation); + ASSERT_EQ(FID, HeaderID); + EXPECT_EQ(Ref.RT, RefType::Ambiguous); + EXPECT_EQ("X", Ref.Target.macro().Name->getName()); + RefOffsets.push_back(Off); + } + EXPECT_THAT(RefOffsets, ElementsAreArray(Header.points())); +} + +TEST_F(RecordPPTest, DoesNotCaptureMacroRefsInTransitiveIncludes) { + Inputs.Code = R"cpp( + #define X 1 + #include "header.h" + )cpp"; + Inputs.ExtraFiles["header.h"] = R"cpp( + #include "nested.h" + )cpp"; + Inputs.ExtraFiles["nested.h"] = R"cpp( + int one = X; + + #ifdef X + #endif + )cpp"; + Inputs.ExtraArgs.push_back("-std=c++2b"); + build(); + + EXPECT_THAT(Recorded.MacroReferences, IsEmpty()); +} + class PragmaIncludeTest : public ::testing::Test { protected: // We don't build an AST, we just run a preprocessor action! `````````` </details> https://github.com/llvm/llvm-project/pull/189366 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
