incubator-stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Travis Vitek" <>
Subject RE: HP aCC 3.63 compilation error in 21.string.append.stdcxx-438.cpp
Date Wed, 25 Jun 2008 00:07:01 GMT

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:
>> Actually, I think it was me that caused the problem, in the following
>> change:
>> 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 
>> 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:
>>> Farid?
>>> Thanks
>>> Martin

View raw message