[PATCH] D77837: [llvm][NFC] Style fixes in Inliner.cpp

[David Blaikie via Phabricator via llvm-commits]

dblaikie accepted this revision.
dblaikie added a comment.

Reaffirming acceptance.



================
Comment at: llvm/lib/Transforms/IPO/Inliner.cpp:598
   unsigned FirstCallInSCC = CallSites.size();
-  for (unsigned i = 0; i < FirstCallInSCC; ++i)
-    if (Function *F = CallSites[i].first->getCalledFunction())
+  for (unsigned Index = 0; Index < FirstCallInSCC; ++Index)
+    if (Function *F = CallSites[Index].first->getCalledFunction())
----------------
mtrofin wrote:
> dblaikie wrote:
> > mtrofin wrote:
> > > dblaikie wrote:
> > > > "I" would suffice here (in general I think of shorter variables for shorter scopes) - the LLVM style guide does provide examples with single uppercase (I thought there used to be some special case for lower case index variables - maybe that was removed with the advent of range-based for loops and such, or I'm misremembering): https://llvm.org/docs/CodingStandards.html#turn-predicate-loops-into-predicate-functions
> > > There's a name collision for 'I'.
> > Hmm - I don't see it. There's an `I` at 560, but that scope ends at 590 - if `I` is good enough for that scope, it's good enough for this one?
> > 
> > Also I think it's not uncommon to use `J`, `K` etc for single name index-y variables after `I`, but more common if the loops are tightly coupled together - would be a bit weird to use them when they're further apart.
> You are correct. I am using CLion to do the refactorings, and its naming conflict seems to be overly conservative. There's no conflict, indeed.
Curious limitation/bug there, indeed. I wonder if there's something more powered by the Language Server Protocol and clangd that'd be more accurate in this regard... 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77837/new/

https://reviews.llvm.org/D77837