kadircet created this revision. kadircet added a reviewer: sammccall. Herald added a project: All. kadircet requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits.
Despite being true positives, these results just confuse users. So filter them out. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D157905 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 @@ -483,5 +483,29 @@ Pair(Code.point("partial"), UnorderedElementsAre(Partial))))); } +TEST_F(WalkUsedTest, IgnoresIdentityMacros) { + llvm::Annotations Code(R"cpp( + #include "header.h" + void $bar^bar() { + $stdin^stdin(); + } + )cpp"); + Inputs.Code = Code.code(); + Inputs.ExtraFiles["header.h"] = guard(R"cpp( + #include "inner.h" + void stdin(); + )cpp"); + Inputs.ExtraFiles["inner.h"] = guard(R"cpp( + #define stdin stdin + )cpp"); + + TestAST AST(Inputs); + auto &SM = AST.sourceManager(); + auto MainFile = Header(SM.getFileEntryForID(SM.getMainFileID())); + EXPECT_THAT(offsetToProviders(AST, SM), + UnorderedElementsAre( + // FIXME: we should have a reference from stdin to header.h + Pair(Code.point("bar"), UnorderedElementsAre(MainFile)))); +} } // namespace } // namespace clang::include_cleaner 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 @@ -28,10 +28,29 @@ #include "llvm/ADT/StringSet.h" #include "llvm/Support/Error.h" #include "llvm/Support/ErrorHandling.h" +#include <cassert> #include <string> namespace clang::include_cleaner { +namespace { +bool shouldSkipMacro(const Macro &M) { + static const auto *MacroNamesToIgnore = new llvm::StringSet<>{ + // C standard says these are implementation defined macros, hence most of + // the standard library providers implement it by defining these as macros + // that resolve to themselves. + // This results in surprising behavior from users point of view (we + // generate a usage of stdio.h, in places unrelated to standard library). + // FIXME: Also eliminate the false positives by treating declarations + // resulting from these expansions as used. + "stdin", + "stdout", + "stderr", + }; + return MacroNamesToIgnore->contains(M.Name->getName()); +} +} // namespace + void walkUsed(llvm::ArrayRef<Decl *> ASTRoots, llvm::ArrayRef<SymbolReference> MacroRefs, const PragmaIncludes *PI, const SourceManager &SM, @@ -51,7 +70,8 @@ } for (const SymbolReference &MacroRef : MacroRefs) { assert(MacroRef.Target.kind() == Symbol::Macro); - if (!SM.isWrittenInMainFile(SM.getSpellingLoc(MacroRef.RefLocation))) + if (!SM.isWrittenInMainFile(SM.getSpellingLoc(MacroRef.RefLocation)) || + shouldSkipMacro(MacroRef.Target.macro())) continue; CB(MacroRef, headersForSymbol(MacroRef.Target, SM, PI)); }
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 @@ -483,5 +483,29 @@ Pair(Code.point("partial"), UnorderedElementsAre(Partial))))); } +TEST_F(WalkUsedTest, IgnoresIdentityMacros) { + llvm::Annotations Code(R"cpp( + #include "header.h" + void $bar^bar() { + $stdin^stdin(); + } + )cpp"); + Inputs.Code = Code.code(); + Inputs.ExtraFiles["header.h"] = guard(R"cpp( + #include "inner.h" + void stdin(); + )cpp"); + Inputs.ExtraFiles["inner.h"] = guard(R"cpp( + #define stdin stdin + )cpp"); + + TestAST AST(Inputs); + auto &SM = AST.sourceManager(); + auto MainFile = Header(SM.getFileEntryForID(SM.getMainFileID())); + EXPECT_THAT(offsetToProviders(AST, SM), + UnorderedElementsAre( + // FIXME: we should have a reference from stdin to header.h + Pair(Code.point("bar"), UnorderedElementsAre(MainFile)))); +} } // namespace } // namespace clang::include_cleaner 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 @@ -28,10 +28,29 @@ #include "llvm/ADT/StringSet.h" #include "llvm/Support/Error.h" #include "llvm/Support/ErrorHandling.h" +#include <cassert> #include <string> namespace clang::include_cleaner { +namespace { +bool shouldSkipMacro(const Macro &M) { + static const auto *MacroNamesToIgnore = new llvm::StringSet<>{ + // C standard says these are implementation defined macros, hence most of + // the standard library providers implement it by defining these as macros + // that resolve to themselves. + // This results in surprising behavior from users point of view (we + // generate a usage of stdio.h, in places unrelated to standard library). + // FIXME: Also eliminate the false positives by treating declarations + // resulting from these expansions as used. + "stdin", + "stdout", + "stderr", + }; + return MacroNamesToIgnore->contains(M.Name->getName()); +} +} // namespace + void walkUsed(llvm::ArrayRef<Decl *> ASTRoots, llvm::ArrayRef<SymbolReference> MacroRefs, const PragmaIncludes *PI, const SourceManager &SM, @@ -51,7 +70,8 @@ } for (const SymbolReference &MacroRef : MacroRefs) { assert(MacroRef.Target.kind() == Symbol::Macro); - if (!SM.isWrittenInMainFile(SM.getSpellingLoc(MacroRef.RefLocation))) + if (!SM.isWrittenInMainFile(SM.getSpellingLoc(MacroRef.RefLocation)) || + shouldSkipMacro(MacroRef.Target.macro())) continue; CB(MacroRef, headersForSymbol(MacroRef.Target, SM, PI)); }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits