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

Reply via email to