[PATCH] D88875: [clangd] Add basic keyword-name-validation in rename.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:124
+
+  // name validation.
+  RenameToKeywords,
----------------
sammccall wrote:
> nit: not sure we need a separate section for these, I can imagine at most keyword/conflict/shadow
yeah, I would leave it as-is for now. we could factor this out when there are more.


================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:479
     return makeError(ReasonToReject::AmbiguousSymbol);
+  // Empty name is legal -- this allows rename API to return all occurrences
+  // even the name is unknown.
----------------
sammccall wrote:
> Now that prepareRename only exposes ranges rather than edits, do we still need this empty special case, or can we go back to using "clangd_rename_dummy" or so?
> 
> (Sorry for the back and forth here)
oh, you're right. And the documentation of the prepareRename is stale. Updated.

A slight concern of using a dummy name is that it may cause a name validation, but it should rarely happen if we choose some name like `clangd_rename_dummy`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88875