[clang-tools-extra] r359796 - Revert rL359778 : [clangd] Fix code completion of macros defined in the preamble region of the main file.
Simon Pilgrim via cfe-commits
cfe-commits at lists.llvm.org
Thu May 2 08:47:33 PDT 2019
Author: rksimon
Date: Thu May 2 08:47:33 2019
New Revision: 359796
URL: http://llvm.org/viewvc/llvm-project?rev=359796&view=rev
Log:
Revert rL359778 : [clangd] Fix code completion of macros defined in the preamble region of the main file.
Summary:
This is a tricky case (we baked the assumption that symbols come from
the preamble xor mainfile pretty deeply) and the fix is a bit of a hack:
We look at the code to guess the macro names, and deserialize them from
the preamble "by hand".
Reviewers: ilya-biryukov
Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D60937
........
Fix buildbots http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/47684/
Modified:
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.h
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=359796&r1=359795&r2=359796&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Thu May 2 08:47:33 2019
@@ -20,8 +20,6 @@
#include "index/Index.h"
#include "clang/AST/ASTContext.h"
#include "clang/Basic/LangOptions.h"
-#include "clang/Basic/SourceManager.h"
-#include "clang/Basic/TokenKinds.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/FrontendActions.h"
@@ -96,36 +94,6 @@ private:
std::vector<Decl *> TopLevelDecls;
};
-class CollectMainFileMacros : public PPCallbacks {
-public:
- explicit CollectMainFileMacros(const SourceManager &SM,
- std::vector<std::string> *Out)
- : SM(SM), Out(Out) {}
-
- void FileChanged(SourceLocation Loc, FileChangeReason,
- SrcMgr::CharacteristicKind, FileID Prev) {
- InMainFile = SM.isWrittenInMainFile(Loc);
- }
-
- void MacroDefined(const Token &MacroName, const MacroDirective *MD) {
- assert(MacroName.is(tok::identifier));
- if (InMainFile)
- MainFileMacros.insert(MacroName.getIdentifierInfo()->getName());
- }
-
- void EndOfMainFile() {
- for (const auto& Entry : MainFileMacros)
- Out->push_back(Entry.getKey());
- llvm::sort(*Out);
- }
-
- private:
- const SourceManager &SM;
- bool InMainFile = true;
- llvm::StringSet<> MainFileMacros;
- std::vector<std::string> *Out;
-};
-
class CppFilePreambleCallbacks : public PreambleCallbacks {
public:
CppFilePreambleCallbacks(PathRef File, PreambleParsedCallback ParsedCallback)
@@ -135,10 +103,6 @@ public:
IncludeStructure takeIncludes() { return std::move(Includes); }
- std::vector<std::string> takeMainFileMacros() {
- return std::move(MainFileMacros);
- }
-
CanonicalIncludes takeCanonicalIncludes() { return std::move(CanonIncludes); }
void AfterExecute(CompilerInstance &CI) override {
@@ -154,9 +118,7 @@ public:
std::unique_ptr<PPCallbacks> createPPCallbacks() override {
assert(SourceMgr && "SourceMgr must be set at this point");
- return llvm::make_unique<PPChainedCallbacks>(
- collectIncludeStructureCallback(*SourceMgr, &Includes),
- llvm::make_unique<CollectMainFileMacros>(*SourceMgr, &MainFileMacros));
+ return collectIncludeStructureCallback(*SourceMgr, &Includes);
}
CommentHandler *getCommentHandler() override {
@@ -169,7 +131,6 @@ private:
PreambleParsedCallback ParsedCallback;
IncludeStructure Includes;
CanonicalIncludes CanonIncludes;
- std::vector<std::string> MainFileMacros;
std::unique_ptr<CommentHandler> IWYUHandler = nullptr;
SourceManager *SourceMgr = nullptr;
};
@@ -498,13 +459,11 @@ const CanonicalIncludes &ParsedAST::getC
PreambleData::PreambleData(PrecompiledPreamble Preamble,
std::vector<Diag> Diags, IncludeStructure Includes,
- std::vector<std::string> MainFileMacros,
std::unique_ptr<PreambleFileStatusCache> StatCache,
CanonicalIncludes CanonIncludes)
: Preamble(std::move(Preamble)), Diags(std::move(Diags)),
- Includes(std::move(Includes)), MainFileMacros(std::move(MainFileMacros)),
- StatCache(std::move(StatCache)), CanonIncludes(std::move(CanonIncludes)) {
-}
+ Includes(std::move(Includes)), StatCache(std::move(StatCache)),
+ CanonIncludes(std::move(CanonIncludes)) {}
ParsedAST::ParsedAST(std::shared_ptr<const PreambleData> Preamble,
std::unique_ptr<CompilerInstance> Clang,
@@ -583,8 +542,7 @@ buildPreamble(PathRef FileName, Compiler
std::vector<Diag> Diags = PreambleDiagnostics.take();
return std::make_shared<PreambleData>(
std::move(*BuiltPreamble), std::move(Diags),
- SerializedDeclsCollector.takeIncludes(),
- SerializedDeclsCollector.takeMainFileMacros(), std::move(StatCache),
+ SerializedDeclsCollector.takeIncludes(), std::move(StatCache),
SerializedDeclsCollector.takeCanonicalIncludes());
} else {
elog("Could not build a preamble for file {0}", FileName);
Modified: clang-tools-extra/trunk/clangd/ClangdUnit.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.h?rev=359796&r1=359795&r2=359796&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdUnit.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.h Thu May 2 08:47:33 2019
@@ -48,7 +48,6 @@ namespace clangd {
struct PreambleData {
PreambleData(PrecompiledPreamble Preamble, std::vector<Diag> Diags,
IncludeStructure Includes,
- std::vector<std::string> MainFileMacros,
std::unique_ptr<PreambleFileStatusCache> StatCache,
CanonicalIncludes CanonIncludes);
@@ -58,10 +57,6 @@ struct PreambleData {
// Processes like code completions and go-to-definitions will need #include
// information, and their compile action skips preamble range.
IncludeStructure Includes;
- // Macros defined in the preamble section of the main file.
- // Users care about headers vs main-file, not preamble vs non-preamble.
- // These should be treated as main-file entities e.g. for code completion.
- std::vector<std::string> MainFileMacros;
// Cache of FS operations performed when building the preamble.
// When reusing a preamble, this cache can be consumed to save IO.
std::unique_ptr<PreambleFileStatusCache> StatCache;
Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=359796&r1=359795&r2=359796&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Thu May 2 08:47:33 2019
@@ -44,8 +44,6 @@
#include "clang/Format/Format.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/FrontendActions.h"
-#include "clang/Lex/ExternalPreprocessorSource.h"
-#include "clang/Lex/Preprocessor.h"
#include "clang/Lex/PreprocessorOptions.h"
#include "clang/Sema/CodeCompleteConsumer.h"
#include "clang/Sema/DeclSpec.h"
@@ -1019,23 +1017,6 @@ struct SemaCompleteInput {
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS;
};
-void loadMainFilePreambleMacros(const Preprocessor &PP,
- const PreambleData &Preamble) {
- // The ExternalPreprocessorSource has our macros, if we know where to look.
- // We can read all the macros using PreambleMacros->ReadDefinedMacros(),
- // but this includes transitively included files, so may deserialize a lot.
- ExternalPreprocessorSource *PreambleMacros = PP.getExternalSource();
- // As we have the names of the macros, we can look up their IdentifierInfo
- // and then use this to load just the macros we want.
- IdentifierInfoLookup *PreambleIdentifiers =
- PP.getIdentifierTable().getExternalIdentifierLookup();
- if (!PreambleIdentifiers || !PreambleMacros)
- return;
- for (const auto& MacroName : Preamble.MainFileMacros)
- if (auto *II = PreambleIdentifiers->get(MacroName))
- PreambleMacros->updateOutOfDateIdentifier(*II);
-}
-
// Invokes Sema code completion on a file.
// If \p Includes is set, it will be updated based on the compiler invocation.
bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer,
@@ -1077,9 +1058,9 @@ bool semaCodeComplete(std::unique_ptr<Co
// However, if we're completing *inside* the preamble section of the draft,
// overriding the preamble will break sema completion. Fortunately we can just
// skip all includes in this case; these completions are really simple.
- PreambleBounds PreambleRegion =
- ComputePreambleBounds(*CI->getLangOpts(), ContentsBuffer.get(), 0);
- bool CompletingInPreamble = PreambleRegion.Size > Input.Offset;
+ bool CompletingInPreamble =
+ ComputePreambleBounds(*CI->getLangOpts(), ContentsBuffer.get(), 0).Size >
+ Input.Offset;
// NOTE: we must call BeginSourceFile after prepareCompilerInstance. Otherwise
// the remapped buffers do not get freed.
IgnoreDiagnostics DummyDiagsConsumer;
@@ -1097,14 +1078,6 @@ bool semaCodeComplete(std::unique_ptr<Co
Input.FileName);
return false;
}
- // Macros can be defined within the preamble region of the main file.
- // They don't fall nicely into our index/Sema dichotomy:
- // - they're not indexed for completion (they're not available across files)
- // - but Sema code complete won't see them: as part of the preamble, they're
- // deserialized only when mentioned.
- // Force them to be deserialized so SemaCodeComplete sees them.
- if (Input.Preamble)
- loadMainFilePreambleMacros(Clang->getPreprocessor(), *Input.Preamble);
if (Includes)
Clang->getPreprocessor().addPPCallbacks(
collectIncludeStructureCallback(Clang->getSourceManager(), Includes));
Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=359796&r1=359795&r2=359796&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Thu May 2 08:47:33 2019
@@ -24,7 +24,6 @@
#include "clang/Basic/SourceManager.h"
#include "clang/Basic/Specifiers.h"
#include "clang/Index/IndexSymbol.h"
-#include "clang/Index/IndexingAction.h"
#include "clang/Index/USRGeneration.h"
#include "clang/Lex/Preprocessor.h"
#include "llvm/Support/Casting.h"
Modified: clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp?rev=359796&r1=359795&r2=359796&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/CodeCompleteTests.cpp Thu May 2 08:47:33 2019
@@ -2073,28 +2073,19 @@ TEST(CompletionTest, MergeMacrosFromInde
UnorderedElementsAre(Named("Clangd_Macro_Test")));
}
-TEST(CompletionTest, MacroFromPreamble) {
- MockFSProvider FS;
- MockCompilationDatabase CDB;
- std::string FooHeader = testPath("foo.h");
- FS.Files[FooHeader] = "#define CLANGD_PREAMBLE_HEADER x\n";
- IgnoreDiagnostics DiagConsumer;
- ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+TEST(CompletionTest, NoMacroFromPreambleIfIndexIsSet) {
auto Results = completions(
- R"cpp(#include "foo.h"
- #define CLANGD_PREAMBLE_MAIN x
+ R"cpp(#define CLANGD_PREAMBLE x
int x = 0;
#define CLANGD_MAIN x
void f() { CLANGD_^ }
)cpp",
{func("CLANGD_INDEX")});
- // We should get results from the main file, including the preamble section.
- // However no results from included files (the index should cover them).
- EXPECT_THAT(Results.Completions,
- UnorderedElementsAre(Named("CLANGD_PREAMBLE_MAIN"),
- Named("CLANGD_MAIN"),
- Named("CLANGD_INDEX")));
+ // Index is overriden in code completion options, so the preamble symbol is
+ // not seen.
+ EXPECT_THAT(Results.Completions, UnorderedElementsAre(Named("CLANGD_MAIN"),
+ Named("CLANGD_INDEX")));
}
TEST(CompletionTest, DeprecatedResults) {
More information about the cfe-commits
mailing list