ioeric added inline comments.
================
Comment at: change-namespace/ChangeNamespace.cpp:125
@@ +124,3 @@
+// applying all existing Replaces first if there is conflict.
+void addOrMergeReplacement(const tooling::Replacement &R,
+ tooling::Replacements *Replaces) {
----------------
alexshap wrote:
> khm, this seems to be a recurring pattern (if i'm not mistaken),
> looks like the interface of tooling::Replacements can be changed/improved
> (although i don't know).
> Another (separate) observation is about duplicates. For example for
> replacements in headers we can get into if(Err) branch pretty frequently
> because the same replacement can be generated multiple times (if that header
> is included into several *.cpp files).
SGTM. An `addOrMerge()` interface for tooling::Replacements should be helpful.
================
Comment at: change-namespace/ChangeNamespace.cpp:481
@@ +480,3 @@
+ continue;
+ }
+ FileToReplacements[FilePath] = *CleanReplacements;
----------------
Added a `FallbackStyle`.
================
Comment at: change-namespace/tool/ClangChangeNamespace.cpp:105
@@ +104,3 @@
+ outs() << "============== " << File << " ==============\n";
+ Rewrite.getEditBuffer(ID).write(llvm::outs());
+ outs() << "\n============================================\n";
----------------
I'd like to keep this for now for better readability. Will change the format in
the future if necessary.
https://reviews.llvm.org/D24183
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
