[llvm] r234963 - Change range-based for-loops to be -Wrange-loop-analysis clean.
Richard Trieu
rtrieu at google.com
Tue Apr 14 21:18:29 PDT 2015
On Tue, Apr 14, 2015 at 8:28 PM, David Blaikie <dblaikie at gmail.com> wrote:
> On Tue, Apr 14, 2015 at 8:21 PM, Richard Trieu <rtrieu at google.com> wrote:
> > On Tue, Apr 14, 2015 at 6:33 PM, David Blaikie <dblaikie at gmail.com>
> wrote:
> >>
> >> On Tue, Apr 14, 2015 at 6:21 PM, Richard Trieu <rtrieu at google.com>
> wrote:
> >> > Author: rtrieu
> >> > Date: Tue Apr 14 20:21:15 2015
> >> > New Revision: 234963
> >> >
> >> > URL: http://llvm.org/viewvc/llvm-project?rev=234963&view=rev
> >> > Log:
> >> > Change range-based for-loops to be -Wrange-loop-analysis clean.
> >> > No functionality change.
> >> >
> >> > Modified:
> >> > llvm/trunk/lib/Analysis/RegionPass.cpp
> >> > llvm/trunk/lib/CodeGen/WinEHPrepare.cpp
> >> > llvm/trunk/lib/MC/MCDwarf.cpp
> >> > llvm/trunk/lib/Object/COFFObjectFile.cpp
> >> > llvm/trunk/lib/Target/AArch64/AArch64CollectLOH.cpp
> >> > llvm/trunk/lib/Transforms/Scalar/StructurizeCFG.cpp
> >> > llvm/trunk/tools/llvm-cxxdump/llvm-cxxdump.cpp
> >> > llvm/trunk/utils/TableGen/CodeGenRegisters.cpp
> >> >
> >> > Modified: llvm/trunk/lib/Analysis/RegionPass.cpp
> >> > URL:
> >> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/RegionPass.cpp?rev=234963&r1=234962&r2=234963&view=diff
> >> >
> >> >
> ==============================================================================
> >> > --- llvm/trunk/lib/Analysis/RegionPass.cpp (original)
> >> > +++ llvm/trunk/lib/Analysis/RegionPass.cpp Tue Apr 14 20:21:15 2015
> >> > @@ -199,7 +199,7 @@ public:
> >> >
> >> > bool runOnRegion(Region *R, RGPassManager &RGM) override {
> >> > Out << Banner;
> >> > - for (const auto &BB : R->blocks()) {
> >> > + for (const auto *BB : R->blocks()) {
> >> > if (BB)
> >> > BB->print(Out);
> >> > else
> >> >
> >> > Modified: llvm/trunk/lib/CodeGen/WinEHPrepare.cpp
> >> > URL:
> >> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/WinEHPrepare.cpp?rev=234963&r1=234962&r2=234963&view=diff
> >> >
> >> >
> ==============================================================================
> >> > --- llvm/trunk/lib/CodeGen/WinEHPrepare.cpp (original)
> >> > +++ llvm/trunk/lib/CodeGen/WinEHPrepare.cpp Tue Apr 14 20:21:15 2015
> >> > @@ -909,7 +909,7 @@ bool WinEHPrepare::outlineHandler(Action
> >> > // save the association of the blocks in LPadTargetBlocks. The
> >> > // return instructions which are created from these branches will
> >> > be
> >> > // replaced after all landing pads have been outlined.
> >> > - for (const auto &MapEntry : VMap) {
> >> > + for (const auto MapEntry : VMap) {
> >>
> >> What's the deal with this change? Is the type small & trivially
> >> copyable & we're suggesting using values instead of references?
> >>
> > VMap does not return a reference, so remove the reference type to
> indicate a
> > copy is always made.
>
> Seems ValueMap uses a proxy (much like std::vector<bool>/bitset/etc).
>
> How much value do you think there is in warning when a reference is
> bound to a value return from operator*? I appreciate that it's useful
> when there's an intentional type difference (though that's tricky -
> are we warning there now? I forget) but if the types match it's sort
> of a benign reference, right?
>
> I think it's mostly benign, as long as the container isn't changed and the
code expects the new value, or the reference is kept around after the
loop. However, this was the whole point of the warning, to divide up the
syntax for range-based for-loops so that every copy is done with a
non-reference type while every reference type will not bind to a
temporary. Exceptions to this rule just weaken the warning.
> >
> >>
> >> > // VMap maps all values and blocks that were just cloned, but
> >> > dead
> >> > // blocks which were pruned will map to nullptr.
> >> > if (!isa<BasicBlock>(MapEntry.first) || MapEntry.second ==
> >> > nullptr)
> >> >
> >> > Modified: llvm/trunk/lib/MC/MCDwarf.cpp
> >> > URL:
> >> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCDwarf.cpp?rev=234963&r1=234962&r2=234963&view=diff
> >> >
> >> >
> ==============================================================================
> >> > --- llvm/trunk/lib/MC/MCDwarf.cpp (original)
> >> > +++ llvm/trunk/lib/MC/MCDwarf.cpp Tue Apr 14 20:21:15 2015
> >> > @@ -803,7 +803,7 @@ static void EmitGenDwarfRanges(MCStreame
> >> >
> >> >
> >> >
> MCOS->SwitchSection(context.getObjectFileInfo()->getDwarfRangesSection());
> >> >
> >> > - for (const auto sec : Sections) {
> >> > + for (const auto &sec : Sections) {
> >> >
> >> > MCSymbol *StartSymbol = sec.second.first;
> >> > MCSymbol *EndSymbol = sec.second.second;
> >> >
> >> > Modified: llvm/trunk/lib/Object/COFFObjectFile.cpp
> >> > URL:
> >> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/COFFObjectFile.cpp?rev=234963&r1=234962&r2=234963&view=diff
> >> >
> >> >
> ==============================================================================
> >> > --- llvm/trunk/lib/Object/COFFObjectFile.cpp (original)
> >> > +++ llvm/trunk/lib/Object/COFFObjectFile.cpp Tue Apr 14 20:21:15 2015
> >> > @@ -262,7 +262,7 @@ std::error_code COFFObjectFile::getSymbo
> >> > }
> >> > const section_iterator SecEnd = section_end();
> >> > uint64_t AfterAddr = UnknownAddressOrSize;
> >> > - for (const symbol_iterator &SymbI : symbols()) {
> >> > + for (const symbol_iterator SymbI : symbols()) {
> >>
> >> Drop const if we're dropping ref?
> >
> > See two lines above where the code uses a const iterator. This change
> > follows the current code.
>
> Fair enough
>
> >>
> >>
> >> > section_iterator SecI = SecEnd;
> >> > if (std::error_code EC = SymbI->getSection(SecI))
> >> > return EC;
> >> >
> >> > Modified: llvm/trunk/lib/Target/AArch64/AArch64CollectLOH.cpp
> >> > URL:
> >> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64CollectLOH.cpp?rev=234963&r1=234962&r2=234963&view=diff
> >> >
> >> >
> ==============================================================================
> >> > --- llvm/trunk/lib/Target/AArch64/AArch64CollectLOH.cpp (original)
> >> > +++ llvm/trunk/lib/Target/AArch64/AArch64CollectLOH.cpp Tue Apr 14
> >> > 20:21:15 2015
> >> > @@ -328,7 +328,7 @@ static void initReachingDef(const Machin
> >> > const uint32_t *PreservedRegs = MO.getRegMask();
> >> >
> >> > // Set generated regs.
> >> > - for (const auto Entry : RegToId) {
> >> > + for (const auto &Entry : RegToId) {
> >> > unsigned Reg = Entry.second;
> >> > // Use the global register ID when querying APIs external
> to
> >> > this
> >> > // pass.
> >> >
> >> > Modified: llvm/trunk/lib/Transforms/Scalar/StructurizeCFG.cpp
> >> > URL:
> >> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/StructurizeCFG.cpp?rev=234963&r1=234962&r2=234963&view=diff
> >> >
> >> >
> ==============================================================================
> >> > --- llvm/trunk/lib/Transforms/Scalar/StructurizeCFG.cpp (original)
> >> > +++ llvm/trunk/lib/Transforms/Scalar/StructurizeCFG.cpp Tue Apr 14
> >> > 20:21:15 2015
> >> > @@ -887,7 +887,7 @@ void StructurizeCFG::createFlow() {
> >> > /// no longer dominate all their uses. Not sure if this is really
> >> > nessasary
> >> > void StructurizeCFG::rebuildSSA() {
> >> > SSAUpdater Updater;
> >> > - for (const auto &BB : ParentRegion->blocks())
> >> > + for (auto *BB : ParentRegion->blocks())
> >> > for (BasicBlock::iterator II = BB->begin(), IE = BB->end();
> >> > II != IE; ++II) {
> >> >
> >> >
> >> > Modified: llvm/trunk/tools/llvm-cxxdump/llvm-cxxdump.cpp
> >> > URL:
> >> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-cxxdump/llvm-cxxdump.cpp?rev=234963&r1=234962&r2=234963&view=diff
> >> >
> >> >
> ==============================================================================
> >> > --- llvm/trunk/tools/llvm-cxxdump/llvm-cxxdump.cpp (original)
> >> > +++ llvm/trunk/tools/llvm-cxxdump/llvm-cxxdump.cpp Tue Apr 14 20:21:15
> >> > 2015
> >> > @@ -355,7 +355,8 @@ static void dumpCXXData(const ObjectFile
> >> > StringRef SymName = VFTableEntry.second;
> >> > outs() << VFTableName << '[' << Offset << "]: " << SymName <<
> '\n';
> >> > }
> >> > - for (const std::pair<StringRef, ArrayRef<little32_t>> &VBTable :
> >> > VBTables) {
> >> > + for (const std::pair<const StringRef, ArrayRef<little32_t>>
> &VBTable
> >> > :
> >>
> >> Can probably use 'const auto &' here.
> >
> > Fixed in r234974.
> >>
> >>
> >> > + VBTables) {
> >> > StringRef VBTableName = VBTable.first;
> >> > uint32_t Idx = 0;
> >> > for (little32_t Offset : VBTable.second) {
> >> > @@ -363,7 +364,8 @@ static void dumpCXXData(const ObjectFile
> >> > Idx += sizeof(Offset);
> >> > }
> >> > }
> >> > - for (const std::pair<StringRef, CompleteObjectLocator> &COLPair :
> >> > COLs) {
> >> > + for (const std::pair<const StringRef, CompleteObjectLocator>
> &COLPair
> >> > :
> >> > + COLs) {
> >> > StringRef COLName = COLPair.first;
> >> > const CompleteObjectLocator &COL = COLPair.second;
> >> > outs() << COLName << "[IsImageRelative]: " << COL.Data[0] <<
> '\n';
> >> > @@ -373,7 +375,8 @@ static void dumpCXXData(const ObjectFile
> >> > outs() << COLName << "[ClassHierarchyDescriptor]: " <<
> >> > COL.Symbols[1]
> >> > << '\n';
> >> > }
> >> > - for (const std::pair<StringRef, ClassHierarchyDescriptor> &CHDPair
> :
> >> > CHDs) {
> >> > + for (const std::pair<const StringRef, ClassHierarchyDescriptor>
> >> > &CHDPair :
> >> > + CHDs) {
> >> > StringRef CHDName = CHDPair.first;
> >> > const ClassHierarchyDescriptor &CHD = CHDPair.second;
> >> > outs() << CHDName << "[AlwaysZero]: " << CHD.Data[0] << '\n';
> >> > @@ -381,14 +384,14 @@ static void dumpCXXData(const ObjectFile
> >> > outs() << CHDName << "[NumClasses]: " << CHD.Data[2] << '\n';
> >> > outs() << CHDName << "[BaseClassArray]: " << CHD.Symbols[0] <<
> >> > '\n';
> >> > }
> >> > - for (const std::pair<std::pair<StringRef, uint64_t>, StringRef>
> >> > &BCAEntry :
> >> > - BCAEntries) {
> >> > + for (const std::pair<const std::pair<StringRef, uint64_t>,
> StringRef>
> >>
> >> And "const auto &" here too, I suspect.
> >>
> >> > + &BCAEntry : BCAEntries) {
> >> > StringRef BCAName = BCAEntry.first.first;
> >> > uint64_t Offset = BCAEntry.first.second;
> >> > StringRef SymName = BCAEntry.second;
> >> > outs() << BCAName << '[' << Offset << "]: " << SymName << '\n';
> >> > }
> >> > - for (const std::pair<StringRef, BaseClassDescriptor> &BCDPair :
> BCDs)
> >> > {
> >> > + for (const std::pair<const StringRef, BaseClassDescriptor>
> &BCDPair :
> >> > BCDs) {
> >>
> >> and here
> >>
> >> > StringRef BCDName = BCDPair.first;
> >> > const BaseClassDescriptor &BCD = BCDPair.second;
> >> > outs() << BCDName << "[TypeDescriptor]: " << BCD.Symbols[0] <<
> >> > '\n';
> >> > @@ -400,7 +403,7 @@ static void dumpCXXData(const ObjectFile
> >> > outs() << BCDName << "[ClassHierarchyDescriptor]: " <<
> >> > BCD.Symbols[1]
> >> > << '\n';
> >> > }
> >> > - for (const std::pair<StringRef, TypeDescriptor> &TDPair : TDs) {
> >> > + for (const std::pair<const StringRef, TypeDescriptor> &TDPair :
> TDs)
> >> > {
> >>
> >> and here
> >>
> >> > StringRef TDName = TDPair.first;
> >> > const TypeDescriptor &TD = TDPair.second;
> >> > outs() << TDName << "[VFPtr]: " << TD.Symbols[0] << '\n';
> >> > @@ -410,7 +413,7 @@ static void dumpCXXData(const ObjectFile
> >> > /*UseHexEscapes=*/true)
> >> > << '\n';
> >> > }
> >> > - for (const std::pair<StringRef, ThrowInfo> &TIPair : TIs) {
> >> > + for (const std::pair<const StringRef, ThrowInfo> &TIPair : TIs) {
> >>
> >> and here
> >>
> >> > StringRef TIName = TIPair.first;
> >> > const ThrowInfo &TI = TIPair.second;
> >> > auto dumpThrowInfoFlag = [&](const char *Name, uint32_t Flag) {
> >> > @@ -429,7 +432,7 @@ static void dumpCXXData(const ObjectFile
> >> > dumpThrowInfoSymbol("ForwardCompat", 8);
> >> > dumpThrowInfoSymbol("CatchableTypeArray", 12);
> >> > }
> >> > - for (const std::pair<StringRef, CatchableTypeArray> &CTAPair :
> CTAs)
> >> > {
> >> > + for (const std::pair<const StringRef, CatchableTypeArray> &CTAPair
> :
> >> > CTAs) {
> >>
> >> And here
> >>
> >> > StringRef CTAName = CTAPair.first;
> >> > const CatchableTypeArray &CTA = CTAPair.second;
> >> >
> >> > @@ -441,7 +444,7 @@ static void dumpCXXData(const ObjectFile
> >> > I != E; ++I)
> >> > outs() << CTAName << '[' << Idx++ << "]: " << I->second <<
> '\n';
> >> > }
> >> > - for (const std::pair<StringRef, CatchableType> &CTPair : CTs) {
> >> > + for (const std::pair<const StringRef, CatchableType> &CTPair :
> CTs) {
> >>
> >> And here
> >>
> >> > StringRef CTName = CTPair.first;
> >> > const CatchableType &CT = CTPair.second;
> >> > auto dumpCatchableTypeFlag = [&](const char *Name, uint32_t
> Flag) {
> >> > @@ -464,14 +467,14 @@ static void dumpCXXData(const ObjectFile
> >> > << "[CopyCtor]: " << (CT.Symbols[1].empty() ? "null" :
> >> > CT.Symbols[1])
> >> > << '\n';
> >> > }
> >> > - for (const std::pair<std::pair<StringRef, uint64_t>, StringRef>
> >> > &VTTPair :
> >> > - VTTEntries) {
> >> > + for (const std::pair<const std::pair<StringRef, uint64_t>,
> StringRef>
> >>
> >> And here
> >>
> >> > + &VTTPair : VTTEntries) {
> >> > StringRef VTTName = VTTPair.first.first;
> >> > uint64_t VTTOffset = VTTPair.first.second;
> >> > StringRef VTTEntry = VTTPair.second;
> >> > outs() << VTTName << '[' << VTTOffset << "]: " << VTTEntry <<
> '\n';
> >> > }
> >> > - for (const std::pair<StringRef, StringRef> &TIPair : TINames) {
> >> > + for (const std::pair<const StringRef, StringRef> &TIPair :
> TINames) {
> >>
> >> And here
> >>
> >> > StringRef TIName = TIPair.first;
> >> > outs() << TIName << ": " << TIPair.second << '\n';
> >> > }
> >> >
> >> > Modified: llvm/trunk/utils/TableGen/CodeGenRegisters.cpp
> >> > URL:
> >> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/CodeGenRegisters.cpp?rev=234963&r1=234962&r2=234963&view=diff
> >> >
> >> >
> ==============================================================================
> >> > --- llvm/trunk/utils/TableGen/CodeGenRegisters.cpp (original)
> >> > +++ llvm/trunk/utils/TableGen/CodeGenRegisters.cpp Tue Apr 14 20:21:15
> >> > 2015
> >> > @@ -1780,7 +1780,7 @@ void CodeGenRegBank::computeRegUnitLaneM
> >> > const CodeGenRegister *SubRegister = S->second;
> >> > unsigned LaneMask = SubRegIndex->LaneMask;
> >> > // Distribute LaneMask to Register Units touched.
> >> > - for (const auto &SUI : SubRegister->getRegUnits()) {
> >> > + for (unsigned SUI : SubRegister->getRegUnits()) {
> >> > bool Found = false;
> >> > unsigned u = 0;
> >> > for (unsigned RU : RegUnits) {
> >> >
> >> >
> >> > _______________________________________________
> >> > llvm-commits mailing list
> >> > llvm-commits at cs.uiuc.edu
> >> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150414/2e435b75/attachment.html>
More information about the llvm-commits
mailing list