[clang-tools-extra] r318925 - [clangd] Drop impossible completions (unavailable or inaccessible)

Sam McCall via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 23 08:58:22 PST 2017


Author: sammccall
Date: Thu Nov 23 08:58:22 2017
New Revision: 318925

URL: http://llvm.org/viewvc/llvm-project?rev=318925&view=rev
Log:
[clangd] Drop impossible completions (unavailable or inaccessible)

Summary: (There must be some reason why D38077 didn't just do this, but I don't get it!)

Reviewers: ilya-biryukov

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D39836

Modified:
    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

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=318925&r1=318924&r2=318925&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Thu Nov 23 08:58:22 2017
@@ -236,14 +236,12 @@ void ClangdLSPServer::onSwitchSourceHead
 
 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) {

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=318925&r1=318924&r2=318925&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Thu Nov 23 08:58:22 2017
@@ -31,7 +31,8 @@ public:
   /// 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);
 

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=318925&r1=318924&r2=318925&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Thu Nov 23 08:58:22 2017
@@ -169,11 +169,14 @@ ClangdScheduler::~ClangdScheduler() {
     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()),

Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=318925&r1=318924&r2=318925&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Thu Nov 23 08:58:22 2017
@@ -182,9 +182,6 @@ public:
   /// 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 @@ public:
                DiagnosticsConsumer &DiagConsumer,
                FileSystemProvider &FSProvider, unsigned AsyncThreadsCount,
                bool StorePreamblesInMemory,
-               clangd::CodeCompleteOptions CodeCompleteOpts,
+               const clangd::CodeCompleteOptions &CodeCompleteOpts,
                clangd::Logger &Logger,
                llvm::Optional<StringRef> ResourceDir = llvm::None);
 

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=318925&r1=318924&r2=318925&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Thu Nov 23 08:58:22 2017
@@ -436,7 +436,12 @@ public:
                                   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 @@ bool invokeCodeComplete(std::unique_ptr<
 
 } // 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;

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.h?rev=318925&r1=318924&r2=318925&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdUnit.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.h Thu Nov 23 08:58:22 2017
@@ -255,16 +255,6 @@ private:
 };
 
 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 @@ struct CodeCompleteOptions {
   /// 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 @@ struct CodeCompleteOptions {
   /// 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;

Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=318925&r1=318924&r2=318925&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Thu Nov 23 08:58:22 2017
@@ -42,8 +42,18 @@ static llvm::cl::opt<unsigned>
 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 @@ int main(int argc, char *argv[]) {
   // 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");

Modified: clang-tools-extra/trunk/test/clangd/completion-items-kinds.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/completion-items-kinds.test?rev=318925&r1=318924&r2=318925&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clangd/completion-items-kinds.test (original)
+++ clang-tools-extra/trunk/test/clangd/completion-items-kinds.test Thu Nov 23 08:58:22 2017
@@ -10,15 +10,11 @@ Content-Length: 220
 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 @@ Content-Length: 58
 # 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}

Modified: clang-tools-extra/trunk/test/clangd/completion-priorities.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/completion-priorities.test?rev=318925&r1=318924&r2=318925&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clangd/completion-priorities.test (original)
+++ clang-tools-extra/trunk/test/clangd/completion-priorities.test Thu Nov 23 08:58:22 2017
@@ -61,28 +61,10 @@ Content-Length: 151
 # 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}

Modified: clang-tools-extra/trunk/test/clangd/completion-qualifiers.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/completion-qualifiers.test?rev=318925&r1=318924&r2=318925&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clangd/completion-qualifiers.test (original)
+++ clang-tools-extra/trunk/test/clangd/completion-qualifiers.test Thu Nov 23 08:58:22 2017
@@ -13,7 +13,7 @@ Content-Length: 151
 # 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 @@ Content-Length: 151
 # 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"}

Modified: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp?rev=318925&r1=318924&r2=318925&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp Thu Nov 23 08:58:22 2017
@@ -788,6 +788,8 @@ struct ClassWithMembers {
   int method();
 
   int field;
+private:
+  int private_field;
 };
 
 int test() {
@@ -828,7 +830,10 @@ int test() {
       // 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 @@ int test() {
     }
   };
 
-  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 {};




More information about the cfe-commits mailing list