[CodeComplete] Fix completion at the end of keywords

[ilya-biryukov]

Summary

Make completion behave consistently no matter if it is run at the
start, in the middle or at the end of an identifier that happens to
be a keyword or a macro name. Since completion is often ran on
incomplete identifiers, they may turn into keywords by accident.

For example, we should produce same results for all of these
completion points:

// ^ is completion point.
^class
cla^ss
class^

Previously clang produced different results for the last case (as if
the completion point was after a space: class ^).

This change also updates some offsets in tests that (unintentionally?)
relied on the old behavior.

[sammccall]

Nice! I'm a little surprised that this wasn't fixed before, so I'm worried I missing something.
But nobody else has reviewed it, and it really seems right (and the test cases you fixed did seem broken). We can always revert :)

lib/Lex/Lexer.cpp

1647 ↗

(On Diff #143335)

move this down to where it's used - it's ignored for CC

1649 ↗

(On Diff #143335)

maybe this deserves a comment describing exactly this change, something like:

If the completion point is at the end of an identifier, we want to treat it as incomplete even if it
is valid. This allows e.g. class^ to complete to classifier.

test/CodeCompletion/end-of-ident-macro.cpp

3 ↗

(On Diff #143335)

nit: remove this blank line?

6 ↗

(On Diff #143335)

Maybe add a comment above this line:

We should get all three completions when the cursor is at the beginning, middle, or end.

(I know documenting the intent of lit tests seems to not be fashionable, but... :-)

[ilya-biryukov]

• Added comments
• Fix nits in tests

lib/Lex/Lexer.cpp

1647 ↗

(On Diff #143335)

It actually updates Result too!
And things break if we don't call it. Added a comment.

cfe/trunk/lib/Lex/Lexer.cpp

FinishIdentifier:
     // If we are in raw mode, return this identifier raw.  There is no need to
     // look up identifier information or attempt to macro expand it.
     if (LexingRawMode)
       return true;
 
     // Fill in Result.IdentifierInfo and update the token kind,
     // looking up the identifier in the identifier table.
     IdentifierInfo *II = PP->LookUpIdentifierInfo(Result);
+    // Note that we have to call PP->LookUpIdentifierInfo() even for code
+    // completion, it writes IdentifierInfo into Result, and callers rely on it.
 
-    // Finally, now that we know we have an identifier, pass this off to the
-    // preprocessor, which may macro expand it or something.
-    if (II->isHandleIdentifierCase())
-      return PP->HandleIdentifier(Result);
-
-    if (II->getTokenID() == tok::identifier && isCodeCompletionPoint(CurPtr)
-        && II->getPPKeywordID() == tok::pp_not_keyword
-        && II->getObjCKeywordID() == tok::objc_not_keyword) {
+    // If the completion point is at the end of an identifier, we want to treat
+    // the identifier as incomplete even if it resolves to a macro or a keyword.
+    // This allows e.g. 'class^' to complete to 'classifier'.
+    if (isCodeCompletionPoint(CurPtr)) {
       // Return the code-completion token.
       Result.setKind(tok::code_completion);
       cutOffLexing();
       return true;
     }
+
+    // Finally, now that we know we have an identifier, pass this off to the
+    // preprocessor, which may macro expand it or something.
+    if (II->isHandleIdentifierCase())
+      return PP->HandleIdentifier(Result);
+
     return true;
   }
 
   // Otherwise, $,\,? in identifier found.  Enter slower path.
 
   C = getCharAndSize(CurPtr, Size);
   while (true) {
     if (C == '$') {

cfe/trunk/test/CodeCompletion/end-of-ident-macro.cpp

+#define FUNC(X) X
+#define FUNCTOR
+using FUNCTION = int();
+// We should get all three completions when the cursor is at the beginning,
+// middle, or end.
+FUNC(int) a = 10;
+// ^FUNC(int)
+// RUN: %clang_cc1 -code-completion-at=%s:6:1 -code-completion-macros %s | FileCheck %s
+// FU^NC(int)
+// RUN: %clang_cc1 -code-completion-at=%s:6:3 -code-completion-macros %s | FileCheck %s
+// FUNC^(int)
+// RUN: %clang_cc1 -code-completion-at=%s:6:5 -code-completion-macros %s | FileCheck %s
+
+// CHECK: COMPLETION: FUNC : FUNC(<#X#>)
+// CHECK: COMPLETION: FUNCTION : FUNCTION
+// CHECK: COMPLETION: FUNCTOR : FUNCTOR
 No newline at end of file

cfe/trunk/test/CodeCompletion/end-of-ident.cpp

+class classifier {};
+// We should get all three completions when the cursor is at the beginning,
+// middle, or end.
+class cls
+// ^class cls
+// RUN: %clang_cc1 -code-completion-at=%s:4:1 %s | FileCheck --check-prefix=CHECK-CLS %s
+// cl^ass cls
+// RUN: %clang_cc1 -code-completion-at=%s:4:3 %s | FileCheck --check-prefix=CHECK-CLS %s
+// class^ cls
+// RUN: %clang_cc1 -code-completion-at=%s:4:6 %s | FileCheck --check-prefix=CHECK-CLS %s
+
+// CHECK-CLS: COMPLETION: class{{$}}
+// CHECK-CLS: COMPLETION: classifier : classifier
+
+// class ^cls
+// RUN: %clang_cc1 -code-completion-at=%s:4:7 %s | FileCheck --check-prefix=CHECK-NO-CLS %s
+// class c^ls
+// RUN: %clang_cc1 -code-completion-at=%s:4:8 %s | FileCheck --check-prefix=CHECK-NO-CLS %s
+// CHECK-NO-CLS-NOT: COMPLETION: class{{$}}
+// CHECK-NO-CLS: COMPLETION: classifier : classifier

cfe/trunk/test/CodeCompletion/macros.c

 enum Color {
   Red, Green, Blue
 };
 
 struct Point {
   float x, y, z;
   enum Color color;
 };
 
 void test(struct Point *p) {
   // RUN: %clang_cc1 -include %S/Inputs/macros.h -fsyntax-only -code-completion-macros -code-completion-at=%s:12:14 %s -o - | FileCheck -check-prefix=CC1 %s
   switch (p->IDENTITY(color)) {
-  // RUN: %clang_cc1 -include %S/Inputs/macros.h -fsyntax-only -code-completion-macros -code-completion-at=%s:14:9 %s -o - | FileCheck -check-prefix=CC2 %s
+  // RUN: %clang_cc1 -include %S/Inputs/macros.h -fsyntax-only -code-completion-macros -code-completion-at=%s:14:10 %s -o - | FileCheck -check-prefix=CC2 %s
     case 
   }
-  // RUN: %clang_cc1 -include %S/Inputs/macros.h -fsyntax-only -code-completion-macros -code-completion-at=%s:17:7 %s -o - | FileCheck -check-prefix=CC3 %s
+  // RUN: %clang_cc1 -include %S/Inputs/macros.h -fsyntax-only -code-completion-macros -code-completion-at=%s:17:8 %s -o - | FileCheck -check-prefix=CC3 %s
 #ifdef Q
 #endif
 
   // Run the same tests, this time with macros loaded from the PCH file.
   // RUN: %clang_cc1 -emit-pch -o %t %S/Inputs/macros.h
   // RUN: %clang_cc1 -include-pch %t -fsyntax-only -code-completion-macros -code-completion-at=%s:12:14 %s -o - | FileCheck -check-prefix=CC1 %s
-  // RUN: %clang_cc1 -include-pch %t -fsyntax-only -code-completion-macros -code-completion-at=%s:14:9 %s -o - | FileCheck -check-prefix=CC2 %s
-  // RUN: %clang_cc1 -include-pch %t -fsyntax-only -code-completion-macros -code-completion-at=%s:17:7 %s -o - | FileCheck -check-prefix=CC3 %s
+  // RUN: %clang_cc1 -include-pch %t -fsyntax-only -code-completion-macros -code-completion-at=%s:14:10 %s -o - | FileCheck -check-prefix=CC2 %s
+  // RUN: %clang_cc1 -include-pch %t -fsyntax-only -code-completion-macros -code-completion-at=%s:17:8 %s -o - | FileCheck -check-prefix=CC3 %s
 
   // CC1: color
   // CC1: x
   // CC1: y
   // CC1: z
 
   // CC2: BAR(<#X#>, <#Y#>)
   // CC2: Blue

cfe/trunk/test/CodeCompletion/namespace.cpp

 namespace N3 {
 }
 
 namespace N2 {
   namespace I1 { }
   namespace I4 = I1;
   namespace I5 { }
   namespace I1 { }
   
-  namespace
-  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:12 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+  namespace 
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:13 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
   // CHECK-CC1: I1
   // CHECK-CC1-NEXT: I5
   

cfe/trunk/test/CodeCompletion/operator.cpp

 class T { };
 
 typedef int Integer;
 
 namespace N { }
 
 void f() {
   typedef float Float;
   
-  operator
-  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:11 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+  operator 
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:10:12 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
   // CHECK-CC1: +
   // CHECK-CC1: Float
   // CHECK-CC1: Integer
   // CHECK-CC1: N
   // CHECK-CC1: short
   // CHECK-CC1: T

cfe/trunk/test/CodeCompletion/tag.c

 enum X { x };
 enum Y { y };
 struct Z { };
 
 void X();
 
 void test() {
   enum X { x };
-  enum
-  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:9:7 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+  enum 
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:9:8 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
   // CHECK-CC1: X
   // CHECK-CC1: Y

cfe/trunk/test/CodeCompletion/tag.cpp

 class X { };
 struct Y { };
 
 namespace N {
   template<typename> class Z;
 }
 
 namespace M {
   class A;
 }
 using M::A;
 
 namespace N {
   class Y;
   
   void test() {
-    class
-    // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:17:10 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+    class 
+    // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:17:11 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
     // FIXME: the redundant Y is really annoying... it needs qualification to 
     // actually be useful. Here, it just looks redundant :(
     // CHECK-CC1: A
     // CHECK-CC1: M : M::
     // CHECK-CC1: N : N::
     // CHECK-CC1: X
     // CHECK-CC1: Y
     // CHECK-CC1: Y
     // CHECK-CC1: Z

cfe/trunk/test/CodeCompletion/using-namespace.cpp

 namespace N4 {
   namespace N3 { }
 }
 
 class N3;
 
 namespace N2 {
   namespace I1 { }
   namespace I4 = I1;
   namespace I5 { }
   namespace I1 { }
   
   void foo() {
-    using namespace
-    // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:14:20 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+    using namespace 
+    // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:14:21 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
     // CHECK-CC1: I1
     // CHECK-CC1: I4
     // CHECK-CC1: I5
     // CHECK-CC1: N2
     // CHECK-CC1-NEXT: N4

cfe/trunk/test/CodeCompletion/using.cpp

 namespace N4 {
   namespace N3 { }
 }
 
 class N3;
 
 namespace N2 {
   namespace I1 { }
   namespace I4 = I1;
   namespace I5 { }
   namespace I1 { }
   
   void foo() {
     int N3;
     
-    using
-    // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:16:10 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+    using 
+    // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:16:11 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
     // CHECK-CC1: I1
     // CHECK-CC1: I4
     // CHECK-CC1: I5
     // CHECK-CC1: N2
     // CHECK-CC1: N3
     // CHECK-CC1-NEXT: N4
 

cfe/trunk/test/Index/complete-exprs.c

void f4(const char* str) {
   f3(str, NULL);
 }
 
 typedef int type;
 void f5(float f) {
   (type)f;
 }
 
-// RUN: c-index-test -code-completion-at=%s:7:9 -Xclang -code-completion-patterns %s | FileCheck -check-prefix=CHECK-CC1 %s
-// RUN: env CINDEXTEST_EDITING=1 CINDEXTEST_COMPLETION_CACHING=1 c-index-test -code-completion-at=%s:7:9 -Xclang -code-completion-patterns %s | FileCheck -check-prefix=CHECK-CC1 %s
+// RUN: c-index-test -code-completion-at=%s:7:10 -Xclang -code-completion-patterns %s | FileCheck -check-prefix=CHECK-CC1 %s
+// RUN: env CINDEXTEST_EDITING=1 CINDEXTEST_COMPLETION_CACHING=1 c-index-test -code-completion-at=%s:7:10 -Xclang -code-completion-patterns %s | FileCheck -check-prefix=CHECK-CC1 %s
 // CHECK-CC1: NotImplemented:{TypedText __PRETTY_FUNCTION__} (65)
 // CHECK-CC1: macro definition:{TypedText __VERSION__} (70)
 // CHECK-CC1: FunctionDecl:{ResultType int}{TypedText f}{LeftParen (}{Placeholder int}{RightParen )} (12) (unavailable)
 // CHECK-CC1-NOT: NotImplemented:{TypedText float} (65)
 // CHECK-CC1: ParmDecl:{ResultType int}{TypedText j} (8)
 // CHECK-CC1: NotImplemented:{ResultType size_t}{TypedText sizeof}{LeftParen (}{Placeholder expression-or-type}{RightParen )} (40)
-// RUN: env CINDEXTEST_EDITING=1 CINDEXTEST_COMPLETION_CACHING=1 c-index-test -code-completion-at=%s:7:9 -Xclang -code-completion-patterns %s | FileCheck -check-prefix=CHECK-CC1 %s
+// RUN: env CINDEXTEST_EDITING=1 CINDEXTEST_COMPLETION_CACHING=1 c-index-test -code-completion-at=%s:7:10 -Xclang -code-completion-patterns %s | FileCheck -check-prefix=CHECK-CC1 %s
 // RUN: c-index-test -code-completion-at=%s:7:14 -Xclang -code-completion-patterns %s | FileCheck -check-prefix=CHECK-CC3 %s
 // RUN: env CINDEXTEST_EDITING=1 CINDEXTEST_COMPLETION_CACHING=1 c-index-test -code-completion-at=%s:7:14 -Xclang -code-completion-patterns %s | FileCheck -check-prefix=CHECK-CC3 %s
 // CHECK-CC3: macro definition:{TypedText __VERSION__} (70)
 // CHECK-CC3: FunctionDecl:{ResultType int}{TypedText f}{LeftParen (}{Placeholder int}{RightParen )} (50)
 // CHECK-CC3-NOT: NotImplemented:{TypedText float}
 // CHECK-CC3: ParmDecl:{ResultType int}{TypedText j} (34)
 // CHECK-CC3: NotImplemented:{ResultType size_t}{TypedText sizeof}{LeftParen (}{Placeholder expressio
 

cfe/trunk/test/Index/complete-preprocessor.m

 // The line and column layout of this test is significant. Run lines
 // are at the end.
 
 #if 1
 #endif
 
 #define FOO(a, b) a##b
 #define BAR
 #ifdef FOO
 #endif
 #if defined(FOO)
 #endif
 
 FOO(in,t) value;
 
-// RUN: c-index-test -code-completion-at=%s:4:2 %s | FileCheck -check-prefix=CHECK-CC1 %s
+// RUN: c-index-test -code-completion-at=%s:4:3 %s | FileCheck -check-prefix=CHECK-CC1 %s
 // CHECK-CC1: NotImplemented:{TypedText define}{HorizontalSpace  }{Placeholder macro} (40)
 // CHECK-CC1-NEXT: NotImplemented:{TypedText define}{HorizontalSpace  }{Placeholder macro}{LeftParen (}{Placeholder args}{RightParen )} (40)
 // CHECK-CC1-NEXT: NotImplemented:{TypedText error}{HorizontalSpace  }{Placeholder message} (40)
 // CHECK-CC1-NEXT: NotImplemented:{TypedText if}{HorizontalSpace  }{Placeholder condition} (40)
 // CHECK-CC1-NEXT: NotImplemented:{TypedText ifdef}{HorizontalSpace  }{Placeholder macro} (40)
 // CHECK-CC1-NEXT: NotImplemented:{TypedText ifndef}{HorizontalSpace  }{Placeholder macro} (40)
 // CHECK-CC1-NEXT: NotImplemented:{TypedText import}{HorizontalSpace  }{Text "}{Placeholder header}{Text "} (40)
 // CHECK-CC1-NEXT: NotImplemented:{TypedText import}{HorizontalSpace  }{Text <}{Placeholder header}{Text >} (40)

 // CHECK-CC2-NEXT: NotImplemented:{TypedText line}{HorizontalSpace  }{Placeholder number} (40)
 // CHECK-CC2-NEXT: NotImplemented:{TypedText line}{HorizontalSpace  }{Placeholder number}{HorizontalSpace  }{Text "}{Placeholder filename}{Text "} (40)
 // CHECK-CC2-NEXT: NotImplemented:{TypedText pragma}{HorizontalSpace  }{Placeholder arguments} (40)
 // CHECK-CC2-NEXT: NotImplemented:{TypedText undef}{HorizontalSpace  }{Placeholder macro} (40)
 // CHECK-CC2-NEXT: NotImplemented:{TypedText warning}{HorizontalSpace  }{Placeholder message} (40)
 // RUN: c-index-test -code-completion-at=%s:9:8 %s | FileCheck -check-prefix=CHECK-CC3 %s
 // CHECK-CC3: macro definition:{TypedText BAR} (40)
 // CHECK-CC3: macro definition:{TypedText FOO} (40)
-// RUN: c-index-test -code-completion-at=%s:11:12 %s | FileCheck -check-prefix=CHECK-CC3 %s
 // RUN: c-index-test -code-completion-at=%s:11:13 %s | FileCheck -check-prefix=CHECK-CC3 %s
+// RUN: c-index-test -code-completion-at=%s:11:14 %s | FileCheck -check-prefix=CHECK-CC3 %s
 // RUN: c-index-test -code-completion-at=%s:11:5 %s | FileCheck -check-prefix=CHECK-CC4 %s
 // CHECK-CC4: macro definition:{TypedText BAR} (70)
 // CHECK-CC4: macro definition:{TypedText FOO}{LeftParen (}{Placeholder a}{Comma , }{Placeholder b}{RightParen )} (70)
 // RUN: c-index-test -code-completion-at=%s:14:5 %s | FileCheck -check-prefix=CHECK-CC5 %s
 // CHECK-CC5: NotImplemented:{TypedText const} (50)
 // CHECK-CC5: NotImplemented:{TypedText double} (50)
 // CHECK-CC5: NotImplemented:{TypedText enum} (50)
 // CHECK-CC5: NotImplemented:{TypedText extern} (40)