From: Kadir Cetinkaya Date: Thu, 23 Apr 2020 15:48:47 +0000 (+0200) Subject: [clangd] Handle PresumedLocations in IncludeCollector X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=4f7917c269d65cd3c85eddee19385861c4b8390c;p=platform%2Fupstream%2Fllvm.git [clangd] Handle PresumedLocations in IncludeCollector Summary: This will enable extraction of correct line locations in preamble patch for includes. Reviewers: sammccall Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D78740 --- diff --git a/clang-tools-extra/clangd/Headers.cpp b/clang-tools-extra/clangd/Headers.cpp index 50c3759..ffd6bd1 100644 --- a/clang-tools-extra/clangd/Headers.cpp +++ b/clang-tools-extra/clangd/Headers.cpp @@ -10,6 +10,8 @@ #include "Compiler.h" #include "SourceCode.h" #include "support/Logger.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/CompilerInvocation.h" #include "clang/Frontend/FrontendActions.h" @@ -21,6 +23,11 @@ namespace clang { namespace clangd { namespace { +bool isMainFile(llvm::StringRef FileName, const SourceManager &SM) { + auto FE = SM.getFileManager().getFile(FileName); + return FE && *FE == SM.getFileEntryForID(SM.getMainFileID()); +} + class RecordHeaders : public PPCallbacks { public: RecordHeaders(const SourceManager &SM, IncludeStructure *Out) @@ -30,11 +37,27 @@ public: // in the main file are collected. void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok, llvm::StringRef FileName, bool IsAngled, - CharSourceRange FilenameRange, const FileEntry *File, - llvm::StringRef /*SearchPath*/, + CharSourceRange /*FilenameRange*/, + const FileEntry *File, llvm::StringRef /*SearchPath*/, llvm::StringRef /*RelativePath*/, const Module * /*Imported*/, SrcMgr::CharacteristicKind FileKind) override { + auto MainFID = SM.getMainFileID(); + // If an include is part of the preamble patch, translate #line directives. + if (InBuiltinFile) { + auto Presumed = SM.getPresumedLoc(HashLoc); + // Presumed locations will have an invalid file id when #line directive + // changes the filename. + if (Presumed.getFileID().isInvalid() && + isMainFile(Presumed.getFilename(), SM)) { + // Now we'll hit the case below. + HashLoc = SM.translateLineCol(MainFID, Presumed.getLine(), + Presumed.getColumn()); + } + } + + // Record main-file inclusions (including those mapped from the preamble + // patch). if (isInsideMainFile(HashLoc, SM)) { Out->MainFileIncludes.emplace_back(); auto &Inc = Out->MainFileIncludes.back(); @@ -47,21 +70,48 @@ public: Inc.FileKind = FileKind; Inc.Directive = IncludeTok.getIdentifierInfo()->getPPKeywordID(); } + + // Record include graph (not just for main-file includes) if (File) { auto *IncludingFileEntry = SM.getFileEntryForID(SM.getFileID(HashLoc)); if (!IncludingFileEntry) { assert(SM.getBufferName(HashLoc).startswith("<") && "Expected #include location to be a file or "); // Treat as if included from the main file. - IncludingFileEntry = SM.getFileEntryForID(SM.getMainFileID()); + IncludingFileEntry = SM.getFileEntryForID(MainFID); } Out->recordInclude(IncludingFileEntry->getName(), File->getName(), File->tryGetRealPathName()); } } + void FileChanged(SourceLocation Loc, FileChangeReason Reason, + SrcMgr::CharacteristicKind FileType, + FileID PrevFID) override { + switch (Reason) { + case PPCallbacks::EnterFile: + if (BuiltinFile.isInvalid() && SM.isWrittenInBuiltinFile(Loc)) { + BuiltinFile = SM.getFileID(Loc); + InBuiltinFile = true; + } + break; + case PPCallbacks::ExitFile: + if (PrevFID == BuiltinFile) + InBuiltinFile = false; + break; + case PPCallbacks::RenameFile: + case PPCallbacks::SystemHeaderPragma: + break; + } + } + private: const SourceManager &SM; + // Set after entering the file. + FileID BuiltinFile; + // Indicates whether file is part of include stack. + bool InBuiltinFile = false; + IncludeStructure *Out; }; diff --git a/clang-tools-extra/clangd/unittests/HeadersTests.cpp b/clang-tools-extra/clangd/unittests/HeadersTests.cpp index a6464c6..b40d883 100644 --- a/clang-tools-extra/clangd/unittests/HeadersTests.cpp +++ b/clang-tools-extra/clangd/unittests/HeadersTests.cpp @@ -16,6 +16,7 @@ #include "clang/Frontend/CompilerInvocation.h" #include "clang/Frontend/FrontendActions.h" #include "clang/Lex/PreprocessorOptions.h" +#include "llvm/Support/FormatVariadic.h" #include "llvm/Support/Path.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -25,7 +26,9 @@ namespace clangd { namespace { using ::testing::AllOf; +using ::testing::Contains; using ::testing::ElementsAre; +using ::testing::Not; using ::testing::UnorderedElementsAre; class HeadersTest : public ::testing::Test { @@ -302,6 +305,26 @@ TEST(Headers, NoHeaderSearchInfo) { llvm::None); } +TEST_F(HeadersTest, PresumedLocations) { + std::string HeaderFile = "implicit_include.h"; + + // Line map inclusion back to main file. + std::string HeaderContents = llvm::formatv("#line 0 \"{0}\"", MainFile); + HeaderContents += R"cpp( +#line 3 +#include )cpp"; + FS.Files[HeaderFile] = HeaderContents; + + // Including through non-builtin file has no effects. + FS.Files[MainFile] = "#include \"implicit_include.h\"\n\n"; + EXPECT_THAT(collectIncludes().MainFileIncludes, + Not(Contains(Written("")))); + + // Now include through built-in file. + CDB.ExtraClangFlags = {"-include", testPath(HeaderFile)}; + EXPECT_THAT(collectIncludes().MainFileIncludes, + Contains(AllOf(IncludeLine(2), Written("")))); +} } // namespace } // namespace clangd } // namespace clang