[PATCH] D23397: [clang-rename] cleanup `auto` usages

Alexander Kornienko via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 11 17:07:33 PDT 2016


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





More information about the cfe-commits mailing list