[PATCH] D73610: [clangd] Replace raw lexer code with token buffer in prepare rename.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clang-tools-extra/clangd/ClangdServer.cpp:322
+    if (!TouchingIdentifier)
+      return CB(llvm::None); // no rename on non-identifiers.
+
----------------
sammccall wrote:
> Agree that this behavior is OK per the spec, but do we actually prefer it/why the change?
> 
> Seems like clients are marginally more likely to handle null incorrectly/silently, and we give up control over the message.
> Agree that this behavior is OK per the spec, but do we actually prefer it/why the change?

I checked with VSCode, it seems fine (`the element can't be renamed`), and YCM (but it doesn't support prepare rename yet). Are you suggesting that we return our message here?



================
Comment at: clang-tools-extra/clangd/ClangdServer.cpp:324
+
+    auto Range = getTokenRange(AST.getSourceManager(), AST.getLangOpts(),
+                               TouchingIdentifier->location());
----------------
sammccall wrote:
> this runs the lexer to compute token boundaries. We actually already have them: syntax::Token has endLocation/length
removed it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73610/new/

https://reviews.llvm.org/D73610