For loop style fix

[majnemer]

lib/CodeGen/CGVTables.cpp

190–191

I'd name the variable BB instead because BasicBlock is the name of a class in LLVM.

[Prazek]

lib/CodeGen/CGVTables.cpp

190–191

that's why I don't like this convention :P

[majnemer]

lib/CodeGen/CGVTables.cpp

190–192

We all take issue with some part of the coding style but it is important that we all stick to it.

[Prazek]

lib/CodeGen/CGVTables.cpp

190–192

Yep, I hope my middle aproach with "BBblock" is similiar enough. Seriously name like BB for newbie like me doesn't give me any information more than 2 random letters

[yaron.keren]

BB very frequently is BasicBlock and I is Instruction, typically in loops

for (... &BB : F)
  for (... &I : BB)

grep "&BB :" to see more examples. The variable name should really be BB to follow existing code conventions.

To clarify the type maybe it's better not using auto:

for (const BasicBlock &BB : F)

It there is a coding guide on using auto? I didn't find in the LLVM Programmer’s Manual.

[majnemer]

lib/CodeGen/CGVTables.cpp

853

It would be nicer to use const CXXRecordDecl *RD : DeferredVTables because the type in the collection isn't trivially obvious. Also, the braces for the for-loop are now superfluous.

lib/CodeGen/ItaniumCXXABI.cpp

1046–1047

I'd change the auto to CXXBasePath because the type isn't clear.

1052

Likewise, I'd change this auto to CXXBasePathElement.

[Prazek]

lib/CodeGen/CGVTables.cpp

853

const CXXRecordDecl *const RD.

[majnemer]

lib/CodeGen/CGVTables.cpp

853

I don't think it is typical LLVM style to stick const in that position, looks pretty verbose.

[majnemer]

LGTM

lib/CodeGen/CGVTables.cpp

CodeGenFunction::GenerateVarArgsThunk(llvm::Function *Fn,
   // Adjust "this", if necessary.
   Builder.SetInsertPoint(ThisStore);
   llvm::Value *AdjustedThisPtr =
       CGM.getCXXABI().performThisAdjustment(*this, ThisPtr, Thunk.This);
   ThisStore->setOperand(0, AdjustedThisPtr);
 
   if (!Thunk.Return.isEmpty()) {
     // Fix up the returned value, if necessary.
-    for (llvm::Function::iterator I = Fn->begin(), E = Fn->end(); I != E; I++) {
-      llvm::Instruction *T = I->getTerminator();
+    for (llvm::BasicBlock &BB : *Fn) {
+      llvm::Instruction *T = BB.getTerminator();
       if (isa<llvm::ReturnInst>(T)) {
         RValue RV = RValue::get(T->getOperand(0));
         T->eraseFromParent();
-        Builder.SetInsertPoint(&*I);
+        Builder.SetInsertPoint(&BB);
         RV = PerformReturnAdjustment(*this, ResultType, RV, Thunk);
         Builder.CreateRet(RV.getScalarVal());
         break;
       }
     }
   }
 
   return Fn;

 /// decide whether we should emit them.
 void CodeGenModule::EmitDeferredVTables() {
 #ifndef NDEBUG
   // Remember the size of DeferredVTables, because we're going to assume
   // that this entire operation doesn't modify it.
   size_t savedSize = DeferredVTables.size();
 #endif
 
-  typedef std::vector<const CXXRecordDecl *>::const_iterator const_iterator;
-  for (const_iterator i = DeferredVTables.begin(),
-                      e = DeferredVTables.end(); i != e; ++i) {
-    const CXXRecordDecl *RD = *i;
+  for (const CXXRecordDecl *RD : DeferredVTables)
     if (shouldEmitVTableAtEndOfTranslationUnit(*this, RD))
       VTables.GenerateClassData(RD);
-  }
 
   assert(savedSize == DeferredVTables.size() &&
          "deferred extra v-tables during v-table emission?");
   DeferredVTables.clear();
 }
 
 bool CodeGenModule::IsCFIBlacklistedRecord(const CXXRecordDecl *RD) {
   if (RD->hasAttr<UuidAttr>() &&

lib/CodeGen/ItaniumCXXABI.cpp

static CharUnits computeOffsetHint(ASTContext &Context,
   // return that Src is not a public base of Dst.  Record all inheritance paths.
   if (!Dst->isDerivedFrom(Src, Paths))
     return CharUnits::fromQuantity(-2ULL);
 
   unsigned NumPublicPaths = 0;
   CharUnits Offset;
 
   // Now walk all possible inheritance paths.
-  for (CXXBasePaths::paths_iterator I = Paths.begin(), E = Paths.end(); I != E;
-       ++I) {
-    if (I->Access != AS_public) // Ignore non-public inheritance.
+  for (const CXXBasePath &Path : Paths) {
+    if (Path.Access != AS_public)  // Ignore non-public inheritance.
       continue;
 
     ++NumPublicPaths;
 
-    for (CXXBasePath::iterator J = I->begin(), JE = I->end(); J != JE; ++J) {
+    for (const CXXBasePathElement &PathElement : Path) {
       // If the path contains a virtual base class we can't give any hint.
       // -1: no hint.
-      if (J->Base->isVirtual())
+      if (PathElement.Base->isVirtual())
         return CharUnits::fromQuantity(-1ULL);
 
       if (NumPublicPaths > 1) // Won't use offsets, skip computation.
         continue;
 
       // Accumulate the base class offsets.
-      const ASTRecordLayout &L = Context.getASTRecordLayout(J->Class);
-      Offset += L.getBaseClassOffset(J->Base->getType()->getAsCXXRecordDecl());
+      const ASTRecordLayout &L = Context.getASTRecordLayout(PathElement.Class);
+      Offset += L.getBaseClassOffset(
+          PathElement.Base->getType()->getAsCXXRecordDecl());
     }
   }
 
   // -2: Src is not a public base of Dst.
   if (NumPublicPaths == 0)
     return CharUnits::fromQuantity(-2ULL);
 
   // -3: Src is a multiple public base type but never a virtual base type.