sammccall created this revision.
sammccall added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
ioeric.
In some circumstances we provide bad completions or no completions, because of
problems in the code. This can be puzzling and opaque to users.
If we can tell the user that this is the case and why, they'll be happy.
Experiments with go language servers have shown this to be a big win.
Two major problems:
- Sema doesn't provide the information we need
- LSP provides no protocol mechanims, and editors don't have UI for this
This patch attempts to guess the information, looking at diagnostics on the
line.
Other heuristics are possible (e.g. using completion context). It's unclear how
useful or successful they will be. This is mostly a quick hack to test
viability.
This is exposed as an extension of the C++ API only (not bound to LSP).
The idea is to test viability with a non-public editor that happens to have the
right UI already (and doesn't use LSP), and experiment with approaches.
If something fairly simple works well, we'll keep this and expose an LSP
extension.
If it's not useful, we should drop this idea.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53406
Files:
clangd/ClangdLSPServer.cpp
clangd/CodeComplete.cpp
clangd/CodeComplete.h
unittests/clangd/CodeCompleteTests.cpp
Index: unittests/clangd/CodeCompleteTests.cpp
===================================================================
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -2176,6 +2176,50 @@
AllOf(Qualifier("nx::"), Named("Clangd2"))));
}
+TEST(CompletionTest, Excuses) {
+ struct {
+ const char *Code;
+ const char *Excuse;
+ } Tests[] = {
+ {
+ R"cpp(
+ class X;
+ X* x;
+ int main() {
+ x->^
+ }
+ )cpp",
+ "member access into incomplete type 'X'",
+ },
+ {
+ R"cpp(
+ // Error is on same line.
+ template <class> struct X;
+ X<int> x; ^
+ )cpp",
+ "implicit instantiation of undefined template 'X<int>'",
+ },
+ {
+ R"cpp(
+ // Error is on previous line.
+ template <class> struct X;
+ X<int> x;
+ ^
+ )cpp",
+ "",
+ },
+ {
+ R"cpp(
+ // Just a warning (declaration does not declare anything).
+ int; x^
+ )cpp",
+ "",
+ },
+ };
+ for (const auto &Test : Tests)
+ EXPECT_EQ(completions(Test.Code).Excuse, Test.Excuse) << Test.Code;
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/CodeComplete.h
===================================================================
--- clangd/CodeComplete.h
+++ clangd/CodeComplete.h
@@ -196,6 +196,10 @@
std::vector<CodeCompletion> Completions;
bool HasMore = false;
CodeCompletionContext::Kind Context = CodeCompletionContext::CCC_Other;
+
+ // Our best guess at why completions might be poor (human-readable string).
+ // Only set if we suspect completions are poor *and* we know why.
+ std::string Excuse;
};
raw_ostream &operator<<(raw_ostream &, const CodeCompleteResult &);
Index: clangd/CodeComplete.cpp
===================================================================
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -671,6 +671,42 @@
return false;
}
+// Tries to come up with a convincing excuse for our bad completion results,
+// based on the diagnostics near the completion point.
+class ExcuseMaker : public DiagnosticConsumer {
+ unsigned StartOfLine, Offset;
+ std::string &Excuse;
+ SmallString<256> ExcuseBuf;
+
+ public:
+ ExcuseMaker(StringRef Code, unsigned Offset, std::string &Excuse)
+ : Offset(Offset), Excuse(Excuse) {
+ assert(Offset <= Code.size());
+ for (StartOfLine = Offset;
+ StartOfLine > 0 && Code[StartOfLine - 1] != '\n'; --StartOfLine)
+ ;
+ }
+
+ void HandleDiagnostic(DiagnosticsEngine::Level Level,
+ const clang::Diagnostic &Diag) override {
+ // We accept >= error diagnostics, before the cursor but on the same line.
+ if (Level < DiagnosticsEngine::Error || !Diag.hasSourceManager())
+ return;
+ auto& SM = Diag.getSourceManager();
+ auto Loc = SM.getDecomposedLoc(Diag.getLocation());
+ if (Loc.first != SM.getMainFileID())
+ return;
+ unsigned DiagLoc = Loc.second;
+ if (DiagLoc < StartOfLine || DiagLoc > Offset)
+ return;
+ ExcuseBuf.clear();
+ Diag.FormatDiagnostic(ExcuseBuf);
+ }
+
+ ~ExcuseMaker() { Excuse = ExcuseBuf.str(); }
+};
+
+
// The CompletionRecorder captures Sema code-complete output, including context.
// It filters out ignored results (but doesn't apply fuzzy-filtering yet).
// It doesn't do scoring or conversion to CompletionItem yet, as we want to
@@ -1000,7 +1036,8 @@
bool semaCodeComplete(std::unique_ptr<CodeCompleteConsumer> Consumer,
const clang::CodeCompleteOptions &Options,
const SemaCompleteInput &Input,
- IncludeStructure *Includes = nullptr) {
+ IncludeStructure *Includes = nullptr,
+ std::string *Excuse = nullptr) {
trace::Span Tracer("Sema completion");
std::vector<const char *> ArgStrs;
for (const auto &S : Input.Command.CommandLine)
@@ -1012,14 +1049,21 @@
// working dirs.
}
+ auto Offset = positionToOffset(Input.Contents, Input.Pos);
+ if (!Offset) {
+ elog("Code completion position was invalid {0}", Offset.takeError());
+ return false;
+ }
+ std::unique_ptr<DiagnosticConsumer> DiagConsumer(
+ Excuse ? new ExcuseMaker(Input.Contents, *Offset, *Excuse)
+ : (DiagnosticConsumer *)new IgnoreDiagnostics());
IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS = Input.VFS;
if (Input.Preamble && Input.Preamble->StatCache)
VFS = Input.Preamble->StatCache->getConsumingFS(std::move(VFS));
- IgnoreDiagnostics DummyDiagsConsumer;
auto CI = createInvocationFromCommandLine(
ArgStrs,
CompilerInstance::createDiagnostics(new DiagnosticOptions,
- &DummyDiagsConsumer, false),
+ DiagConsumer.get(), false),
VFS);
if (!CI) {
elog("Couldn't create CompilerInvocation");
@@ -1034,11 +1078,6 @@
// Setup code completion.
FrontendOpts.CodeCompleteOpts = Options;
FrontendOpts.CodeCompletionAt.FileName = Input.FileName;
- auto Offset = positionToOffset(Input.Contents, Input.Pos);
- if (!Offset) {
- elog("Code completion position was invalid {0}", Offset.takeError());
- return false;
- }
std::tie(FrontendOpts.CodeCompletionAt.Line,
FrontendOpts.CodeCompletionAt.Column) =
offsetToClangLineColumn(Input.Contents, *Offset);
@@ -1063,7 +1102,7 @@
(Input.Preamble && !CompletingInPreamble) ? &Input.Preamble->Preamble
: nullptr,
std::move(ContentsBuffer), std::move(Input.PCHs), std::move(VFS),
- DummyDiagsConsumer);
+ *DiagConsumer);
Clang->getPreprocessorOpts().SingleFileParseMode = CompletingInPreamble;
Clang->setCodeCompletionConsumer(Consumer.release());
@@ -1308,7 +1347,7 @@
Recorder = RecorderOwner.get();
semaCodeComplete(std::move(RecorderOwner), Opts.getClangCompleteOpts(),
- SemaCCInput, &Includes);
+ SemaCCInput, &Includes, &Output.Excuse);
SPAN_ATTACH(Tracer, "sema_results", NSema);
SPAN_ATTACH(Tracer, "index_results", NIndex);
Index: clangd/ClangdLSPServer.cpp
===================================================================
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -586,6 +586,7 @@
llvm::Expected<CodeCompleteResult> List) {
if (!List)
return Reply(List.takeError());
+ log("completion excuse: {0}", List->Excuse);
CompletionList LSPList;
LSPList.isIncomplete = List->HasMore;
for (const auto &R : List->Completions) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
