[PATCH] D45887: [CodeComplete] Fix completion at the end of keywords

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 24 02:04:19 PDT 2018


sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

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



================
Comment at: lib/Lex/Lexer.cpp:1647
     // looking up the identifier in the identifier table.
     IdentifierInfo *II = PP->LookUpIdentifierInfo(Result);
 
----------------
move this down to where it's used - it's ignored for CC


================
Comment at: lib/Lex/Lexer.cpp:1649
 
-    // 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 (isCodeCompletionPoint(CurPtr)) {
       // Return the code-completion token.
----------------
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`.



================
Comment at: test/CodeCompletion/end-of-ident-macro.cpp:3
+#define FUNCTOR
+
+using FUNCTION = int();
----------------
nit: remove this blank line?


================
Comment at: test/CodeCompletion/end-of-ident-macro.cpp:6
+
+FUNC(int) a = 10;
+// ^FUNC(int)
----------------
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... :-)


Repository:
  rC Clang

https://reviews.llvm.org/D45887





More information about the cfe-commits mailing list