maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[clangd] Avoid expensive checks of buffer names in IncludeCleaner 

This changes the handling of special buffers (<command-line> etc) that
SourceManager treats as files but FileManager does not.

We now include them in findReferencedFiles() and drop them as part of
translateToHeaderIDs(). This pairs more naturally with the data representations
we're using, and so avoids a bunch of converting between representations for
filtering.

Differential Revision: https://reviews.llvm.org/D112652
This commit is contained in:
Sam McCall 2021-10-27 21:13:32 +02:00
parent 259e4c5658
commit 73453e7ade
3 changed files with 42 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
clang-tools-extra/clangd/IncludeCleaner.cpp
View File

@ -129,9 +129,7 @@ struct ReferencedFiles {
void add(SourceLocation Loc) { add(SM.getFileID(Loc), Loc); }
void add(FileID FID, SourceLocation Loc) {
// Check if Loc is not written in a physical file.
if (FID.isInvalid() || SM.isWrittenInBuiltinFile(Loc) ||
SM.isWrittenInCommandLineFile(Loc))
if (FID.isInvalid())
return;
assert(SM.isInFileID(Loc, FID));
if (Loc.isFileID()) {
@ -142,15 +140,9 @@ struct ReferencedFiles {
if (!Macros.insert(FID).second)
return;
const auto &Exp = SM.getSLocEntry(FID).getExpansion();
// For token pasting operator in macros, spelling and expansion locations
// can be within a temporary buffer that Clang creates (scratch space or
// ScratchBuffer). That is not a real file we can include.
if (!SM.isWrittenInScratchSpace(Exp.getSpellingLoc()))
add(Exp.getSpellingLoc());
if (!SM.isWrittenInScratchSpace(Exp.getExpansionLocStart()))
add(Exp.getExpansionLocStart());
if (!SM.isWrittenInScratchSpace(Exp.getExpansionLocEnd()))
add(Exp.getExpansionLocEnd());
add(Exp.getSpellingLoc());
add(Exp.getExpansionLocStart());
add(Exp.getExpansionLocEnd());
}
};
@ -249,6 +241,14 @@ getUnused(const IncludeStructure &Structure,
return Unused;
}
#ifndef NDEBUG
// Is FID a <built-in>, <scratch space> etc?
static bool isSpecialBuffer(FileID FID, const SourceManager &SM) {
const SrcMgr::FileInfo &FI = SM.getSLocEntry(FID).getFile();
return FI.getName().startswith("<");
}
#endif
llvm::DenseSet<IncludeStructure::HeaderID>
translateToHeaderIDs(const llvm::DenseSet<FileID> &Files,
const IncludeStructure &Includes,
@ -257,7 +257,10 @@ translateToHeaderIDs(const llvm::DenseSet<FileID> &Files,
TranslatedHeaderIDs.reserve(Files.size());
for (FileID FID : Files) {
const FileEntry *FE = SM.getFileEntryForID(FID);
assert(FE);
if (!FE) {
assert(isSpecialBuffer(FID, SM));
continue;
}
const auto File = Includes.getID(FE);
assert(File);
TranslatedHeaderIDs.insert(*File);

3
clang-tools-extra/clangd/IncludeCleaner.h
View File

@ -49,10 +49,13 @@ using ReferencedLocations = llvm::DenseSet<SourceLocation>;
ReferencedLocations findReferencedLocations(ParsedAST &AST);
/// Retrieves IDs of all files containing SourceLocations from \p Locs.
/// The output only includes things SourceManager sees as files (not macro IDs).
/// This can include <built-in>, <scratch space> etc that are not true files.
llvm::DenseSet<FileID> findReferencedFiles(const ReferencedLocations &Locs,
const SourceManager &SM);
/// Maps FileIDs to the internal IncludeStructure representation (HeaderIDs).
/// FileIDs that are not true files (<built-in> etc) are dropped.
llvm::DenseSet<IncludeStructure::HeaderID>
translateToHeaderIDs(const llvm::DenseSet<FileID> &Files,
const IncludeStructure &Includes, const SourceManager &SM);

37
clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
View File

@ -16,6 +16,7 @@ namespace clang {
namespace clangd {
namespace {
using ::testing::ElementsAre;
using ::testing::UnorderedElementsAre;
TEST(IncludeCleaner, ReferencedLocations) {
@ -236,9 +237,8 @@ TEST(IncludeCleaner, GetUnusedHeaders) {
TEST(IncludeCleaner, VirtualBuffers) {
TestTU TU;
TU.Filename = "foo.cpp";
TU.Code = R"cpp(
#include "macro_spelling_in_scratch_buffer.h"
#include "macros.h"
using flags::FLAGS_FOO;
@ -251,7 +251,7 @@ TEST(IncludeCleaner, VirtualBuffers) {
// The pasting operator in combination with DEFINE_FLAG will create
// ScratchBuffer with `flags::FLAGS_FOO` that will have FileID but not
// FileEntry.
TU.AdditionalFiles["macro_spelling_in_scratch_buffer.h"] = R"cpp(
TU.AdditionalFiles["macros.h"] = R"cpp(
#define DEFINE_FLAG(X) \
namespace flags { \
int FLAGS_##X; \
@ -266,18 +266,27 @@ TEST(IncludeCleaner, VirtualBuffers) {
ParsedAST AST = TU.build();
auto &SM = AST.getSourceManager();
auto &Includes = AST.getIncludeStructure();
auto ReferencedFiles = findReferencedFiles(findReferencedLocations(AST), SM);
auto Entry = SM.getFileManager().getFile(
testPath("macro_spelling_in_scratch_buffer.h"));
ASSERT_TRUE(Entry);
auto FID = SM.translateFile(*Entry);
// No "<scratch space>" FID.
EXPECT_THAT(ReferencedFiles, UnorderedElementsAre(FID));
// Should not crash due to <scratch space> "files" missing from include
// structure.
EXPECT_THAT(
getUnused(Includes, translateToHeaderIDs(ReferencedFiles, Includes, SM)),
::testing::IsEmpty());
llvm::StringSet<> ReferencedFileNames;
for (FileID FID : ReferencedFiles)
ReferencedFileNames.insert(
SM.getPresumedLoc(SM.getLocForStartOfFile(FID)).getFilename());
// Note we deduped the names as _number_ of <built-in>s is uninteresting.
EXPECT_THAT(ReferencedFileNames.keys(),
UnorderedElementsAre("<built-in>", "<scratch space>",
testPath("macros.h")));
// Should not crash due to FileIDs that are not headers.
auto ReferencedHeaders = translateToHeaderIDs(ReferencedFiles, Includes, SM);
std::vector<llvm::StringRef> ReferencedHeaderNames;
for (IncludeStructure::HeaderID HID : ReferencedHeaders)
ReferencedHeaderNames.push_back(Includes.getRealPath(HID));
// Non-header files are gone at this point.
EXPECT_THAT(ReferencedHeaderNames, ElementsAre(testPath("macros.h")));
// Sanity check.
EXPECT_THAT(getUnused(Includes, ReferencedHeaders), ::testing::IsEmpty());
}
} // namespace