[clang-rename] cleanup `auto` usages

[omtcyfz]

Summary

As Alexander pointed out, LLVM Coding Standards are more conservative about using auto, i.e. it should be used in the following situations:

• When the type is obvious, i.e. explicitly mentioned in the same expression. For example if (const clang::FieldDecl *FieldDecl = Initializer->getMember()).
• When the type is totally non-obvious and one iterates over something. For example for (const auto &CurrDecl : Context.getTranslationUnitDecl()->decls()).

Otherwise the type should be explicitly stated.

[alexfh]

A couple of comments inline.

clang-rename/RenamingAction.cpp

73 ↗

(On Diff #67666)

Just remove the = tooling::Replacement part.

clang-rename/USRFinder.cpp

78 ↗

(On Diff #67666)

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?

clang-rename/USRLocFinder.cpp

130 ↗

(On Diff #67666)

Same problem with multiple values per declaration here.

[omtcyfz]

Address comments.

[alexfh]

LG with a couple of nits.

clang-rename/USRFinder.cpp

182 ↗

(On Diff #67989)

Should this be const auto * instead?

clang-rename/USRLocFinder.cpp

132 ↗

(On Diff #67989)

I suspect, clang-format will join the arguments to one line. Can you try? (maybe just git clang-format)

[omtcyfz]

Address comments.

[omtcyfz]

clang-rename/USRFinder.cpp

182–184 ↗

(On Diff #68104)

Aww, sure. Thanks!

clang-tools-extra/trunk/clang-rename/RenamingAction.cpp

void HandleTranslationUnit(ASTContext &Context) override {
     for (unsigned I = 0; I < NewNames.size(); ++I) {
       HandleOneRename(Context, NewNames[I], PrevNames[I], USRList[I]);
     }
   }
 
   void HandleOneRename(ASTContext &Context, const std::string &NewName,
                        const std::string &PrevName,
                        const std::vector<std::string> &USRs) {
-    const auto &SourceMgr = Context.getSourceManager();
+    const SourceManager &SourceMgr = Context.getSourceManager();
     std::vector<SourceLocation> RenamingCandidates;
     std::vector<SourceLocation> NewCandidates;
 
     NewCandidates =
         getLocationsOfUSRs(USRs, PrevName, Context.getTranslationUnitDecl());
     RenamingCandidates.insert(RenamingCandidates.end(), NewCandidates.begin(),
                               NewCandidates.end());
 
-    auto PrevNameLen = PrevName.length();
+    unsigned PrevNameLen = PrevName.length();
     for (const auto &Loc : RenamingCandidates) {
       if (PrintLocations) {
         FullSourceLoc FullLoc(Loc, SourceMgr);
         errs() << "clang-rename: renamed at: " << SourceMgr.getFilename(Loc)
                << ":" << FullLoc.getSpellingLineNumber() << ":"
                << FullLoc.getSpellingColumnNumber() << "\n";
       }
       // FIXME: better error handling.
-      auto Replace = tooling::Replacement(SourceMgr, Loc, PrevNameLen, NewName);
-      auto Err = FileToReplaces[Replace.getFilePath()].add(Replace);
+      tooling::Replacement Replace(SourceMgr, Loc, PrevNameLen, NewName);
+      llvm::Error Err = FileToReplaces[Replace.getFilePath()].add(Replace);
       if (Err)
         llvm::errs() << "Renaming failed in " << Replace.getFilePath() << "! "
                      << llvm::toString(std::move(Err)) << "\n";
     }
   }
 
 private:
   const std::vector<std::string> &NewNames, &PrevNames;

clang-tools-extra/trunk/clang-rename/USRFinder.cpp

return dyn_cast<CXXConversionDecl>(Decl)
                ? true
                : setResult(Decl, Decl->getLocation(),
                            Decl->getNameAsString().length());
   }
 
   // Expression visitors:
 
   bool VisitDeclRefExpr(const DeclRefExpr *Expr) {
-    const auto *Decl = Expr->getFoundDecl();
+    const NamedDecl *Decl = Expr->getFoundDecl();
     return setResult(Decl, Expr->getLocation(),
                      Decl->getNameAsString().length());
   }
 
   bool VisitMemberExpr(const MemberExpr *Expr) {
-    const auto *Decl = Expr->getFoundDecl().getDecl();
+    const NamedDecl *Decl = Expr->getFoundDecl().getDecl();
     return setResult(Decl, Expr->getMemberLoc(),
                      Decl->getNameAsString().length());
   }
 
   // Other visitors:
 
   bool VisitTypeLoc(const TypeLoc Loc) {
-    const auto TypeBeginLoc = Loc.getBeginLoc();
-    const auto TypeEndLoc = Lexer::getLocForEndOfToken(
-        TypeBeginLoc, 0, Context.getSourceManager(), Context.getLangOpts());
+    const SourceLocation TypeBeginLoc = Loc.getBeginLoc();
+    const SourceLocation TypeEndLoc = Lexer::getLocForEndOfToken(
+                             TypeBeginLoc, 0, Context.getSourceManager(),
+                             Context.getLangOpts());
     if (const auto *TemplateTypeParm =
             dyn_cast<TemplateTypeParmType>(Loc.getType())) {
       return setResult(TemplateTypeParm->getDecl(), TypeBeginLoc, TypeEndLoc);
     }
     if (const auto *TemplateSpecType =
             dyn_cast<TemplateSpecializationType>(Loc.getType())) {
       return setResult(TemplateSpecType->getTemplateName().getAsTemplateDecl(),
                        TypeBeginLoc, TypeEndLoc);
public:
   // Other:
 
   const NamedDecl *getNamedDecl() { return Result; }
 
   // \brief Determines if a namespace qualifier contains the point.
   // \returns false on success and sets Result.
   void handleNestedNameSpecifierLoc(NestedNameSpecifierLoc NameLoc) {
     while (NameLoc) {
-      const auto *Decl = NameLoc.getNestedNameSpecifier()->getAsNamespace();
+      const NamespaceDecl *Decl =
+          NameLoc.getNestedNameSpecifier()->getAsNamespace();
       setResult(Decl, NameLoc.getLocalBeginLoc(), NameLoc.getLocalEndLoc());
       NameLoc = NameLoc.getPrefix();
     }
   }
 
 private:
   // \brief Sets Result to Decl if the Point is within Start and End.
   // \returns false on success.
private:
   const SourceLocation Point; // The location to find the NamedDecl.
   const std::string Name;
   const ASTContext &Context;
 };
 } // namespace
 
 const NamedDecl *getNamedDeclAt(const ASTContext &Context,
                                 const SourceLocation Point) {
-  const auto SearchFile = Context.getSourceManager().getFilename(Point);
+  StringRef SearchFile = Context.getSourceManager().getFilename(Point);
   NamedDeclFindingASTVisitor Visitor(Point, Context);
 
   // We only want to search the decls that exist in the same file as the point.
-  auto Decls = Context.getTranslationUnitDecl()->decls();
-  for (auto &CurrDecl : Decls) {
-    const auto FileLoc = CurrDecl->getLocStart();
-    const auto FileName = Context.getSourceManager().getFilename(FileLoc);
+  for (const auto *CurrDecl : Context.getTranslationUnitDecl()->decls()) {
+    const SourceLocation FileLoc = CurrDecl->getLocStart();
+    StringRef FileName = Context.getSourceManager().getFilename(FileLoc);
     // FIXME: Add test.
     if (FileName == SearchFile) {
       Visitor.TraverseDecl(CurrDecl);
     }
   }
 
   NestedNameSpecifierLocFinder Finder(const_cast<ASTContext &>(Context));
   for (const auto &Location : Finder.getNestedNameSpecifierLocations()) {

clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp

for (const auto *CtorDecl : RecordDecl->ctors()) {
       USRSet.insert(getUSRForDecl(CtorDecl));
     }
     USRSet.insert(getUSRForDecl(RecordDecl->getDestructor()));
     USRSet.insert(getUSRForDecl(RecordDecl));
   }
 
   void addUSRsOfOverridenFunctions(const CXXMethodDecl *MethodDecl) {
     USRSet.insert(getUSRForDecl(MethodDecl));
-    for (auto &OverriddenMethod : MethodDecl->overridden_methods()) {
+    for (const auto &OverriddenMethod : MethodDecl->overridden_methods()) {
       // Recursively visit each OverridenMethod.
       addUSRsOfOverridenFunctions(OverriddenMethod);
     }
   }
 
   bool checkIfOverriddenFunctionAscends(const CXXMethodDecl *MethodDecl) {
-    for (auto &OverriddenMethod : MethodDecl->overridden_methods()) {
+    for (const auto &OverriddenMethod : MethodDecl->overridden_methods()) {
       if (USRSet.find(getUSRForDecl(OverriddenMethod)) != USRSet.end()) {
         return true;
       }
       return checkIfOverriddenFunctionAscends(OverriddenMethod);
     }
     return false;
   }
 
   const Decl *FoundDecl;
   ASTContext &Context;
   std::vector<std::string> *USRs;
   std::set<std::string> USRSet;
   std::vector<const CXXMethodDecl *> OverriddenMethods;
   std::vector<const ClassTemplatePartialSpecializationDecl *> PartialSpecs;
 };
 } // namespace
 
 struct NamedDeclFindingConsumer : public ASTConsumer {
   void HandleTranslationUnit(ASTContext &Context) override {
-    const auto &SourceMgr = Context.getSourceManager();
+    const SourceManager &SourceMgr = Context.getSourceManager();
     // The file we look for the USR in will always be the main source file.
-    const auto Point = SourceMgr.getLocForStartOfFile(SourceMgr.getMainFileID())
-                           .getLocWithOffset(SymbolOffset);
+    const SourceLocation Point =
+        SourceMgr.getLocForStartOfFile(SourceMgr.getMainFileID())
     if (!Point.isValid())
       return;
     const NamedDecl *FoundDecl = nullptr;
     if (OldName.empty()) {
       FoundDecl = getNamedDeclAt(Context, Point);
     } else {
       FoundDecl = getNamedDeclFor(Context, OldName);
     }

clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp

if (USRSet.find(getUSRForDecl(Decl)) != USRSet.end()) {
       checkAndAddLocation(Decl->getLocation());
     }
     return true;
   }
 
   // Expression visitors:
 
   bool VisitDeclRefExpr(const DeclRefExpr *Expr) {
-    const auto *Decl = Expr->getFoundDecl();
+    const NamedDecl *Decl = Expr->getFoundDecl();
 
     if (USRSet.find(getUSRForDecl(Decl)) != USRSet.end()) {
       const SourceManager &Manager = Decl->getASTContext().getSourceManager();
       SourceLocation Location = Manager.getSpellingLoc(Expr->getLocation());
       checkAndAddLocation(Location);
     }
 
     return true;
   }
 
   bool VisitMemberExpr(const MemberExpr *Expr) {
-    const auto *Decl = Expr->getFoundDecl().getDecl();
+    const NamedDecl *Decl = Expr->getFoundDecl().getDecl();
     if (USRSet.find(getUSRForDecl(Decl)) != USRSet.end()) {
       const SourceManager &Manager = Decl->getASTContext().getSourceManager();
       SourceLocation Location = Manager.getSpellingLoc(Expr->getMemberLoc());
       checkAndAddLocation(Location);
     }
     return true;
   }
 
public:
   // locations are erroneous and should be reported!
   const std::vector<clang::SourceLocation> &getLocationsFound() const {
     return LocationsFound;
   }
 
   // Namespace traversal:
   void handleNestedNameSpecifierLoc(NestedNameSpecifierLoc NameLoc) {
     while (NameLoc) {
-      const auto *Decl = NameLoc.getNestedNameSpecifier()->getAsNamespace();
+      const NamespaceDecl *Decl =
+          NameLoc.getNestedNameSpecifier()->getAsNamespace();
       if (Decl && USRSet.find(getUSRForDecl(Decl)) != USRSet.end()) {
         checkAndAddLocation(NameLoc.getLocalBeginLoc());
       }
       NameLoc = NameLoc.getPrefix();
     }
   }
 
 private:
   void checkAndAddLocation(SourceLocation Loc) {
-    const auto BeginLoc = Loc;
-    const auto EndLoc = Lexer::getLocForEndOfToken(
+    const SourceLocation BeginLoc = Loc;
+    const SourceLocation EndLoc = Lexer::getLocForEndOfToken(
         BeginLoc, 0, Context.getSourceManager(), Context.getLangOpts());
     StringRef TokenName =
         Lexer::getSourceText(CharSourceRange::getTokenRange(BeginLoc, EndLoc),
                              Context.getSourceManager(), Context.getLangOpts());
     size_t Offset = TokenName.find(PrevName);
     if (Offset != StringRef::npos) {
       // The token of the source location we find actually has the old
       // name.