[Clang] Fix Clang-tidy modernize-use-auto warnings in headers and generated files; other minor cleanups.

[Eugene.Zelenko]

Summary

I checked this patch on my own build on RHEL 6. Regressions were OK.

[Eugene.Zelenko]

Synchronize with current code.

[hans]

Some of these are great improvements, especially using auto for iterators etc.

But I'm not sure we'll want to start changing "Foo *foo = new Foo;" to use auto everywhere. I expect we'd have to do that in a huge number of places?

include/clang/AST/ASTVector.h

385

I'm not sure this one is an improvement.

include/clang/AST/DeclContextInternals.h

90

Same here.

include/clang/AST/DeclTemplate.h

1726

But here it does make it nicer :-)

[aaron.ballman]

include/clang/Analysis/Analyses/Consumed.h

266

I don't know whether this is an improvement or not.

include/clang/Rewrite/Core/Rewriter.h

171

Not that this matters in the cases in this code, but const iterator is not the same as const_iterator. I think these changes are a step forward in terms of readability, but a step backwards in terms of the type system.

tools/libclang/Indexing.cpp

672

The extra parens do not add any value here.

utils/TableGen/ClangAttrEmitter.cpp

959

I really like all of these changes to this file!

[Eugene.Zelenko]

include/clang/AST/ASTVector.h

385

At least handling of new will be consistent. This will also silent Clang-tidy warning.

include/clang/Analysis/Analyses/Consumed.h

266

This was changed for consistency with other code. I also think that it's good idea to place one statement per line.

include/clang/Rewrite/Core/Rewriter.h

171

I'm still learning C++11, so advices how to handle such situations properly are welcome!

tools/libclang/Indexing.cpp

672

Will remove them.

[aaron.ballman]

include/clang/AST/ASTVector.h

385

I'm not too concerned about the clang-tidy warning. It suggests that perhaps this checker could use an option to control this behavior, however.

I agree with Hans that a lot of the treatments of new expressions don't really seem to be improvements.

include/clang/AST/DeclTemplate.h

1726

I wonder if the heuristic is: when the new expression spans multiple lines, only replace the declaration type with auto if it causes the new expression to become a single-line expression.

include/clang/Analysis/Analyses/Consumed.h

266

This pattern is used in many places in the code base, it reduces vertical clutter, and it could be argued that it is easier to read because you don't have to interpret the namespace hierarchy.

include/clang/Rewrite/Core/Rewriter.h

171

In this case, I suppose it doesn't matter at all -- RewriteBuffers is a member variable being accessed from a const method, and so it will return a const_iterator. So technically, you don't even need the const here, just auto I.

The issue is more with code like:

struct S {
  SomeContainer C;

  void f() {
    SomeContainer::const_iterator I = C.begin();
  }
};

Replacing with auto here will deduce SomeContainer::iterator. The only way around that is with ugly casting, at which point I think leaving the declaration alone is the better choice.

For this code, I would remove the const and just leave it as auto I.

include/clang/AST/ASTVector.h

 #include <memory>
 
 namespace clang {
-  class ASTContext;
 
+class ASTContext;
+
 template<typename T>
 class ASTVector {
 private:

     NewCapacity = MinSize;
 
   // Allocate the memory from the ASTContext.
-  T *NewElts = new (C, llvm::alignOf<T>()) T[NewCapacity];
+  auto *NewElts = new (C, llvm::alignOf<T>()) T[NewCapacity];
 
   // Copy the elements over.
   if (Begin != End) {

   Capacity.setPointer(Begin+NewCapacity);
 }
 
-} // end: clang namespace
-#endif
+} // end namespace clang
+
+#endif // LLVM_CLANG_AST_ASTVECTOR_H

include/clang/AST/DeclContextInternals.h

 //  of DeclContext.
 //
 //===----------------------------------------------------------------------===//
+
 #ifndef LLVM_CLANG_AST_DECLCONTEXTINTERNALS_H
 #define LLVM_CLANG_AST_DECLCONTEXTINTERNALS_H
 

     if (DeclsTy *Vec = getAsVector())
       Data = DeclsAndHasExternalTy(Vec, true);
     else {
-      DeclsTy *VT = new DeclsTy();
+      auto *VT = new DeclsTy();
       if (NamedDecl *OldD = getAsDecl())
         VT->push_back(OldD);
       Data = DeclsAndHasExternalTy(VT, true);

     // If this is the second decl added to the list, convert this to vector
     // form.
     if (NamedDecl *OldD = getAsDecl()) {
-      DeclsTy *VT = new DeclsTy();
+      auto *VT = new DeclsTy();
       VT->push_back(OldD);
       Data = DeclsAndHasExternalTy(VT, false);
     }

 
 } // end namespace clang
 
-#endif
+#endif // LLVM_CLANG_AST_DECLCONTEXTINTERNALS_H

include/clang/AST/DeclTemplate.h

                           const TemplateArgumentList *TemplateArgs) {
     assert(!SpecializedTemplate.is<SpecializedPartialSpecialization*>() &&
            "Already set to a class template partial specialization!");
-    SpecializedPartialSpecialization *PS
-      = new (getASTContext()) SpecializedPartialSpecialization();
+    auto *PS = new (getASTContext()) SpecializedPartialSpecialization();
     PS->PartialSpecialization = PartialSpec;
     PS->TemplateArgs = TemplateArgs;
     SpecializedTemplate = PS;

     return static_cast<TypeAliasDecl*>(TemplatedDecl);
   }
 
-
   TypeAliasTemplateDecl *getCanonicalDecl() override {
     return cast<TypeAliasTemplateDecl>(
              RedeclarableTemplateDecl::getCanonicalDecl());

              RedeclarableTemplateDecl::getInstantiatedFromMemberTemplate());
   }
 
-
   /// \brief Create a function template node.
   static TypeAliasTemplateDecl *Create(ASTContext &C, DeclContext *DC,
                                        SourceLocation L,

                           const TemplateArgumentList *TemplateArgs) {
     assert(!SpecializedTemplate.is<SpecializedPartialSpecialization *>() &&
            "Already set to a variable template partial specialization!");
-    SpecializedPartialSpecialization *PS =
-        new (getASTContext()) SpecializedPartialSpecialization();
+    auto *PS = new (getASTContext()) SpecializedPartialSpecialization();
     PS->PartialSpecialization = PartialSpec;
     PS->TemplateArgs = TemplateArgs;
     SpecializedTemplate = PS;

   friend class ASTDeclWriter;
 };
 
-} /* end of namespace clang */
+} // end namespace clang
 
-#endif
+#endif // LLVM_CLANG_AST_DECLTEMPLATE_H

include/clang/ASTMatchers/ASTMatchersInternal.h

   /// it cannot be converted to the specified type.
   template <typename T>
   const T *getNodeAs(StringRef ID) const {
-    IDToNodeMap::const_iterator It = NodeMap.find(ID);
-    if (It == NodeMap.end()) {
-      return nullptr;
-    }
-    return It->second.get<T>();
+    const auto It = NodeMap.find(ID);
+    return It == NodeMap.end() ? nullptr : It->second.get<T>();
   }
 
   ast_type_traits::DynTypedNode getNode(StringRef ID) const {
-    IDToNodeMap::const_iterator It = NodeMap.find(ID);
-    if (It == NodeMap.end()) {
-      return ast_type_traits::DynTypedNode();
-    }
-    return It->second;
+    const auto It = NodeMap.find(ID);
+    return It == NodeMap.end() ? ast_type_traits::DynTypedNode() : It->second;
   }
 
   /// \brief Imposes an order on BoundNodesMaps.

   /// BoundNodesTree.
   class Visitor {
   public:
-    virtual ~Visitor() {}
+    virtual ~Visitor() = default;
 
     /// \brief Called multiple times during a single call to VisitMatches(...).
     ///

 template <typename T>
 class MatcherInterface : public DynMatcherInterface {
 public:
-  ~MatcherInterface() override {}
+  ~MatcherInterface() override = default;
 
   /// \brief Returns true if 'Node' can be matched.
   ///

     AMM_ParentOnly
   };
 
-  virtual ~ASTMatchFinder() {}
+  virtual ~ASTMatchFinder() = default;
 
   /// \brief Returns true if the given class is directly or indirectly derived
   /// from a base type matching \c base.

 } // end namespace ast_matchers
 } // end namespace clang
 
-#endif
+#endif // LLVM_CLANG_ASTMATCHERS_ASTMATCHERSINTERNAL_H

include/clang/Analysis/Analyses/Consumed.h

-//===- Consumed.h ----------------------------------------------*- C++ --*-===//
+//===- Consumed.h -----------------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //

     ConsumedBlockInfo(unsigned int NumBlocks, PostOrderCFGView *SortedGraph)
         : StateMapsArray(NumBlocks), VisitOrder(NumBlocks, 0) {
       unsigned int VisitOrderCounter = 0;
-      for (PostOrderCFGView::iterator BI = SortedGraph->begin(),
-           BE = SortedGraph->end(); BI != BE; ++BI) {
-        VisitOrder[(*BI)->getBlockID()] = VisitOrderCounter++;
+      for (const auto &BI : *SortedGraph ) {
+        VisitOrder[BI->getBlockID()] = VisitOrderCounter++;
       }
     }
     

     /// exactly once.
     void run(AnalysisDeclContext &AC);
   };
-}} // end namespace clang::consumed
+} // end namespace consumed
+} // end namespace clang
 
-#endif
+#endif // LLVM_CLANG_ANALYSIS_ANALYSES_CONSUMED_H

include/clang/Rewrite/Core/Rewriter.h

   /// getRewriteBufferFor - Return the rewrite buffer for the specified FileID.
   /// If no modification has been made to it, return null.
   const RewriteBuffer *getRewriteBufferFor(FileID FID) const {
-    std::map<FileID, RewriteBuffer>::const_iterator I =
-      RewriteBuffers.find(FID);
+    const auto I = RewriteBuffers.find(FID);
     return I == RewriteBuffers.end() ? nullptr : &I->second;
   }
 

 
 } // end namespace clang
 
-#endif
+#endif // LLVM_CLANG_REWRITE_CORE_REWRITER_H

include/clang/Sema/Template.h

 //  This file provides types used in the semantic analysis of C++ templates.
 //
 //===----------------------------------------------------------------------===/
+
 #ifndef LLVM_CLANG_SEMA_TEMPLATE_H
 #define LLVM_CLANG_SEMA_TEMPLATE_H
 

       // will overwrite it on construction
       LocalInstantiationScope *oldScope = SemaRef.CurrentInstantiationScope;
 
-      LocalInstantiationScope *newScope =
+      auto *newScope =
         new LocalInstantiationScope(SemaRef, CombineWithOuterScope);
 
       newScope->Outer = nullptr;

           Stored = I->second.get<Decl *>();
         } else {
           DeclArgumentPack *OldPack = I->second.get<DeclArgumentPack *>();
-          DeclArgumentPack *NewPack = new DeclArgumentPack(*OldPack);
+          auto *NewPack = new DeclArgumentPack(*OldPack);
           Stored = NewPack;
           newScope->ArgumentPacks.push_back(NewPack);
         }

         VarTemplatePartialSpecializationDecl *PartialSpec);
     void InstantiateEnumDefinition(EnumDecl *Enum, EnumDecl *Pattern);
   };  
-}
+} // end namespace clang
 
 #endif // LLVM_CLANG_SEMA_TEMPLATE_H

include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h

 #include <vector>
 
 namespace clang {
+
 class ConditionalOperator;
 class AnalysisDeclContext;
 class BinaryOperator;

     PDFileEntry::ConsumerFiles *getFiles(const PathDiagnostic &PD);
   };
 
-private:
-  virtual void anchor();
-public:
   PathDiagnosticConsumer() : flushed(false) {}
   virtual ~PathDiagnosticConsumer();
 

 protected:
   bool flushed;
   llvm::FoldingSet<PathDiagnostic> Diags;
+
+private:
+  virtual void anchor();
 };
 
 //===----------------------------------------------------------------------===//

 class PathDiagnosticLocationPair {
 private:
   PathDiagnosticLocation Start, End;
+
 public:
   PathDiagnosticLocationPair(const PathDiagnosticLocation &start,
                              const PathDiagnosticLocation &end)

   virtual void dump() const = 0;
 };
   
-  
 class PathPieces : public std::list<IntrusiveRefCntPtr<PathDiagnosticPiece> > {
   void flattenTo(PathPieces &Primary, PathPieces &Current,
                  bool ShouldFlattenMacros) const;

 class PathDiagnosticSpotPiece : public PathDiagnosticPiece {
 private:
   PathDiagnosticLocation Pos;
+
 public:
   PathDiagnosticSpotPiece(const PathDiagnosticLocation &pos,
                           StringRef s,

 
 public:
   StackHintGeneratorForSymbol(SymbolRef S, StringRef M) : Sym(S), Msg(M) {}
-  ~StackHintGeneratorForSymbol() override {}
+  ~StackHintGeneratorForSymbol() override = default;
 
   /// \brief Search the call expression for the symbol Sym and dispatch the
   /// 'getMessageForX()' methods to construct a specific message.

   void flattenLocations() override {
     callEnter.flatten();
     callReturn.flatten();
-    for (PathPieces::iterator I = path.begin(), 
-         E = path.end(); I != E; ++I) (*I)->flattenLocations();
+    for (auto &I : path)
+      I->flattenLocations();
   }
   
   static PathDiagnosticCallPiece *construct(const ExplodedNode *N,

   iterator end()   { return LPairs.end(); }
 
   void flattenLocations() override {
-    for (iterator I=begin(), E=end(); I!=E; ++I) I->flatten();
+    for (auto &I : *this)
+      I.flatten();
   }
 
   typedef std::vector<PathDiagnosticLocationPair>::const_iterator

 
   void flattenLocations() override {
     PathDiagnosticSpotPiece::flattenLocations();
-    for (PathPieces::iterator I = subPieces.begin(), 
-         E = subPieces.end(); I != E; ++I) (*I)->flattenLocations();
+    for (auto &I : subPieces)
+      I->flattenLocations();
   }
 
   static inline bool classof(const PathDiagnosticPiece *P) {

   const Decl *UniqueingDecl;
 
   PathDiagnostic() = delete;
+
 public:
   PathDiagnostic(StringRef CheckName, const Decl *DeclWithIssue,
                  StringRef bugtype, StringRef verboseDesc, StringRef shortDesc,

 
   void flattenLocations() {
     Loc.flatten();
-    for (PathPieces::iterator I = pathImpl.begin(), E = pathImpl.end(); 
-         I != E; ++I) (*I)->flattenLocations();
+    for (auto &I : pathImpl)
+      I->flattenLocations();
   }
 
   /// Profiles the diagnostic, independent of the path it references.

   void FullProfile(llvm::FoldingSetNodeID &ID) const;
 };  
 
-} // end GR namespace
+} // end namespace ento
+} // end namespace clang
 
-} //end clang namespace
-
-#endif
+#endif // LLVM_CLANG_STATICANALYZER_CORE_BUGREPORTER_PATHDIAGNOSTIC_H

include/clang/StaticAnalyzer/Core/CheckerManager.h

     if (ref)
       return static_cast<CHECKER *>(ref); // already registered.
 
-    CHECKER *checker = new CHECKER();
+    auto *checker = new CHECKER();
     checker->Name = CurrentCheckName;
     CheckerDtors.push_back(CheckerDtor(checker, destruct<CHECKER>));
     CHECKER::_register(checker, *this);

     if (ref)
       return static_cast<CHECKER *>(ref); // already registered.
 
-    CHECKER *checker = new CHECKER(AOpts);
+    auto *checker = new CHECKER(AOpts);
     checker->Name = CurrentCheckName;
     CheckerDtors.push_back(CheckerDtor(checker, destruct<CHECKER>));
     CHECKER::_register(checker, *this);

                               Eng);
   }
 
-
   /// \brief Run checkers for visiting obj-c messages.
   void runCheckersForObjCMessage(ObjCMessageVisitKind visitKind,
                                  ExplodedNodeSet &Dst,

 
 } // end clang namespace
 
-#endif
+#endif // LLVM_CLANG_STATICANALYZER_CORE_CHECKERMANAGER_H

tools/libclang/Indexing.cpp

 
 typedef llvm::DenseSet<PPRegion> PPRegionSetTy;
 
-} // end anonymous namespace
+} // anonymous namespace
 
 namespace llvm {
   template <> struct isPodLike<PPRegion> {

 static bool topLevelDeclVisitor(void *context, const Decl *D) {
   IndexingContext &IdxCtx = *static_cast<IndexingContext*>(context);
   IdxCtx.indexTopLevelDecl(D);
-  if (IdxCtx.shouldAbort())
-    return false;
-  return true;
+  return (!IdxCtx.shouldAbort());
 }
 
 static void indexTranslationUnit(ASTUnit &Unit, IndexingContext &IdxCtx) {

 }
 
 } // end: extern "C"
-

utils/TableGen/ClangAttrEmitter.cpp

     }
 
     void writeTemplateInstantiation(raw_ostream &OS) const override {
-      OS << "      " << getType() << " *tempInst" << getUpperName()
+      OS << "      auto *tempInst" << getUpperName()
          << " = new (C, 16) " << getType()
          << "[A->" << getLowerName() << "_size()];\n";
       OS << "      {\n";

       }
       OS << ", SourceRange Loc = SourceRange()";
       OS << ") {\n";
-      OS << "    " << R.getName() << "Attr *A = new (Ctx) " << R.getName();
+      OS << "    auto *A = new (Ctx) " << R.getName();
       OS << "Attr(Loc, Ctx, ";
       for (auto const &ai : Args) {
         if (ai->isFake() && !emitFake) continue;

     OS << "  case attr::" << R.getName() << ": {\n";
     Args = R.getValueAsListOfDefs("Args");
     if (R.isSubClassOf(InhClass) || !Args.empty())
-      OS << "    const " << R.getName() << "Attr *SA = cast<" << R.getName()
-         << "Attr>(A);\n";
+      OS << "    const auto *SA = cast<" << R.getName() << "Attr>(A);\n";
     if (R.isSubClassOf(InhClass))
       OS << "    Record.push_back(SA->isInherited());\n";
     OS << "    Record.push_back(A->isImplicit());\n";

       continue;
     }
 
-    OS << "      const " << R.getName() << "Attr *A = cast<"
-       << R.getName() << "Attr>(At);\n";
+    OS << "      const auto *A = cast<" << R.getName() << "Attr>(At);\n";
     bool TDependent = R.getValueAsBit("TemplateDependent");
 
     if (!TDependent) {

 
     Args = R.getValueAsListOfDefs("Args");
     if (!Args.empty()) {
-      OS << "    const " << R.getName() << "Attr *SA = cast<" << R.getName()
-         << "Attr>(A);\n";
+      OS << "    const auto *SA = cast<" << R.getName() << "Attr>(A);\n";
       for (const auto *Arg : Args)
         createArgument(*Arg, R.getName())->writeDump(OS);