[PATCH] D116643: [clangd] Don't rename on symbols from system headers.

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.


================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:163
+// By default, we exclude symbols from system headers and protobuf symbols as
+// rename these symbols would change system/generated files which are unlikely
+// to be modified.
----------------
nit: "as renaming these symbols..."


================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:164
+// rename these symbols would change system/generated files which are unlikely
+// to be modified.
 bool isExcluded(const NamedDecl &RenameDecl) {
----------------
nit: "unlikely to be good candidates for modification"

Maybe my wording is bad, too, but what I want to convey is that the problem isn't that they can't be modified but that we don't __want__ them to be modified!


================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:171
+    return true;
+  // FIXME: Remove this std symbol list, as they should be covered by the
+  // above isInSystemHeader check.
----------------
Any reason not to do this right now? (if we keep the comment, then it's probably better as "Remove this check because it is redundant in the presence of isInSystemHeader")


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116643