sammccall requested changes to this revision. sammccall added a comment. This revision now requires changes to proceed.
Postfiltering non-textual references won't work for the index-based rename, as the whole adjust-the-ranges algorithm relies on the index references being a subset of textual matches. More details in https://github.com/clangd/clangd/issues/238 It looks like this code change only affects index-based rename, but the attached test is only for AST-based rename. I thought the plan of record was to make sure the index knew whether the reference was textual or not, and filter them out before calling adjustRenameRanges? ================ Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:798 [[Foo]] foo; + foo.~[[Foo]]; } ---------------- this isn't valid c++ ================ Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:882 + [[Foo]] foo; + FooFoo z; + } ---------------- If you want to be sure of fixing this bug, I'd like to see a test case: - where the index is out of date - where the macro name is unrelated to the symbol name - where the macro usages are interleaved with the non-macro usages - where the macro usages are in the file which is renamed based on the index, not the one where renamed based on ast Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72071/new/ https://reviews.llvm.org/D72071 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
