Sorry, always end up not replying to the list: The main issue with dealing with next this way is that people adding new uses of next will probably not be using c++0x and therefore won't know it's ambiguous and that it needs to be qualified. There are also two issues with rvalue references and the STL: 1. EquivalenceClasses, in the insert and findLeader functions, it uses map functions which have versions taking rvalue references. This results in a compiler error because the private constructor of ECValue is inaccessible. The easiest fix is to explicitly call the constructor of ECValue before the insert/find calls. 2. There are numerous places where there is a std::pair of pointer types and passing 0 results in the rvalue reference constructor being called. The type of 0 is deduced to be an int and an int isn't convertable to a pointer, so it fails to compile. The fix is to use nullpte (GCC doesn't seem to have this yet) or to static_cast<T*>(0) where T is the appropriate type. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20091203/91fdda1b/attachment.html>
On Thu, Dec 3, 2009 at 3:24 AM, Ahmed Charles <ahmedcharles at gmail.com> wrote:> Sorry, always end up not replying to the list: > > The main issue with dealing with next this way is that people adding new > uses of next will probably not be using c++0x and therefore won't know it's > ambiguous and that it needs to be qualified.What does llvm::next do that std::next (or boost::next) do not do? If they do the same thing, why not just conditional create llvm::next, if std::next is available then just include it into the llvm namespace so it can be used as they currently are. If they are not compatible, why not fix the iterators to follow the std standards?
On Dec 3, 2009, at 5:24 AM, Ahmed Charles wrote:> Sorry, always end up not replying to the list: > > The main issue with dealing with next this way is that people adding new uses of next will probably not be using c++0x and therefore won't know it's ambiguous and that it needs to be qualified.True. But when this code is compiled under C++0X you get an easy to diagnose, easy to fix, compile-time diagnostic (as opposed to a hard-to-find run time error). On Dec 3, 2009, at 5:32 AM, OvermindDL1 wrote:> What does llvm::next do that std::next (or boost::next) do not do? If > they do the same thing, why not just conditional create llvm::next, if > std::next is available then just include it into the llvm namespace so > it can be used as they currently are. If they are not compatible, why > not fix the iterators to follow the std standards?They all look identical to me. The conditional creation of llvm::next is a viable, yet more complicated solution. Note that this is a larger issue than just next(). The C++0X standard is introducing several new generic free functions in several existing headers: <iterator> next prev begin end <utility> move forward <memory> addressof undeclare_reachable <functional> ref cref bind <algorithm> all_of any_of none_of move copy_if <numeric> iota (this is not a complete list). Additionally when /any/ two libraries are mixed (e.g. llvm and boost), there is a large potential for name clashes even when namespaces are judiciously used. The carefully crafted C++ library should be aware of ADL issues. It should know when ADL is being used, and when it isn't, and use ADL purposefully, not accidentally.> There are also two issues with rvalue references and the STL: > > 1. EquivalenceClasses, in the insert and findLeader functions, it uses map functions which have versions taking rvalue references. This results in a compiler error because the private constructor of ECValue is inaccessible. The easiest fix is to explicitly call the constructor of ECValue before the insert/find calls.If I'm understanding the concern, this looks easy to fix to me by simply making the private construction explicit within the scope of the code which has access to this constructor: iterator insert(const ElemTy &Data) { return TheMapping.insert(ECValue(Data)).first; } You can make this change today, and it will work with your C++03 compiler with no loss of efficiency, and arguably improved clarity.> 2. There are numerous places where there is a std::pair of pointer types and passing 0 results in the rvalue reference constructor being called. The type of 0 is deduced to be an int and an int isn't convertable to a pointer, so it fails to compile. The fix is to use nullpte (GCC doesn't seem to have this yet) or to static_cast<T*>(0) where T is the appropriate type.I anticipate a standards fix for this one. The pair<T1, T2>::pair<U, V>(U&&, V&&) constructor should not participate in overload resolution unless U is implicitly convertible to T1 and V is implicitly convertible to T2 (easily implemented with enable_if and is_convertible). pair is still a hotbed of controversy on the committee, but I expect it to be greatly simplified over its current form prior to ratification. -Howard
On Thu, Dec 3, 2009 at 6:29 AM, Howard Hinnant <hhinnant at apple.com> wrote:> On Dec 3, 2009, at 5:24 AM, Ahmed Charles wrote: > > > Sorry, always end up not replying to the list: > > > > The main issue with dealing with next this way is that people adding new > uses of next will probably not be using c++0x and therefore won't know it's > ambiguous and that it needs to be qualified. > > True. But when this code is compiled under C++0X you get an easy to > diagnose, easy to fix, compile-time diagnostic (as opposed to a hard-to-find > run time error). >Is the runtime error you are inferring potentially due to a behavioral difference between llvm::next and std::next?> (this is not a complete list). Additionally when /any/ two libraries are > mixed (e.g. llvm and boost), there is a large potential for name clashes > even when namespaces are judiciously used. The carefully crafted C++ > library should be aware of ADL issues. It should know when ADL is being > used, and when it isn't, and use ADL purposefully, not accidentally. >It's unfortunate that the standard isn't a bit more deliberate here, especially given that these issues are (should be) well known.> > There are also two issues with rvalue references and the STL: > > > > 1. EquivalenceClasses, in the insert and findLeader functions, it uses > map functions which have versions taking rvalue references. This results in > a compiler error because the private constructor of ECValue is inaccessible. > The easiest fix is to explicitly call the constructor of ECValue before the > insert/find calls. > > If I'm understanding the concern, this looks easy to fix to me by simply > making the private construction explicit within the scope of the code which > has access to this constructor: > > iterator insert(const ElemTy &Data) { > return TheMapping.insert(ECValue(Data)).first; > } > > You can make this change today, and it will work with your C++03 compiler > with no loss of efficiency, and arguably improved clarity.That is correct.> > 2. There are numerous places where there is a std::pair of pointer types > and passing 0 results in the rvalue reference constructor being called. The > type of 0 is deduced to be an int and an int isn't convertable to a pointer, > so it fails to compile. The fix is to use nullpte (GCC doesn't seem to have > this yet) or to static_cast<T*>(0) where T is the appropriate type. > > I anticipate a standards fix for this one. The pair<T1, T2>::pair<U, > V>(U&&, V&&) constructor should not participate in overload resolution > unless U is implicitly convertible to T1 and V is implicitly convertible to > T2 (easily implemented with enable_if and is_convertible). pair is still a > hotbed of controversy on the committee, but I expect it to be greatly > simplified over its current form prior to ratification. >There will probably be at least one vendor that will ship with this issue, so if people are interested on llvm compiling there, this would need to be addressed. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20091203/b09c7cc5/attachment.html>