Boost logo

Boost :

From: Aleksey Gurtovoy (agurtovoy_at_[hidden])
Date: 2005-05-24 20:53:58


David Abrahams writes:
> Aleksey Gurtovoy <agurtovoy_at_[hidden]> writes:
>
>>> Maybe you could add some commentary?
>>
>> Sure. Both of the marked lines above invoke undefined behavior on
>> dereference of the corresponding composite transform iterator because
>> its 'operator*' by the iterator is basically equivalent to this:
>>
>> int const& operator*() const
>> // ^^^^^^^^^^
>> {
>> // returning a reference to a member of a temporary object
>> // that will be destroyed at the end of the function block scope
>> return make_x( *base() ).get_i(); // #1
>> // return X( base()->x ).get_i(); // #2
>> }
>
> Oh. Well, I'm not sure that separating transform_iterator and
> projection_iterator would help here.

It would help in a sense that the choice whether to return by value or
by reference would be explicit, and the default (==
transform_iterator) would be always safe. Note that I'm not talking
about implementation here, just about what is presented to user at
high level.

> AFAICT this issue only really
> applies when using make_transform_iterator, and only when you're
>
> a. assuming a transform_iterator always returns by value
>
> and
>
> b. unclear on what your functions are doing.

There is more to it: even if you _are_ clear what your functions are
doing when you are writing the code, the trap is still there in the
form of code fragility and ultimately one of the worst maintanance
nightmares: suppose 'X::get_i' in my original example was written as

    struct X
    {
    ...
        int get_i() const { return this->i_; }
    // ^^^
    ...
    };

Now, given this particular signature, the code I cited is perfectly
valid and working, and I'd have hard time blaming anybody for writing
something along those lines. Yet change the signature to return by
reference (for instance, as an optimization measure) and you've got a
hundred of places in your program invoking undefined behavior, and
your compiler doesn't even warn you about any of them.

>
> OR
>
> c. You expect transform_iterator to be a perfect drop-in replacement
> for the transform_iterator from the earlier Boost library, which I
> admit is a fair expectation.

That, too.

>
> Maybe we need a make_transform_iterator variant that allows us to
> force a by-value return or something?

I would say transform_iterator should return by value by default and
there should be an explicit notation to request by reference semantics
(if applicable).

-- 
Aleksey Gurtovoy
MetaCommunications Engineering

Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk