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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
