[PATCH] Introducing new tool clang-replace

Guillaume Papin guillaume.papin at epitech.eu
Mon Aug 19 13:19:56 PDT 2013


  Just some minor comments from me.


================
Comment at: clang-replace/ApplyReplacements.cpp:89
@@ +88,3 @@
+               unsigned firstConflictIdx, unsigned numConflicting,
+               SourceManager &SM, DiagnosticsEngine &Diagnostics) {
+  FileID FID = SM.translateFile(File);
----------------
DiagnosticsEngine &Diagnostics is redundant with SourceManager::getDiagnostics().


================
Comment at: clang-replace/ApplyReplacements.cpp:133-134
@@ +132,4 @@
+static bool deduplicateAndDetectConflicts(FileToReplacementsMap &Replacements,
+                                          SourceManager &SM,
+                                          DiagnosticsEngine &Diagnostics) {
+  bool conflictsFound = false;
----------------
Same as for reportConflict(), DiagnosticsEngine is already accessible via SourceManager::getDiagnostics().


================
Comment at: clang-replace/ApplyReplacements.cpp:87-88
@@ +86,4 @@
+reportConflict(const FileEntry *File,
+               const FileToReplacementsMap::mapped_type &Replacements,
+               unsigned firstConflictIdx, unsigned numConflicting,
+               SourceManager &SM, DiagnosticsEngine &Diagnostics) {
----------------
I think you can reduce these 3 arguments into a llvm::ArrayRef.



http://llvm-reviews.chandlerc.com/D1424



More information about the cfe-commits mailing list