alexfh added a comment.

A couple of comments inline.


================
Comment at: clang-rename/RenamingAction.cpp:73
@@ -72,3 +72,3 @@
       // FIXME: better error handling.
-      auto Replace = tooling::Replacement(SourceMgr, Loc, PrevNameLen, 
NewName);
-      auto Err = FileToReplaces[Replace.getFilePath()].add(Replace);
+      tooling::Replacement Replace =
+          tooling::Replacement(SourceMgr, Loc, PrevNameLen, NewName);
----------------
Just remove the ` = tooling::Replacement` part.

================
Comment at: clang-rename/USRFinder.cpp:78
@@ +77,3 @@
+    const SourceLocation TypeBeginLoc = Loc.getBeginLoc(),
+                         TypeEndLoc = Lexer::getLocForEndOfToken(
+                             TypeBeginLoc, 0, Context.getSourceManager(),
----------------
Look, when you join these two declarations, the code becomes slightly less 
clear, slightly worse formatted and takes a bit more screen space. And I don't 
think any aspects of the code improve with this change. Seems like a net loss?

================
Comment at: clang-rename/USRLocFinder.cpp:130
@@ -128,4 +129,3 @@
   void checkAndAddLocation(SourceLocation Loc) {
-    const auto BeginLoc = Loc;
-    const auto EndLoc = Lexer::getLocForEndOfToken(
-        BeginLoc, 0, Context.getSourceManager(), Context.getLangOpts());
+    const SourceLocation BeginLoc = Loc,
+                         EndLoc = Lexer::getLocForEndOfToken(
----------------
Same problem with multiple values per declaration here.


https://reviews.llvm.org/D23397



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to