Martin Sebor wrote: > >Travis Vitek wrote: >> >>> Martin Sebor wrote: >>> >>> The test fails to compile with HP aCC 3.63. The error messages >>> point to the patch for the issue: >>> >>> http://svn.apache.org/viewvc?view=rev&revision=629550 >> >> Actually, I think it was me that caused the problem, in the following >> change: >> >> http://svn.apache.org/viewvc?view=rev&revision=669747 >> >> The fix was for STDCXX-170 and friends, but break on every >> compiler when the Iterator::operator*() returns a temporary >> (which is legal). > >That's what I thought was your objection to Farid's patch.
Yes, it was. Unfortunately I forgot about this issue part way through making my patch. >Am I >to understand that the code somehow detects whether operator*() >returns a rvalue or lvalue and the branch with the cast is only >supposed to be entered for lvalues? Sort of. The code checks that the _InputIter is a pointer. If it is, then we know it is safe to use the pointer to check for overlap. >(I'm still uncomfortable >with the cast and would like to understand why it's safe). It seems that the cast itself is legal because expr.static.cast says An expression e can be explicitly converted to a type T using static_cast of the form static_cast<T>(e) if the declaration "T t(e);" is well-formed, for some invented temporary variable t. The declaration const_reference t(*__first); is legal if *__first is convertible to value_type, which is required, so everything seems okay here, right? The problem with the original testcase was that the cast can end up giving us a pointer to a temporary if *__first doesn't return a reference, which can result in invalid results. The testcase I provided showed this. My fix eliminated the cast (which caused breakage), but verifies that __first is actually a raw pointer type before trying to get a reference out of it. So I think that we may be able to combine the two patches to come up with a usable fix. If we avoid doing the cast until we know that __first is a pointer, then we can be sure that the cast is giving us reliable results. If __first is not a pointer, then all bets are off and we have to fall back to making a copy. > >> >> It looks like I'd need to do a special case when the iterator type is >> pointer. I don't see any way to legally check for no overlap without >> that, so the only option I can see then is to always make >the copy and >> fix it with an overload selection trick (which would only be >appropriate >> for 4.3.x). > >I think it should be fine to optimize just the common case >(const_pointer) and leave the rest unoptimized (i.e., make >a copy). Or can you think of another common use that should >be optimized as well? I think all cv-qualified pointer types could be optimized in this way. > >> >> Travis >> >>> Looking at the patch I don't see how the reinterpret_cast to >>> const_reference can possibly be correct, and I'm not sure we >>> satisfactorily resolved the same question the first time it >>> was raised back in March: >>> >>> http://markmail.org/message/eijfmt3wxhg25bvs >>> >>> Farid? >>> >>> Thanks >>> Martin >>> > >
