[PATCH] D15251: [Clang] Use range loops and autos in lib/Serialization/ASTReader.cpp

[Hans Wennborg via cfe-commits]

hans added inline comments.

================
Comment at: lib/Serialization/ASTReader.cpp:3390
@@ -3393,5 +3389,3 @@
     Mod->getExportedModules(Exports);
-    for (SmallVectorImpl<Module *>::iterator
-           I = Exports.begin(), E = Exports.end(); I != E; ++I) {
-      Module *Exported = *I;
+    for (const auto &Exported : Exports) {
       if (Visited.insert(Exported).second)
----------------
  for (Module *Exported : Exports)

would be much easier to read. (And having Exported by a reference to a pointer seems odd.)

================
Comment at: lib/Serialization/ASTReader.cpp:3512
@@ -3517,5 +3511,3 @@
   // Load the AST blocks of all of the modules that we loaded.
-  for (SmallVectorImpl<ImportedModule>::iterator M = Loaded.begin(),
-                                              MEnd = Loaded.end();
-       M != MEnd; ++M) {
-    ModuleFile &F = *M->Mod;
+  for (const auto &M : Loaded) {
+    ModuleFile &F = *M.Mod;
----------------
Using a range-based for loop here is very nice, but again I'm not sure the "auto" is helping.

  for (const auto &ImportedModule : ...

would be easier to read, I think.

================
Comment at: lib/Serialization/ASTReader.cpp:3570
@@ -3577,5 +3569,3 @@
   // committed to these module files.
-  for (SmallVectorImpl<ImportedModule>::iterator M = Loaded.begin(),
-                                              MEnd = Loaded.end();
-       M != MEnd; ++M) {
-    ModuleFile &F = *M->Mod;
+  for (const auto &M : Loaded) {
+    ModuleFile &F = *M.Mod;
----------------
Same thing.

================
Comment at: lib/Serialization/ASTReader.cpp:4306
@@ -4317,3 +4305,3 @@
         case INPUT_FILE:
-          bool Overridden = static_cast<bool>(Record[3]);
+          auto Overridden = static_cast<bool>(Record[3]);
           std::string Filename = Blob;
----------------
I'd say "bool" is a better choice here.

================
Comment at: lib/Serialization/ASTReader.cpp:5104
@@ -5117,3 +5103,3 @@
   SmallVector<DiagnosticsEngine::DiagState *, 32> DiagStates;
-  for (ModuleIterator I = ModuleMgr.begin(), E = ModuleMgr.end(); I != E; ++I) {
-    ModuleFile &F = *(*I);
+  for (const auto I : ModuleMgr) {
+    ModuleFile &F = *I;
----------------
I'd spell out ModuleIterator instead of auto here.

================
Comment at: lib/Serialization/ASTReader.cpp:6860
@@ -6878,5 +6859,3 @@
   llvm::errs() << "\n*** PCH/Modules Loaded:";
-  for (ModuleManager::ModuleConstIterator M = ModuleMgr.begin(), 
-                                       MEnd = ModuleMgr.end();
-       M != MEnd; ++M)
-    (*M)->dump();
+  for (const auto M : ModuleMgr)
+    M->dump();
----------------
Maybe:

  for (const ModuleFile &MF : ModuleMgr)

================
Comment at: lib/Serialization/ASTReader.cpp:6867
@@ -6887,4 +6866,3 @@
 void ASTReader::getMemoryBufferSizes(MemoryBufferSizes &sizes) const {
-  for (ModuleConstIterator I = ModuleMgr.begin(),
-      E = ModuleMgr.end(); I != E; ++I) {
-    if (llvm::MemoryBuffer *buf = (*I)->Buffer.get()) {
+  for (const auto I : ModuleMgr) {
+    if (llvm::MemoryBuffer *buf = I->Buffer.get()) {
----------------
Same here.

================
Comment at: lib/Serialization/ASTReader.cpp:7220
@@ -7240,2 +7219,3 @@
+    auto *D
       = dyn_cast_or_null<TypedefNameDecl>(GetDecl(ExtVectorDecls[I]));
     if (D)
----------------
Does this fit on one line now?

================
Comment at: lib/Serialization/ASTReader.cpp:7854
@@ -7875,1 +7853,3 @@
+  auto AS = static_cast<AccessSpecifier>(Record[Idx++]);
+  auto inheritConstructors = static_cast<bool>(Record[Idx++]);
   TypeSourceInfo *TInfo = GetTypeSourceInfo(F, Record, Idx);
----------------
bool seems better than auto for these (well, not AS I suppose).


Repository:
  rL LLVM

http://reviews.llvm.org/D15251