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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits