[clang-tools-extra] r333174 - [clangd] Fix code completion in MACROs with stringification.

Eric Liu via cfe-commits cfe-commits at lists.llvm.org
Thu May 24 04:20:19 PDT 2018


Author: ioeric
Date: Thu May 24 04:20:19 2018
New Revision: 333174

URL: http://llvm.org/viewvc/llvm-project?rev=333174&view=rev
Log:
[clangd] Fix code completion in MACROs with stringification.

Summary:
Currently, we only handle the first callback from sema code completion
and ignore results from potential following callbacks. This causes
causes loss of completion results when multiple contexts are tried by Sema.

For example, we wouldn't get any completion result in the following completion
as the first attemped context is natural language which has no
candidate. The parser would backtrack and tried a completion with AST
semantic, which would find candidate "::x".

```
void f(const char*, int);
#define F(x) f(#x, x)
int x;
void main() {
	F(::^);
}
```

To fix this, we only process a sema callback when it gives completion results or
the context supports index-based completion.

Reviewers: ilya-biryukov

Reviewed By: ilya-biryukov

Subscribers: klimek, MaskRay, jkorous, cfe-commits

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

Modified:
    clang-tools-extra/trunk/clangd/CodeComplete.cpp
    clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=333174&r1=333173&r2=333174&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Thu May 24 04:20:19 2018
@@ -406,6 +406,50 @@ std::vector<std::string> getQueryScopes(
   return Info.scopesForIndexQuery();
 }
 
+// Should we perform index-based completion in a context of the specified kind?
+// FIXME: consider allowing completion, but restricting the result types.
+bool contextAllowsIndex(enum CodeCompletionContext::Kind K) {
+  switch (K) {
+  case CodeCompletionContext::CCC_TopLevel:
+  case CodeCompletionContext::CCC_ObjCInterface:
+  case CodeCompletionContext::CCC_ObjCImplementation:
+  case CodeCompletionContext::CCC_ObjCIvarList:
+  case CodeCompletionContext::CCC_ClassStructUnion:
+  case CodeCompletionContext::CCC_Statement:
+  case CodeCompletionContext::CCC_Expression:
+  case CodeCompletionContext::CCC_ObjCMessageReceiver:
+  case CodeCompletionContext::CCC_EnumTag:
+  case CodeCompletionContext::CCC_UnionTag:
+  case CodeCompletionContext::CCC_ClassOrStructTag:
+  case CodeCompletionContext::CCC_ObjCProtocolName:
+  case CodeCompletionContext::CCC_Namespace:
+  case CodeCompletionContext::CCC_Type:
+  case CodeCompletionContext::CCC_Name: // FIXME: why does ns::^ give this?
+  case CodeCompletionContext::CCC_PotentiallyQualifiedName:
+  case CodeCompletionContext::CCC_ParenthesizedExpression:
+  case CodeCompletionContext::CCC_ObjCInterfaceName:
+  case CodeCompletionContext::CCC_ObjCCategoryName:
+    return true;
+  case CodeCompletionContext::CCC_Other: // Be conservative.
+  case CodeCompletionContext::CCC_OtherWithMacros:
+  case CodeCompletionContext::CCC_DotMemberAccess:
+  case CodeCompletionContext::CCC_ArrowMemberAccess:
+  case CodeCompletionContext::CCC_ObjCPropertyAccess:
+  case CodeCompletionContext::CCC_MacroName:
+  case CodeCompletionContext::CCC_MacroNameUse:
+  case CodeCompletionContext::CCC_PreprocessorExpression:
+  case CodeCompletionContext::CCC_PreprocessorDirective:
+  case CodeCompletionContext::CCC_NaturalLanguage:
+  case CodeCompletionContext::CCC_SelectorName:
+  case CodeCompletionContext::CCC_TypeQualifiers:
+  case CodeCompletionContext::CCC_ObjCInstanceMessage:
+  case CodeCompletionContext::CCC_ObjCClassMessage:
+  case CodeCompletionContext::CCC_Recovery:
+    return false;
+  }
+  llvm_unreachable("unknown code completion context");
+}
+
 // 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
@@ -431,12 +475,17 @@ struct CompletionRecorder : public CodeC
   void ProcessCodeCompleteResults(class Sema &S, CodeCompletionContext Context,
                                   CodeCompletionResult *InResults,
                                   unsigned NumResults) override final {
+    // If a callback is called without any sema result and the context does not
+    // support index-based completion, we simply skip it to give way to
+    // potential future callbacks with results.
+    if (NumResults == 0 && !contextAllowsIndex(Context.getKind()))
+      return;
     if (CCSema) {
       log(llvm::formatv(
           "Multiple code complete callbacks (parser backtracked?). "
           "Dropping results from context {0}, keeping results from {1}.",
-          getCompletionKindString(this->CCContext.getKind()),
-          getCompletionKindString(Context.getKind())));
+          getCompletionKindString(Context.getKind()),
+          getCompletionKindString(this->CCContext.getKind())));
       return;
     }
     // Record the completion context.
@@ -719,50 +768,6 @@ bool semaCodeComplete(std::unique_ptr<Co
   return true;
 }
 
-// Should we perform index-based completion in a context of the specified kind?
-// FIXME: consider allowing completion, but restricting the result types.
-bool contextAllowsIndex(enum CodeCompletionContext::Kind K) {
-  switch (K) {
-  case CodeCompletionContext::CCC_TopLevel:
-  case CodeCompletionContext::CCC_ObjCInterface:
-  case CodeCompletionContext::CCC_ObjCImplementation:
-  case CodeCompletionContext::CCC_ObjCIvarList:
-  case CodeCompletionContext::CCC_ClassStructUnion:
-  case CodeCompletionContext::CCC_Statement:
-  case CodeCompletionContext::CCC_Expression:
-  case CodeCompletionContext::CCC_ObjCMessageReceiver:
-  case CodeCompletionContext::CCC_EnumTag:
-  case CodeCompletionContext::CCC_UnionTag:
-  case CodeCompletionContext::CCC_ClassOrStructTag:
-  case CodeCompletionContext::CCC_ObjCProtocolName:
-  case CodeCompletionContext::CCC_Namespace:
-  case CodeCompletionContext::CCC_Type:
-  case CodeCompletionContext::CCC_Name: // FIXME: why does ns::^ give this?
-  case CodeCompletionContext::CCC_PotentiallyQualifiedName:
-  case CodeCompletionContext::CCC_ParenthesizedExpression:
-  case CodeCompletionContext::CCC_ObjCInterfaceName:
-  case CodeCompletionContext::CCC_ObjCCategoryName:
-    return true;
-  case CodeCompletionContext::CCC_Other: // Be conservative.
-  case CodeCompletionContext::CCC_OtherWithMacros:
-  case CodeCompletionContext::CCC_DotMemberAccess:
-  case CodeCompletionContext::CCC_ArrowMemberAccess:
-  case CodeCompletionContext::CCC_ObjCPropertyAccess:
-  case CodeCompletionContext::CCC_MacroName:
-  case CodeCompletionContext::CCC_MacroNameUse:
-  case CodeCompletionContext::CCC_PreprocessorExpression:
-  case CodeCompletionContext::CCC_PreprocessorDirective:
-  case CodeCompletionContext::CCC_NaturalLanguage:
-  case CodeCompletionContext::CCC_SelectorName:
-  case CodeCompletionContext::CCC_TypeQualifiers:
-  case CodeCompletionContext::CCC_ObjCInstanceMessage:
-  case CodeCompletionContext::CCC_ObjCClassMessage:
-  case CodeCompletionContext::CCC_Recovery:
-    return false;
-  }
-  llvm_unreachable("unknown code completion context");
-}
-
 // Should we allow index completions in the specified context?
 bool allowIndex(CodeCompletionContext &CC) {
   if (!contextAllowsIndex(CC.getKind()))

Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=333174&r1=333173&r2=333174&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Thu May 24 04:20:19 2018
@@ -693,6 +693,42 @@ TEST(CompletionTest, BacktrackCrashes) {
 )cpp");
 }
 
+TEST(CompletionTest, CompleteInMacroWithStringification) {
+  auto Results = completions(R"cpp(
+void f(const char *, int x);
+#define F(x) f(#x, x)
+
+namespace ns {
+int X;
+int Y;
+}  // namespace ns
+
+int f(int input_num) {
+  F(ns::^)
+}
+)cpp");
+
+  EXPECT_THAT(Results.items,
+              UnorderedElementsAre(Named("X"), Named("Y")));
+}
+
+TEST(CompletionTest, CompleteInMacroAndNamespaceWithStringification) {
+  auto Results = completions(R"cpp(
+void f(const char *, int x);
+#define F(x) f(#x, x)
+
+namespace ns {
+int X;
+
+int f(int input_num) {
+  F(^)
+}
+}  // namespace ns
+)cpp");
+
+  EXPECT_THAT(Results.items, Contains(Named("X")));
+}
+
 TEST(CompletionTest, CompleteInExcludedPPBranch) {
   auto Results = completions(R"cpp(
     int bar(int param_in_bar) {




More information about the cfe-commits mailing list