[PATCH] D20216: clang-rename: check that the source location we find actually has the old name

[Manuel Klimek via cfe-commits]

klimek added inline comments.

================
Comment at: clang-rename/USRLocFinder.cpp:37
@@ -35,3 +36,3 @@
 public:
-  explicit USRLocFindingASTVisitor(const std::string USR) : USR(USR) {
+  explicit USRLocFindingASTVisitor(const std::string USR, const std::string &PrevName) : USR(USR), PrevName(PrevName) {
   }
----------------
For constructors, we nowadays usually want to take strings by value and then std::move them into the fields.

================
Comment at: clang-rename/USRLocFinder.cpp:75
@@ +74,3 @@
+          StringRef TokenName = Lexer::getSourceText(CharSourceRange::getTokenRange(Range), Context.getSourceManager(), Context.getLangOpts());
+          if (TokenName.startswith(PrevName)) {
+            // The token of the source location we find actually has the old name.
----------------
Why startswith as opposed to ==?

================
Comment at: clang-rename/USRLocFinder.h:30-33
@@ -29,5 +29,6 @@
 // FIXME: make this an AST matcher. Wouldn't that be awesome??? I agree!
 std::vector<SourceLocation> getLocationsOfUSR(const std::string usr,
+                                              const std::string &PrevName,
                                               Decl *decl);
 }
 }
----------------
Now that I see this - is there a reason not to use const std::string& for the usr? Also, should we instead use StringRef?


http://reviews.llvm.org/D20216