stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Travis Vitek" <Travis.Vi...@roguewave.com>
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:
>>>
>>>   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
>>>
>
>

Mime
View raw message