[PATCH] D26288: Deduplicate replacements by FileEntry instead of file names.

Eric Liu via cfe-commits cfe-commits at lists.llvm.org
Sat Nov 5 13:13:32 PDT 2016


ioeric added a comment.

In https://reviews.llvm.org/D26288#587513, @klimek wrote:

> In https://reviews.llvm.org/D26288#586932, @ioeric wrote:
>
> > - Addressed comments: handle non-existing files.
>
>
> We're not really handling them now though? We're just printing an error?
>
> My point is that we might run the replacement generation on a distributed system, and then group/deduplicate/apply them somewhere where the files might not actually exist (think the reduce stage of a mapreduce). If possible, I'd like to not rely on the existence of the file when we deal with Replacements. I'd find it especially problematic if the existence or non-existence of files changes the semantics of those operations.


This function is only used in replacements application phase, and I think this is the most easiest if not the best way to ensure that two sets of replacements for the same file are applied without needing to handle dots and relative/absolute paths.

For distributed replacements generation, I think it makes more sense to duplications on the replacements application phase since this needs to be done when combining all replacements anyway.

It might make more sense to make this function local in tooling/Refactoring.cpp. But in that case, I can't unit test it :(


https://reviews.llvm.org/D26288





More information about the cfe-commits mailing list