For loop style fix

[majnemer]

lib/CodeGen/CGVTables.cpp

190

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

[Prazek]

lib/CodeGen/CGVTables.cpp

190

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

835

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

1026 ↗

(On Diff #30519)

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

1032 ↗

(On Diff #30519)

Likewise, I'd change this auto to CXXBasePathElement.

[Prazek]

lib/CodeGen/CGVTables.cpp

835

const CXXRecordDecl *const RD.

[majnemer]

lib/CodeGen/CGVTables.cpp

835

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 (auto &BasicBlock : *Fn) {
+      llvm::Instruction *T = BasicBlock.getTerminator();
       if (isa<llvm::ReturnInst>(T)) {
         RValue RV = RValue::get(T->getOperand(0));
         T->eraseFromParent();
-        Builder.SetInsertPoint(&*I);
+        Builder.SetInsertPoint(&BasicBlock);
         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 auto &RD : DeferredVTables) {
     if (shouldEmitVTableAtEndOfTranslationUnit(*this, RD))
       VTables.GenerateClassData(RD);
   }
 
   assert(savedSize == DeferredVTables.size() &&
          "deferred extra v-tables during v-table emission?");
   DeferredVTables.clear();
 }