[PATCH] D24224: [clang-rename] Merge rename-{ at | all } and optimise USRFindingAction.

[Alexander Kornienko via cfe-commits]

alexfh requested changes to this revision.
This revision now requires changes to proceed.

================
Comment at: clang-rename/USRFindingAction.cpp:157
@@ +156,3 @@
+    if (!Point.isValid()) {
+      ErrorOccurred = true;
+      return false;
----------------
Should we emit a diagnostic in this case as well?

================
Comment at: clang-rename/USRFindingAction.cpp:171
@@ +170,3 @@
+            DiagnosticsEngine::Error, "clang-rename could not find symbol "
+                                      "in %0 at %1:%2 (offset %3).");
+        Engine.Report(CouldNotFindSymbolAt)
----------------
nit: no trailing period needed

================
Comment at: clang-rename/USRFindingAction.cpp:173
@@ +172,3 @@
+        Engine.Report(CouldNotFindSymbolAt)
+            << SourceMgr.getFilename(Point) << FullLoc.getSpellingLineNumber()
+            << FullLoc.getSpellingColumnNumber() << SymbolOffset;
----------------
How about reporting diagnostics at this specific location?

================
Comment at: clang-rename/USRFindingAction.h:40
@@ -36,1 +39,3 @@
+  ArrayRef<std::vector<std::string>> getUSRList() { return USRList; }
+  bool hasErrorOccurred() { return ErrorOccurred; }
 
----------------
I'm not a native speaker, but to me `has` here reads as "possesses", not as a marker of past perfect tense. Maybe just `errorOccurred()`?


https://reviews.llvm.org/D24224