[PATCH] Introducing new tool clang-replace
Manuel Klimek
klimek at google.com
Fri Aug 16 09:23:18 PDT 2013
Yes! This is much more like it. I especially like that you used DiagnosticsEngine.
================
Comment at: clang-replace/ApplyReplacements.h:35-36
@@ +34,4 @@
+/// \li false If there were conflicts or some other error occurred.
+bool applyChangeDescriptions(const llvm::StringRef Directory,
+ clang::DiagnosticsEngine &Diagnostics);
+
----------------
I think this function does too much unrelated things.
I'd expect this split up into multiple parts:
- a function that finds all replacement files in a directory, slurps them in, and returns a data structure with all replacements for all files
- a function that takes such a data structure and applies the replacements
Depending on the infrastructure you use, you might have very different ways to find those replacements - having a public interface that requires you to put them into files on a disk seems very restrictive.
================
Comment at: clang-replace/ApplyReplacements.cpp:42-44
@@ +41,5 @@
+/// directory structure.
+static error_code collectReplacementsDocs(const StringRef Directory,
+ ReplacementsDocs &ChangeDocs,
+ DiagnosticsEngine &Diagnostics) {
+ using namespace llvm::sys::fs;
----------------
clang-format :D
Also, this is probably what I'd just put into the public interface instead of hiding it down here, where nobody can observe its beauty ;)
http://llvm-reviews.chandlerc.com/D1424
More information about the cfe-commits
mailing list