omtcyfz abandoned this revision.
omtcyfz added a comment.
Abandoning this, because https://reviews.llvm.org/rL282577, which introduces
replacement deduplication, eliminates this issue. Big thanks to Eric!
https://reviews.llvm.org/D24914
___
omtcyfz added inline comments.
Comment at: clang-rename/RenamingAction.cpp:74
@@ +73,3 @@
+ // FIXME: An error might happen here if USRLocFinder finds the symbol
+ // twice or if the symbol happens to be in a header included multiple
+ // times independently. Such
alexshap added inline comments.
Comment at: clang-rename/RenamingAction.cpp:74
@@ +73,3 @@
+ // FIXME: An error might happen here if USRLocFinder finds the symbol
+ // twice or if the symbol happens to be in a header included multiple
+ // times independently. Such
omtcyfz updated this revision to Diff 72616.
omtcyfz added a comment.
Slightly change wording.
https://reviews.llvm.org/D24914
Files:
clang-rename/RenamingAction.cpp
Index: clang-rename/RenamingAction.cpp
===
---
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
alexshap 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
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
alexshap added a subscriber: alexshap.
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
omtcyfz updated this revision to Diff 72604.
omtcyfz marked 2 inline comments as done.
omtcyfz added a comment.
Address two comments from Alex.
https://reviews.llvm.org/D24914
Files:
clang-rename/RenamingAction.cpp
Index: clang-rename/RenamingAction.cpp
alexfh requested changes to this revision.
This revision now requires changes to proceed.
Comment at: clang-rename/RenamingAction.cpp:73
@@ -73,1 +72,3 @@
llvm::Error Err = FileToReplaces[Replace.getFilePath()].add(Replace);
+ // FIXME: As for clang-rename,
omtcyfz added a comment.
@arphaman, thank you for the comments! Improved the wording.
https://reviews.llvm.org/D24914
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
omtcyfz updated this revision to Diff 72511.
omtcyfz marked 2 inline comments as done.
omtcyfz added a comment.
Address two comments.
https://reviews.llvm.org/D24914
Files:
clang-rename/RenamingAction.cpp
Index: clang-rename/RenamingAction.cpp
arphaman added a subscriber: arphaman.
Comment at: clang-rename/RenamingAction.cpp:75
@@ +74,3 @@
+ // has been matched multiple times (which shouldn't happen in reality,
+ // need to fix that) or when it's a specific header. For now, just ingore
+ // there error
omtcyfz created this revision.
omtcyfz added a reviewer: alexfh.
omtcyfz added a subscriber: cfe-commits.
`clang-rename` might sometimes have multiple replacements at one
SourceLocation, both trying to perform the same renaming. While this issue
should be addressed properly at some point, for
14 matches
Mail list logo