[LLVMdev] patch for portability

[Howard Hinnant]

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