[LLVMdev] DenseMap iterator constness fix

[Victor Zverovich]

Hi Jeffrey,

You are right that the generated copy constructor is used for
const_iterator. I have added a comment clarifying this. Also I have added
the tests you suggested and corrected the comparison operators. Please find
attached the updated patches.

Best regards,
Victor

2009/11/3 Jeffrey Yasskin <jyasskin at google.com>

> +template <bool, typename True, typename False>
> +struct If { typedef True Result; };
> +
> +template <typename True, typename False>
> +struct If<false, True, False> { typedef False Result; };
>
> These should probably go into include/llvm/Support/type_traits.h. In
> C++0x, this is named "conditional" (section 20.6.7), so I think you
> should use the same name, despite the standard committee's bad taste.
>
> +  DenseMapIterator(const DenseMapIterator<KeyT, ValueT,
> +                                          KeyInfoT, ValueInfoT, false>& I)
>
> This looks like it will make it impossible to copy const_iterators. I
> guess it doesn't because the copy-constructor is auto-generated, but
> please comment that and add tests for it and for the non-const->const
> conversion to unittests/ADT/DenseMapTest.cpp.
>
> Otherwise, the patches just change iterator to const_iterator, which looks
> fine.
>
> On Tue, Nov 3, 2009 at 3:09 AM, Victor Zverovich
> <victor.zverovich at googlemail.com> wrote:
> > Dear all,
> > The first of the proposed patches (DenseMapIterator.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.
> > The second patch (DenseMapIterator-clang.patch) corrects clang's usage of
> > DenseMap iterators.
> > Best regards,
> > Victor
> > _______________________________________________
> > LLVM Developers mailing list
> > LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20091104/70d963e7/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: DenseMapIterator2.patch
Type: text/x-patch
Size: 15547 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20091104/70d963e7/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: DenseMapIterator-clang2.patch
Type: text/x-patch
Size: 3293 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20091104/70d963e7/attachment-0001.bin>