[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