[PATCH] D24914: [clang-rename] Do not print out error message upon encountering multiple replacements in the same SourceLocation.

[Kirill Bobyrev via cfe-commits]

omtcyfz added inline comments.

================
Comment at: clang-rename/RenamingAction.cpp:74
@@ +73,3 @@
+      // FIXME: As for clang-rename, adding a replacement fails IFF either the
+      // AST node has been matched multiple times (which shouldn't happen in
+      // reality, need to fix that). For now, just ignore the error as it
----------------
alexshap wrote:
> Let's consider the following example:
> src/include/Point.h:
>    struct Point {};
> src/a.cpp: 
>    include <Point.h>
> src/b.cpp:
>    include <Point.h>
> clang-rename -qualified-name Point -new-name Point2 srcs/a.cpp srcs/b.cpp
> Renaming failed in /Users/Alexshap/PlayRename/srcs/./include/Point.h! New replacement:
> /Users/Alexshap/PlayRename/srcs/./include/Point.h: 7:+5:"Point2"
> conflicts with existing replacement:
> /Users/Alexshap/PlayRename/srcs/./include/Point.h: 7:+5:"Point2"
> 
> The thing is that clang-rename is trying to modify the same code twice (as in the example) and the return value (Error) of the method Replacements::add doesn't allow us to distinguish two cases: A. conflict (trying to apply different modifications to the same source code) B. (still conflict, but different) (trying to apply the same modification twice). 
> In the past when Replacements was a typedef on std::set and clang-rename was using insert(...) the case B wasn't an issue. 
> P.S. However (imo) the new (FIXME) comment seems to be a little bit misleading.
But the thing is that it's never the A. case. Unlike other refactorings, clang-rename doesn't introduce any name conflicts, it will only sometimes try to rename something multiple times.


https://reviews.llvm.org/D24914