Hi Jeffrey,<div><br></div><div>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.</div>
<div><br></div><div>Best regards,</div><div>Victor</div><div><br><div class="gmail_quote">2009/11/3 Jeffrey Yasskin <span dir="ltr"><<a href="mailto:jyasskin@google.com">jyasskin@google.com</a>></span><br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
+template <bool, typename True, typename False><br>
+struct If { typedef True Result; };<br>
+<br>
+template <typename True, typename False><br>
+struct If<false, True, False> { typedef False Result; };<br>
<br>
These should probably go into include/llvm/Support/type_traits.h. In<br>
C++0x, this is named "conditional" (section 20.6.7), so I think you<br>
should use the same name, despite the standard committee's bad taste.<br>
<br>
+  DenseMapIterator(const DenseMapIterator<KeyT, ValueT,<br>
+                                          KeyInfoT, ValueInfoT, false>& I)<br>
<br>
This looks like it will make it impossible to copy const_iterators. I<br>
guess it doesn't because the copy-constructor is auto-generated, but<br>
please comment that and add tests for it and for the non-const->const<br>
conversion to unittests/ADT/DenseMapTest.cpp.<br>
<br>
Otherwise, the patches just change iterator to const_iterator, which looks fine.<br>
<div><div></div><div class="h5"><br>
On Tue, Nov 3, 2009 at 3:09 AM, Victor Zverovich<br>
<<a href="mailto:victor.zverovich@googlemail.com">victor.zverovich@googlemail.com</a>> wrote:<br>
> Dear all,<br>
> The first of the proposed patches (DenseMapIterator.patch) forbids implicit<br>
> conversion of DenseMap::const_iterator to DenseMap::iterator which was<br>
> possible because DenseMapIterator inherited (publicly) from<br>
> DenseMapConstIterator. Conversion the other way around is now allowed as one<br>
> may expect.<br>
> The template DenseMapConstIterator is removed and the template<br>
> parameter IsConst which specifies whether the iterator is constant is added<br>
> to DenseMapIterator.<br>
> Actually IsConst parameter is not necessary since the constness can be<br>
> determined from KeyT but this is not relevant to the fix and can be<br>
> addressed later.<br>
> The second patch (DenseMapIterator-clang.patch) corrects clang's usage of<br>
> DenseMap iterators.<br>
> Best regards,<br>
> Victor<br>
</div></div>> _______________________________________________<br>
> LLVM Developers mailing list<br>
> <a href="mailto:LLVMdev@cs.uiuc.edu">LLVMdev@cs.uiuc.edu</a>         <a href="http://llvm.cs.uiuc.edu" target="_blank">http://llvm.cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br>
><br>
><br>
</blockquote></div><br></div>