This revision was automatically updated to reflect the committed changes.
Closed by commit rL318925: [clangd] Drop impossible completions (unavailable or
inaccessible) (authored by sammccall).
Repository:
rL LLVM
https://reviews.llvm.org/D39836
Files:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdLSPServer.h
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.h
clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
clang-tools-extra/trunk/test/clangd/completion-items-kinds.test
clang-tools-extra/trunk/test/clangd/completion-priorities.test
clang-tools-extra/trunk/test/clangd/completion-qualifiers.test
clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
Index: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
===================================================================
--- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
@@ -788,6 +788,8 @@
int method();
int field;
+private:
+ int private_field;
};
int test() {
@@ -828,7 +830,10 @@
// Class members. The only items that must be present in after-dor
// completion.
EXPECT_TRUE(ContainsItem(Results, MethodItemText));
+ EXPECT_TRUE(ContainsItem(Results, MethodItemText));
EXPECT_TRUE(ContainsItem(Results, "field"));
+ EXPECT_EQ(Opts.IncludeIneligibleResults,
+ ContainsItem(Results, "private_field"));
// Global items.
EXPECT_FALSE(ContainsItem(Results, "global_var"));
EXPECT_FALSE(ContainsItem(Results, GlobalFuncItemText));
@@ -889,18 +894,26 @@
}
};
- for (bool IncludeMacros : {true, false})
- for (bool IncludeGlobals : {true, false})
- for (bool IncludeBriefComments : {true, false})
- for (bool EnableSnippets : {true, false})
+ clangd::CodeCompleteOptions CCOpts;
+ for (bool IncludeMacros : {true, false}){
+ CCOpts.IncludeMacros = IncludeMacros;
+ for (bool IncludeGlobals : {true, false}){
+ CCOpts.IncludeGlobals = IncludeGlobals;
+ for (bool IncludeBriefComments : {true, false}){
+ CCOpts.IncludeBriefComments = IncludeBriefComments;
+ for (bool EnableSnippets : {true, false}){
+ CCOpts.EnableSnippets = EnableSnippets;
for (bool IncludeCodePatterns : {true, false}) {
- TestWithOpts(clangd::CodeCompleteOptions(
- /*EnableSnippets=*/EnableSnippets,
- /*IncludeCodePatterns=*/IncludeCodePatterns,
- /*IncludeMacros=*/IncludeMacros,
- /*IncludeGlobals=*/IncludeGlobals,
- /*IncludeBriefComments=*/IncludeBriefComments));
+ CCOpts.IncludeCodePatterns = IncludeCodePatterns;
+ for (bool IncludeIneligibleResults : {true, false}) {
+ CCOpts.IncludeIneligibleResults = IncludeIneligibleResults;
+ TestWithOpts(CCOpts);
+ }
}
+ }
+ }
+ }
+ }
}
class ClangdThreadingTest : public ClangdVFSTest {};
Index: clang-tools-extra/trunk/clangd/ClangdServer.h
===================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.h
+++ clang-tools-extra/trunk/clangd/ClangdServer.h
@@ -182,9 +182,6 @@
/// AsyncThreadsCount worker threads. However, if \p AsyncThreadsCount is 0,
/// all requests will be processed on the calling thread.
///
- /// When \p SnippetCompletions is true, completion items will be presented
- /// with embedded snippets. Otherwise, plaintext items will be presented.
- ///
/// ClangdServer uses \p FSProvider to get an instance of vfs::FileSystem for
/// each parsing request. Results of code completion and diagnostics also
/// include a tag, that \p FSProvider returns along with the vfs::FileSystem.
@@ -213,7 +210,7 @@
DiagnosticsConsumer &DiagConsumer,
FileSystemProvider &FSProvider, unsigned AsyncThreadsCount,
bool StorePreamblesInMemory,
- clangd::CodeCompleteOptions CodeCompleteOpts,
+ const clangd::CodeCompleteOptions &CodeCompleteOpts,
clangd::Logger &Logger,
llvm::Optional<StringRef> ResourceDir = llvm::None);
Index: clang-tools-extra/trunk/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp
@@ -169,11 +169,14 @@
Worker.join();
}
-ClangdServer::ClangdServer(
- GlobalCompilationDatabase &CDB, DiagnosticsConsumer &DiagConsumer,
- FileSystemProvider &FSProvider, unsigned AsyncThreadsCount,
- bool StorePreamblesInMemory, clangd::CodeCompleteOptions CodeCompleteOpts,
- clangd::Logger &Logger, llvm::Optional<StringRef> ResourceDir)
+ClangdServer::ClangdServer(GlobalCompilationDatabase &CDB,
+ DiagnosticsConsumer &DiagConsumer,
+ FileSystemProvider &FSProvider,
+ unsigned AsyncThreadsCount,
+ bool StorePreamblesInMemory,
+ const clangd::CodeCompleteOptions &CodeCompleteOpts,
+ clangd::Logger &Logger,
+ llvm::Optional<StringRef> ResourceDir)
: Logger(Logger), CDB(CDB), DiagConsumer(DiagConsumer),
FSProvider(FSProvider),
ResourceDir(ResourceDir ? ResourceDir->str() : getStandardResourceDir()),
Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
===================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.h
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h
@@ -31,7 +31,8 @@
/// loaded only from \p CompileCommandsDir. Otherwise, clangd will look
/// for compile_commands.json in all parent directories of each file.
ClangdLSPServer(JSONOutput &Out, unsigned AsyncThreadsCount,
- bool StorePreamblesInMemory, bool SnippetCompletions,
+ bool StorePreamblesInMemory,
+ const clangd::CodeCompleteOptions &CCOpts,
llvm::Optional<StringRef> ResourceDir,
llvm::Optional<Path> CompileCommandsDir);
Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
@@ -236,14 +236,12 @@
ClangdLSPServer::ClangdLSPServer(JSONOutput &Out, unsigned AsyncThreadsCount,
bool StorePreamblesInMemory,
- bool SnippetCompletions,
+ const clangd::CodeCompleteOptions &CCOpts,
llvm::Optional<StringRef> ResourceDir,
llvm::Optional<Path> CompileCommandsDir)
: Out(Out), CDB(/*Logger=*/Out, std::move(CompileCommandsDir)),
Server(CDB, /*DiagConsumer=*/*this, FSProvider, AsyncThreadsCount,
- StorePreamblesInMemory,
- clangd::CodeCompleteOptions(
- /*EnableSnippetsAndCodePatterns=*/SnippetCompletions),
+ StorePreamblesInMemory, CCOpts,
/*Logger=*/Out, ResourceDir) {}
bool ClangdLSPServer::run(std::istream &In) {
Index: clang-tools-extra/trunk/clangd/ClangdUnit.h
===================================================================
--- clang-tools-extra/trunk/clangd/ClangdUnit.h
+++ clang-tools-extra/trunk/clangd/ClangdUnit.h
@@ -255,16 +255,6 @@
};
struct CodeCompleteOptions {
- CodeCompleteOptions() = default;
-
- /// Uses default values for all flags, but sets EnableSnippets and
- /// IncludeCodePatterns to the value of EnableSnippetsAndCodePatterns.
- explicit CodeCompleteOptions(bool EnableSnippetsAndCodePatterns);
-
- CodeCompleteOptions(bool EnableSnippets, bool IncludeCodePatterns,
- bool IncludeMacros, bool IncludeGlobals,
- bool IncludeBriefComments);
-
/// Returns options that can be passed to clang's completion engine.
clang::CodeCompleteOptions getClangCompleteOpts() const;
@@ -276,7 +266,7 @@
/// Add code patterns to completion results.
/// If EnableSnippets is false, this options is ignored and code patterns will
/// always be omitted.
- bool IncludeCodePatterns = false;
+ bool IncludeCodePatterns = true;
/// Add macros to code completion results.
bool IncludeMacros = true;
@@ -289,6 +279,10 @@
/// down completion, investigate if it can be made faster.
bool IncludeBriefComments = true;
+ /// Include results that are not legal completions in the current context.
+ /// For example, private members are usually inaccessible.
+ bool IncludeIneligibleResults = false;
+
/// Limit the number of results returned (0 means no limit).
/// If more results are available, we set CompletionList.isIncomplete.
size_t Limit = 0;
Index: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
@@ -42,8 +42,18 @@
static llvm::cl::opt<bool> EnableSnippets(
"enable-snippets",
llvm::cl::desc(
- "Present snippet completions instead of plaintext completions"),
- llvm::cl::init(false));
+ "Present snippet completions instead of plaintext completions. "
+ "This also enables code pattern results." /* FIXME: should it? */),
+ llvm::cl::init(clangd::CodeCompleteOptions().EnableSnippets));
+
+// FIXME: Flags are the wrong mechanism for user preferences.
+// We should probably read a dotfile or similar.
+static llvm::cl::opt<bool> IncludeIneligibleResults(
+ "include-ineligible-results",
+ llvm::cl::desc(
+ "Include ineligible completion results (e.g. private members)"),
+ llvm::cl::init(clangd::CodeCompleteOptions().IncludeIneligibleResults),
+ llvm::cl::Hidden);
static llvm::cl::opt<bool>
PrettyPrint("pretty", llvm::cl::desc("Pretty-print JSON output"),
@@ -157,9 +167,12 @@
// Change stdin to binary to not lose \r\n on windows.
llvm::sys::ChangeStdinToBinary();
+ clangd::CodeCompleteOptions CCOpts;
+ CCOpts.EnableSnippets = EnableSnippets;
+ CCOpts.IncludeIneligibleResults = IncludeIneligibleResults;
// Initialize and run ClangdLSPServer.
ClangdLSPServer LSPServer(Out, WorkerThreadsCount, StorePreamblesInMemory,
- EnableSnippets, ResourceDirRef,
+ CCOpts, ResourceDirRef,
CompileCommandsDirPath);
constexpr int NoShutdownRequestErrorCode = 1;
llvm::set_thread_name("clangd.main");
Index: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
===================================================================
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp
@@ -436,7 +436,12 @@
unsigned NumResults) override final {
std::priority_queue<CompletionCandidate> Candidates;
for (unsigned I = 0; I < NumResults; ++I) {
- Candidates.emplace(Results[I]);
+ auto &Result = Results[I];
+ if (!ClangdOpts.IncludeIneligibleResults &&
+ (Result.Availability == CXAvailability_NotAvailable ||
+ Result.Availability == CXAvailability_NotAccessible))
+ continue;
+ Candidates.emplace(Result);
if (ClangdOpts.Limit && Candidates.size() > ClangdOpts.Limit) {
Candidates.pop();
Items.isIncomplete = true;
@@ -806,22 +811,6 @@
} // namespace
-clangd::CodeCompleteOptions::CodeCompleteOptions(
- bool EnableSnippetsAndCodePatterns)
- : CodeCompleteOptions() {
- EnableSnippets = EnableSnippetsAndCodePatterns;
- IncludeCodePatterns = EnableSnippetsAndCodePatterns;
-}
-
-clangd::CodeCompleteOptions::CodeCompleteOptions(bool EnableSnippets,
- bool IncludeCodePatterns,
- bool IncludeMacros,
- bool IncludeGlobals,
- bool IncludeBriefComments)
- : EnableSnippets(EnableSnippets), IncludeCodePatterns(IncludeCodePatterns),
- IncludeMacros(IncludeMacros), IncludeGlobals(IncludeGlobals),
- IncludeBriefComments(IncludeBriefComments) {}
-
clang::CodeCompleteOptions
clangd::CodeCompleteOptions::getClangCompleteOpts() const {
clang::CodeCompleteOptions Result;
Index: clang-tools-extra/trunk/test/clangd/completion-qualifiers.test
===================================================================
--- clang-tools-extra/trunk/test/clangd/completion-qualifiers.test
+++ clang-tools-extra/trunk/test/clangd/completion-qualifiers.test
@@ -13,7 +13,7 @@
# CHECK-NEXT: "result": {
# CHECK-NEXT: "isIncomplete": false,
# CHECK-NEXT: "items": [
-# Eligible const functions are at the top of the list.
+# Eligible functions are at the top of the list.
# CHECK-NEXT: {
# CHECK-NEXT: "detail": "int",
# CHECK-NEXT: "filterText": "bar",
@@ -32,18 +32,9 @@
# CHECK-NEXT: "label": "Foo::foo() const",
# CHECK-NEXT: "sortText": "000037foo"
# CHECK-NEXT: },
-# Ineligible non-const function is at the bottom of the list.
-# CHECK-NEXT: {
-# CHECK: "detail": "int",
-# CHECK: "filterText": "foo",
-# CHECK-NEXT: "insertText": "foo",
-# CHECK-NEXT: "insertTextFormat": 1,
-# CHECK-NEXT: "kind": 2,
-# CHECK-NEXT: "label": "foo() const",
-# CHECK-NEXT: "sortText": "200035foo"
-# CHECK-NEXT: }
-# CHECK-NEXT: ]
-# CHECK-NEXT: }
+# Ineligible private functions are not present.
+# CHECK-NOT: "label": "foo() const",
+# CHECK: ]
Content-Length: 44
{"jsonrpc":"2.0","id":4,"method":"shutdown"}
Index: clang-tools-extra/trunk/test/clangd/completion-priorities.test
===================================================================
--- clang-tools-extra/trunk/test/clangd/completion-priorities.test
+++ clang-tools-extra/trunk/test/clangd/completion-priorities.test
@@ -61,28 +61,10 @@
# CHECK-NEXT: "kind": 2,
# CHECK-NEXT: "label": "pub()",
# CHECK-NEXT: "sortText": "000034pub"
-# CHECK-NEXT: },
-# priv() and prot() are at the end of the list
-# CHECK-NEXT: {
-# CHECK: "detail": "void",
-# CHECK: "filterText": "priv",
-# CHECK-NEXT: "insertText": "priv",
-# CHECK-NEXT: "insertTextFormat": 1,
-# CHECK-NEXT: "kind": 2,
-# CHECK-NEXT: "label": "priv()",
-# CHECK-NEXT: "sortText": "200034priv"
-# CHECK-NEXT: },
-# CHECK-NEXT: {
-# CHECK-NEXT: "detail": "void",
-# CHECK-NEXT: "filterText": "prot",
-# CHECK-NEXT: "insertText": "prot",
-# CHECK-NEXT: "insertTextFormat": 1,
-# CHECK-NEXT: "kind": 2,
-# CHECK-NEXT: "label": "prot()",
-# CHECK-NEXT: "sortText": "200034prot"
# CHECK-NEXT: }
-# CHECK-NEXT: ]
-# CHECK-NEXT: }
+# CHECK-NOT: "label": "priv()",
+# CHECK-NOT: "label": "prot()",
+# CHECK: ]
Content-Length: 58
{"jsonrpc":"2.0","id":4,"method":"shutdown","params":null}
Index: clang-tools-extra/trunk/test/clangd/completion-items-kinds.test
===================================================================
--- clang-tools-extra/trunk/test/clangd/completion-items-kinds.test
+++ clang-tools-extra/trunk/test/clangd/completion-items-kinds.test
@@ -10,15 +10,11 @@
Content-Length: 148
{"jsonrpc":"2.0","id":1,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":4,"character":7}}}
-Content-Length: 58
# CHECK: {"id":1,"jsonrpc":"2.0","result":{"isIncomplete":false,"items":
#
# Keyword
# CHECK-DAG: {"filterText":"int","insertText":"int","insertTextFormat":1,"kind":14,"label":"int","sortText":"000050int"}
#
-# Code pattern
-# CHECK-DAG: {"filterText":"static_cast","insertText":"static_cast<${1:type}>(${2:expression})","insertTextFormat":2,"kind":15,"label":"static_cast<type>(expression)","sortText":"000040static_cast"}
-#
# Struct
# CHECK-DAG: {"filterText":"Struct","insertText":"Struct","insertTextFormat":1,"kind":7,"label":"Struct","sortText":"000050Struct"}
#
@@ -32,5 +28,15 @@
# CHECK-DAG: {"detail":"int","filterText":"function","insertText":"function()","insertTextFormat":1,"kind":3,"label":"function()","sortText":"000012function"}
#
# CHECK-SAME: ]}}
+Content-Length: 146
+
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///main.cpp","languageId":"cpp","version":1,"text":"whi"}}}
+Content-Length: 148
+
+{"jsonrpc":"2.0","id":2,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":1,"character":3}}}
+# Code pattern (unfortunately there are none in expression context)
+# CHECK-DAG: {"filterText":"namespace","insertText":"namespace ${1:identifier}{${2:declarations}\n}","insertTextFormat":2,"kind":15,"label":"namespace identifier{declarations}","sortText":"000040namespace"}
+#
+Content-Length: 58
{"jsonrpc":"2.0","id":3,"method":"shutdown","params":null}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
