jdemeule added inline comments.
================ Comment at: clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h:75 +/// \brief Deduplicate, check for conflicts, and convert all Replacements stored +/// in \c TUs to AtomicChange. If conflicts occur, no Replacements are applied. /// ---------------- ioeric wrote: > ` If conflicts occur, no Replacements are applied.` This doesn't seem to be > accurate; non-conflicting replacements are still added. You are perfectly right the description does not match the behavior. I think I will update this part to //"Deduplicate, check for conflicts, and convert all Replacements stored in TUs to a single AtomicChange per file. Conflicting replacement are skipped. However, the order of converting Replacements extracted from TUs and TUDs to AtomicChange is undefined."// And amend `FileChanges` with //"Only non conflicting replacements are kept into FileChanges."// ================ Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:179 + // To keep current clang-apply-replacement behavior, report conflicting + // replacements skip file containing conflicts, all replacements are stored + // into 1 big AtomicChange. ---------------- ioeric wrote: > I think we are only skipping conflicts; non-conflicting replacements are > still added even if the file contains other conflicts? > > This doesn't seem to have caused any problem because the current caller > simply drops all changes if conflict is detected in any file, but we should > still make it clear what the behavior of `mergeAndDeduplicate` is (in the API > doc) e.g. what would `FileChanges` contain if there is conflict in some but > not all files? I will update this comment to: //"To keep current clang-apply-replacement behavior, report conflicting replacements on corresponding file, all replacements are stored into 1 big AtomicChange."// https://reviews.llvm.org/D43764 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
