From: Jeffrey Yasskin Date: Tue, 10 Nov 2009 01:02:17 +0000 (+0000) Subject: Fix DenseMap iterator constness. X-Git-Url: http://plrg.eecs.uci.edu/git/?a=commitdiff_plain;h=81cf4325698b48b02eddab921ac333c7f25005c3;p=oota-llvm.git Fix DenseMap iterator constness. This patch forbids implicit conversion of DenseMap::const_iterator to DenseMap::iterator which was possible because DenseMapIterator inherited (publicly) from DenseMapConstIterator. Conversion the other way around is now allowed as one may expect. The template DenseMapConstIterator is removed and the template parameter IsConst which specifies whether the iterator is constant is added to DenseMapIterator. Actually IsConst parameter is not necessary since the constness can be determined from KeyT but this is not relevant to the fix and can be addressed later. Patch by Victor Zverovich! git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@86636 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/llvm/ADT/DenseMap.h b/include/llvm/ADT/DenseMap.h index 52354b7f222..83299478e92 100644 --- a/include/llvm/ADT/DenseMap.h +++ b/include/llvm/ADT/DenseMap.h @@ -14,8 +14,9 @@ #ifndef LLVM_ADT_DENSEMAP_H #define LLVM_ADT_DENSEMAP_H -#include "llvm/Support/PointerLikeTypeTraits.h" #include "llvm/Support/MathExtras.h" +#include "llvm/Support/PointerLikeTypeTraits.h" +#include "llvm/Support/type_traits.h" #include "llvm/ADT/DenseMapInfo.h" #include #include @@ -27,12 +28,8 @@ namespace llvm { template, - typename ValueInfoT = DenseMapInfo > + typename ValueInfoT = DenseMapInfo, bool IsConst = false> class DenseMapIterator; -template, - typename ValueInfoT = DenseMapInfo > -class DenseMapConstIterator; template, @@ -73,7 +70,8 @@ public: } typedef DenseMapIterator iterator; - typedef DenseMapConstIterator const_iterator; + typedef DenseMapIterator const_iterator; inline iterator begin() { return iterator(Buckets, Buckets+NumBuckets); } @@ -426,32 +424,47 @@ private: } }; -template -class DenseMapIterator : - public std::iterator, - ptrdiff_t> { - typedef std::pair BucketT; -protected: - const BucketT *Ptr, *End; +template +class DenseMapIterator { + typedef std::pair Bucket; + typedef DenseMapIterator ConstIterator; + friend class DenseMapIterator; +public: + typedef ptrdiff_t difference_type; + typedef typename conditional::type value_type; + typedef value_type *pointer; + typedef value_type &reference; + typedef std::forward_iterator_tag iterator_category; +private: + pointer Ptr, End; public: DenseMapIterator() : Ptr(0), End(0) {} - DenseMapIterator(const BucketT *Pos, const BucketT *E) : Ptr(Pos), End(E) { + DenseMapIterator(pointer Pos, pointer E) : Ptr(Pos), End(E) { AdvancePastEmptyBuckets(); } - std::pair &operator*() const { - return *const_cast(Ptr); + // If IsConst is true this is a converting constructor from iterator to + // const_iterator and the default copy constructor is used. + // Otherwise this is a copy constructor for iterator. + DenseMapIterator(const DenseMapIterator& I) + : Ptr(I.Ptr), End(I.End) {} + + reference operator*() const { + return *Ptr; } - std::pair *operator->() const { - return const_cast(Ptr); + pointer operator->() const { + return Ptr; } - bool operator==(const DenseMapIterator &RHS) const { - return Ptr == RHS.Ptr; + bool operator==(const ConstIterator &RHS) const { + return Ptr == RHS.operator->(); } - bool operator!=(const DenseMapIterator &RHS) const { - return Ptr != RHS.Ptr; + bool operator!=(const ConstIterator &RHS) const { + return Ptr != RHS.operator->(); } inline DenseMapIterator& operator++() { // Preincrement @@ -475,22 +488,6 @@ private: } }; -template -class DenseMapConstIterator : public DenseMapIterator { -public: - DenseMapConstIterator() : DenseMapIterator() {} - DenseMapConstIterator(const std::pair *Pos, - const std::pair *E) - : DenseMapIterator(Pos, E) { - } - const std::pair &operator*() const { - return *this->Ptr; - } - const std::pair *operator->() const { - return this->Ptr; - } -}; - } // end namespace llvm #endif diff --git a/include/llvm/Analysis/SparsePropagation.h b/include/llvm/Analysis/SparsePropagation.h index 820e1bd1e43..677d41d80b2 100644 --- a/include/llvm/Analysis/SparsePropagation.h +++ b/include/llvm/Analysis/SparsePropagation.h @@ -153,7 +153,7 @@ public: /// value. If an value is not in the map, it is returned as untracked, /// unlike the getOrInitValueState method. LatticeVal getLatticeState(Value *V) const { - DenseMap::iterator I = ValueState.find(V); + DenseMap::const_iterator I = ValueState.find(V); return I != ValueState.end() ? I->second : LatticeFunc->getUntrackedVal(); } diff --git a/include/llvm/Support/type_traits.h b/include/llvm/Support/type_traits.h index cfaae4b8419..ce916b5fcc6 100644 --- a/include/llvm/Support/type_traits.h +++ b/include/llvm/Support/type_traits.h @@ -96,6 +96,12 @@ template struct remove_pointer { typedef T type; }; template struct remove_pointer { typedef T type; }; +template +struct conditional { typedef T type; }; + +template +struct conditional { typedef F type; }; + } #endif diff --git a/lib/Analysis/IPA/Andersens.cpp b/lib/Analysis/IPA/Andersens.cpp index 17f304c0211..40a8cd5b23e 100644 --- a/lib/Analysis/IPA/Andersens.cpp +++ b/lib/Analysis/IPA/Andersens.cpp @@ -518,7 +518,7 @@ namespace { /// getObject - Return the node corresponding to the memory object for the /// specified global or allocation instruction. unsigned getObject(Value *V) const { - DenseMap::iterator I = ObjectNodes.find(V); + DenseMap::const_iterator I = ObjectNodes.find(V); assert(I != ObjectNodes.end() && "Value does not have an object in the points-to graph!"); return I->second; @@ -527,7 +527,7 @@ namespace { /// getReturnNode - Return the node representing the return value for the /// specified function. unsigned getReturnNode(Function *F) const { - DenseMap::iterator I = ReturnNodes.find(F); + DenseMap::const_iterator I = ReturnNodes.find(F); assert(I != ReturnNodes.end() && "Function does not return a value!"); return I->second; } @@ -535,7 +535,7 @@ namespace { /// getVarargNode - Return the node representing the variable arguments /// formal for the specified function. unsigned getVarargNode(Function *F) const { - DenseMap::iterator I = VarargNodes.find(F); + DenseMap::const_iterator I = VarargNodes.find(F); assert(I != VarargNodes.end() && "Function does not take var args!"); return I->second; } diff --git a/lib/CodeGen/AsmPrinter/DwarfException.cpp b/lib/CodeGen/AsmPrinter/DwarfException.cpp index 6c03b559b77..8c1c0c98ec4 100644 --- a/lib/CodeGen/AsmPrinter/DwarfException.cpp +++ b/lib/CodeGen/AsmPrinter/DwarfException.cpp @@ -497,7 +497,7 @@ ComputeCallSiteTable(SmallVectorImpl &CallSites, SawPotentiallyThrowing = false; // Beginning of a new try-range? - RangeMapType::iterator L = PadMap.find(BeginLabel); + RangeMapType::const_iterator L = PadMap.find(BeginLabel); if (L == PadMap.end()) // Nope, it was just some random label. continue; diff --git a/lib/CodeGen/PreAllocSplitting.cpp b/lib/CodeGen/PreAllocSplitting.cpp index 374ae667938..e23024c085a 100644 --- a/lib/CodeGen/PreAllocSplitting.cpp +++ b/lib/CodeGen/PreAllocSplitting.cpp @@ -366,10 +366,10 @@ PreAllocSplitting::IsAvailableInStack(MachineBasicBlock *DefMBB, if (!DefMBB) return false; - DenseMap::iterator I = IntervalSSMap.find(Reg); + DenseMap::const_iterator I = IntervalSSMap.find(Reg); if (I == IntervalSSMap.end()) return false; - DenseMap::iterator + DenseMap::const_iterator II = Def2SpillMap.find(DefIndex); if (II == Def2SpillMap.end()) return false; diff --git a/lib/CodeGen/SlotIndexes.cpp b/lib/CodeGen/SlotIndexes.cpp index 9519114b5a3..7365ec1b9a0 100644 --- a/lib/CodeGen/SlotIndexes.cpp +++ b/lib/CodeGen/SlotIndexes.cpp @@ -201,7 +201,7 @@ void SlotIndexes::dump() const { } } - for (MBB2IdxMap::iterator itr = mbb2IdxMap.begin(); + for (MBB2IdxMap::const_iterator itr = mbb2IdxMap.begin(); itr != mbb2IdxMap.end(); ++itr) { errs() << "MBB " << itr->first->getNumber() << " (" << itr->first << ") - [" << itr->second.first << ", " << itr->second.second << "]\n"; diff --git a/lib/Target/MSP430/MSP430ISelDAGToDAG.cpp b/lib/Target/MSP430/MSP430ISelDAGToDAG.cpp index e9e5d34ba51..c0084be9179 100644 --- a/lib/Target/MSP430/MSP430ISelDAGToDAG.cpp +++ b/lib/Target/MSP430/MSP430ISelDAGToDAG.cpp @@ -364,7 +364,7 @@ bool MSP430DAGToDAGISel::IsLegalAndProfitableToFold(SDNode *N, SDNode *U, /// TokenFactor by PreprocessForRMW. Query the map Store => Load1 (created /// during preprocessing) to determine whether it's legal to introduce such /// "cycle" for a moment. - DenseMap::iterator I = RMWStores.find(Root); + DenseMap::const_iterator I = RMWStores.find(Root); if (I != RMWStores.end() && I->second == N) return true; diff --git a/lib/Target/X86/X86InstrInfo.cpp b/lib/Target/X86/X86InstrInfo.cpp index 87bc10d9fe0..c7d9aaa9d54 100644 --- a/lib/Target/X86/X86InstrInfo.cpp +++ b/lib/Target/X86/X86InstrInfo.cpp @@ -2170,7 +2170,7 @@ X86InstrInfo::foldMemoryOperandImpl(MachineFunction &MF, // If table selected... if (OpcodeTablePtr) { // Find the Opcode to fuse - DenseMap >::iterator I = + DenseMap >::const_iterator I = OpcodeTablePtr->find((unsigned*)MI->getOpcode()); if (I != OpcodeTablePtr->end()) { unsigned Opcode = I->second.first; @@ -2402,7 +2402,7 @@ bool X86InstrInfo::canFoldMemoryOperand(const MachineInstr *MI, if (OpcodeTablePtr) { // Find the Opcode to fuse - DenseMap >::iterator I = + DenseMap >::const_iterator I = OpcodeTablePtr->find((unsigned*)Opc); if (I != OpcodeTablePtr->end()) return true; @@ -2413,7 +2413,7 @@ bool X86InstrInfo::canFoldMemoryOperand(const MachineInstr *MI, bool X86InstrInfo::unfoldMemoryOperand(MachineFunction &MF, MachineInstr *MI, unsigned Reg, bool UnfoldLoad, bool UnfoldStore, SmallVectorImpl &NewMIs) const { - DenseMap >::iterator I = + DenseMap >::const_iterator I = MemOp2RegOpTable.find((unsigned*)MI->getOpcode()); if (I == MemOp2RegOpTable.end()) return false; @@ -2530,7 +2530,7 @@ X86InstrInfo::unfoldMemoryOperand(SelectionDAG &DAG, SDNode *N, if (!N->isMachineOpcode()) return false; - DenseMap >::iterator I = + DenseMap >::const_iterator I = MemOp2RegOpTable.find((unsigned*)N->getMachineOpcode()); if (I == MemOp2RegOpTable.end()) return false; @@ -2623,7 +2623,7 @@ X86InstrInfo::unfoldMemoryOperand(SelectionDAG &DAG, SDNode *N, unsigned X86InstrInfo::getOpcodeAfterMemoryUnfold(unsigned Opc, bool UnfoldLoad, bool UnfoldStore, unsigned *LoadRegIndex) const { - DenseMap >::iterator I = + DenseMap >::const_iterator I = MemOp2RegOpTable.find((unsigned*)Opc); if (I == MemOp2RegOpTable.end()) return 0; diff --git a/lib/Transforms/Scalar/ABCD.cpp b/lib/Transforms/Scalar/ABCD.cpp index 3ceda0cbec3..e58fa636047 100644 --- a/lib/Transforms/Scalar/ABCD.cpp +++ b/lib/Transforms/Scalar/ABCD.cpp @@ -1049,7 +1049,7 @@ void ABCD::InequalityGraph::printHeader(raw_ostream &OS, Function &F) const { /// Prints the body of the dot file void ABCD::InequalityGraph::printBody(raw_ostream &OS) const { - DenseMap >::iterator begin = + DenseMap >::const_iterator begin = graph.begin(), end = graph.end(); for (; begin != end ; ++begin) { diff --git a/lib/Transforms/Scalar/GVN.cpp b/lib/Transforms/Scalar/GVN.cpp index 0e3f7507da8..30ab8286d24 100644 --- a/lib/Transforms/Scalar/GVN.cpp +++ b/lib/Transforms/Scalar/GVN.cpp @@ -624,7 +624,7 @@ uint32_t ValueTable::lookup_or_add(Value *V) { /// lookup - Returns the value number of the specified value. Fails if /// the value has not yet been numbered. uint32_t ValueTable::lookup(Value *V) const { - DenseMap::iterator VI = valueNumbering.find(V); + DenseMap::const_iterator VI = valueNumbering.find(V); assert(VI != valueNumbering.end() && "Value not numbered?"); return VI->second; } @@ -644,7 +644,7 @@ void ValueTable::erase(Value *V) { /// verifyRemoved - Verify that the value is removed from all internal data /// structures. void ValueTable::verifyRemoved(const Value *V) const { - for (DenseMap::iterator + for (DenseMap::const_iterator I = valueNumbering.begin(), E = valueNumbering.end(); I != E; ++I) { assert(I->first != V && "Inst still occurs in value numbering map!"); } @@ -2011,12 +2011,12 @@ void GVN::verifyRemoved(const Instruction *Inst) const { // Walk through the value number scope to make sure the instruction isn't // ferreted away in it. - for (DenseMap::iterator + for (DenseMap::const_iterator I = localAvail.begin(), E = localAvail.end(); I != E; ++I) { const ValueNumberScope *VNS = I->second; while (VNS) { - for (DenseMap::iterator + for (DenseMap::const_iterator II = VNS->table.begin(), IE = VNS->table.end(); II != IE; ++II) { assert(II->second != Inst && "Inst still in value numbering scope!"); } diff --git a/lib/Transforms/Scalar/SCCVN.cpp b/lib/Transforms/Scalar/SCCVN.cpp index c047fca9938..8dd718ab218 100644 --- a/lib/Transforms/Scalar/SCCVN.cpp +++ b/lib/Transforms/Scalar/SCCVN.cpp @@ -507,7 +507,7 @@ void ValueTable::erase(Value *V) { /// verifyRemoved - Verify that the value is removed from all internal data /// structures. void ValueTable::verifyRemoved(const Value *V) const { - for (DenseMap::iterator + for (DenseMap::const_iterator I = valueNumbering.begin(), E = valueNumbering.end(); I != E; ++I) { assert(I->first != V && "Inst still occurs in value numbering map!"); } diff --git a/lib/VMCore/Metadata.cpp b/lib/VMCore/Metadata.cpp index 4fadfed503b..c84afb2ea09 100644 --- a/lib/VMCore/Metadata.cpp +++ b/lib/VMCore/Metadata.cpp @@ -341,11 +341,11 @@ MDNode *MetadataContextImpl::getMD(unsigned MDKind, const Instruction *Inst) { /// getMDs - Get the metadata attached to an Instruction. void MetadataContextImpl:: getMDs(const Instruction *Inst, SmallVectorImpl &MDs) const { - MDStoreTy::iterator I = MetadataStore.find(Inst); + MDStoreTy::const_iterator I = MetadataStore.find(Inst); if (I == MetadataStore.end()) return; MDs.resize(I->second.size()); - for (MDMapTy::iterator MI = I->second.begin(), ME = I->second.end(); + for (MDMapTy::const_iterator MI = I->second.begin(), ME = I->second.end(); MI != ME; ++MI) // MD kinds are numbered from 1. MDs[MI->first - 1] = std::make_pair(MI->first, MI->second); diff --git a/unittests/ADT/DenseMapTest.cpp b/unittests/ADT/DenseMapTest.cpp index 15a5379fb8d..afac651a6b2 100644 --- a/unittests/ADT/DenseMapTest.cpp +++ b/unittests/ADT/DenseMapTest.cpp @@ -164,4 +164,16 @@ TEST_F(DenseMapTest, IterationTest) { } } +// const_iterator test +TEST_F(DenseMapTest, ConstIteratorTest) { + // Check conversion from iterator to const_iterator. + DenseMap::iterator it = uintMap.begin(); + DenseMap::const_iterator cit(it); + EXPECT_TRUE(it == cit); + + // Check copying of const_iterators. + DenseMap::const_iterator cit2(cit); + EXPECT_TRUE(cit == cit2); +} + }