sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land.
================ Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:182 + CharSourceRange::getCharRange(Rename.getNameRanges()[0]), NewName))) + return std::move(Err); } ---------------- hokein wrote: > sammccall wrote: > > for actual error handling behavior (if this can actually fail): is this a > > deliberate choice to refuse to perform any of the edits if there are > > conflicts? Is this better than applying some non-conflicting set? > > > > Unlike most error-propagation, this is a nontrivial policy choice and needs > > a comment. > I think if there are conflicts, it is a signal that indicates we have bugs in > the code (either in clangd, or rename library), applying parts of them may be > not valuable (like generating uncompilable code) > > I'd prefer to refuse to perform renaming if we have conflicts, WDYT? That sounds OK to me, can you add a comment? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65936/new/ https://reviews.llvm.org/D65936 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits