sammccall created this revision.
sammccall added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
ioeric.
Herald added a project: clang.
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".
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D60937
Files:
clangd/CodeComplete.cpp
clangd/index/SymbolCollector.cpp
unittests/clangd/CodeCompleteTests.cpp
Index: unittests/clangd/CodeCompleteTests.cpp
===================================================================
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -2069,19 +2069,28 @@
UnorderedElementsAre(Named("Clangd_Macro_Test")));
}
-TEST(CompletionTest, NoMacroFromPreambleIfIndexIsSet) {
+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());
auto Results = completions(
- R"cpp(#define CLANGD_PREAMBLE x
+ R"cpp(#include "foo.h"
+ define CLANGD_PREAMBLE_MAIN x
int x = 0;
#define CLANGD_MAIN x
void f() { CLANGD_^ }
)cpp",
{func("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")));
+ // 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")));
}
TEST(CompletionTest, DeprecatedResults) {
Index: clangd/index/SymbolCollector.cpp
===================================================================
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -24,6 +24,7 @@
#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"
Index: clangd/CodeComplete.cpp
===================================================================
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -44,6 +44,8 @@
#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,6 +1021,44 @@
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS;
};
+void loadMainFilePreambleMacros(const Preprocessor &PP,
+ llvm::StringRef PreambleText) {
+ // 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();
+ // If 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;
+ auto Probe = [&](llvm::StringRef MacroName) {
+ if (auto *II = PreambleIdentifiers->get(MacroName))
+ PreambleMacros->updateOutOfDateIdentifier(*II);
+ };
+
+ // We get the identifiers by lexing the preamble text for #define lines.
+ // This may contain false positives (e.g. #if'ed out), it doesn't matter.
+ // This doesn't handle pathological input (e.g line continuation before name).
+ llvm::StringRef Line;
+ while (!PreambleText.empty()) {
+ std::tie(Line, PreambleText) = PreambleText.split('\n');
+ Line = Line.ltrim();
+ if (!Line.consume_front("#"))
+ continue;
+ Line = Line.ltrim();
+ if (!Line.consume_front("define"))
+ continue;
+ Line = Line.ltrim();
+ if (Line.empty() || !isIdentifierHead(Line.front()))
+ continue;
+ llvm::StringRef MacroName =
+ Line.take_while([&](char C) { return isIdentifierBody(C); });
+ Probe(MacroName);
+ }
+}
+
// 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,
@@ -1060,9 +1100,9 @@
// 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.
- bool CompletingInPreamble =
- ComputePreambleBounds(*CI->getLangOpts(), ContentsBuffer.get(), 0).Size >
- Input.Offset;
+ PreambleBounds PreambleRegion =
+ ComputePreambleBounds(*CI->getLangOpts(), ContentsBuffer.get(), 0);
+ bool CompletingInPreamble = PreambleRegion.Size > Input.Offset;
// NOTE: we must call BeginSourceFile after prepareCompilerInstance. Otherwise
// the remapped buffers do not get freed.
IgnoreDiagnostics DummyDiagsConsumer;
@@ -1080,6 +1120,14 @@
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.
+ loadMainFilePreambleMacros(Clang->getPreprocessor(),
+ Input.Contents.take_front(PreambleRegion.Size));
if (Includes)
Clang->getPreprocessor().addPPCallbacks(
collectIncludeStructureCallback(Clang->getSourceManager(), Includes));
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
