Given my combined hashmap and hashset container `HashMapOrSet` defined at

https://github.com/nordlow/phobos-next/blob/master/src/hashmap_or_hashset.d

with deterministic memory management and disabled copy constructions and a member byElement() defined as

        @property auto byElement()() inout
        {
            dln("entering byElement");
            alias This = ConstThis;
            // TODO is the use of `&this` incorrect when `this` is an r-value?
            auto result = ByElement!This((ElementRef!This(cast(This*)&this)));
            result.initFirstNonEmptyBin();
            dln("leaving byElement");
            return result;
        }

        scope auto opSlice()() inout return
        {
            return byElement();
        }

where

        static private struct ByElement(HashMapOrSetType)
        {
            static if (is(ElementType == class))
            {
                /// Get reference to front element (key and value).
                @property scope auto front()() return @trusted
                {
                    /* cast away const from `HashMapOrSetType` for classes
                     * because class elements are currently hashed and compared
                     * compared using their identity (pointer value) `is`
                     */
                    return cast(ElementType)table.binElementsAt(binIx)[elementOffset];
                }
            }
            else
            {
                /// Get reference to front element (key and value).
                @property scope auto front()()
                {
                    return table.binElementsAt(binIx)[elementOffset];
                }
            }

            public ElementRef!HashMapOrSetType _elementRef;

            alias _elementRef this;
        }

iteration via opSlice works fine when `X` is fed as `this` to `byElement` as an l-value as in

    import digestx.fnv : FNV;
    alias X = HashMapOrSet!(uint, void, null, FNV!(64, true));
    const x = X.withElements([11].s);
    foreach (e; x.byElement) {}

but when `X` is fed as an r-value `this` to `byElement` as in

    import digestx.fnv : FNV;
    alias X = HashMapOrSet!(uint, void, null, FNV!(64, true));
    foreach (e; X.withElements([11].s).byElement) {}

I get a behaviour that seems to indicate that the instance of `X` is freed after `byValue()` (opSlice) returns but before the `ByValue`-range is consumed, resulting in incorrect (undefind) behaviour.

Is it incorrect to use `&this` as follows

            auto result = ByElement!This((ElementRef!This(cast(This*)&this)));

in the `byElement`?

If so, is there a way around this problem except for making my container be RC-allocated?

My current proposal for a solution is to make `byElement` a free unary function

    byElement(auto ref X x)

which statically checks via

    static if (__traits(isRef, x))

whether the `X`-instance is passed as either an

- l-value, where my current solution works (ByLvalueElement), or
- r-value, in which the X-instance instead is moved into range (ByRvalueElement).

byValue can be used via UFCS, but this solution removes the possibility for using the opSlice-overload which I can live with.

Further, does all EMSI-container-style containers (with disabled postblit) have the same issue with ranges over r-value instances of its containers?

On Sunday, 14 January 2018 at 01:38:17 UTC, Nordlöw wrote:
> My current proposal for a solution is to make `byElement` a free unary function
>
>     byElement(auto ref X x)
>
> which statically checks via
>
>     static if (__traits(isRef, x))
>
> whether the `X`-instance is passed as either an
>
> - l-value, where my current solution works (ByLvalueElement), or
> - r-value, in which the X-instance instead is moved into range (ByRvalueElement).

That sounds reasonable. For something like `foreach (e; makeRange().wrapRangeByRef())`, referencing the makeRange() struct rvalue by pointer in the wrapped range won't work, as the underlying range lifetime ends with the foreach range expression, while the wrapped range's lifetime extends to the end of the foreach loop. So making the wrapped range take ownership of a range rvalue by moving it into a member (of ByRvalueElement) makes sense IMO.
Lvalues aren't moved implicitly by the compiler, so referencing them by pointer (&this) is safe.

On Sunday, 14 January 2018 at 14:04:46 UTC, kinke wrote:
> That sounds reasonable. For something like `foreach (e; makeRange().wrapRangeByRef())`, referencing the makeRange() struct rvalue by pointer in the wrapped range won't work, as the underlying range lifetime ends with the foreach range expression, while the wrapped range's lifetime extends to the end of the foreach loop. So making the wrapped range take ownership of a range rvalue by moving it into a member (of ByRvalueElement) makes sense IMO.
> Lvalues aren't moved implicitly by the compiler, so referencing them by pointer (&this) is safe.

Thanks for confirmation, kinke.

Further, if we add the new trait(s)

__trait(isLvalue, symbol)
__trait(isRvalue, symbol)

or perhaps simply just

__trait(isRvalue, symbol)

that can be applied with symbol being `this` to make this static code branching work inside member functions such as opSlice aswell. What do you think of such an addition?

Note that __trait(isLvalue, this) cannot be used to detect whether `this` is an l-value or an r-value, which I find strange.

On Sunday, 14 January 2018 at 21:57:37 UTC, Nordlöw wrote:
> Note that __trait(isLvalue, this) cannot be used to detect whether `this` is an l-value or an r-value, which I find strange.

Shall be

__traits(isRef, this)

On 1/14/18 4:59 PM, Nordlöw wrote:
> On Sunday, 14 January 2018 at 21:57:37 UTC, Nordlöw wrote:
>> Note that __trait(isLvalue, this) cannot be used to detect whether `this` is an l-value or an r-value, which I find strange.
> 
> Shall be
> 
> __traits(isRef, this)

That would be difficult because lval/rval is known on he callee side. -- Andrei