Fix 22977 - can escape scope pointer returned by nested function

[dkorpel]

Blocked by:

• [x] https://github.com/dlang/phobos/pull/8481
• [x] https://github.com/atilaneves/automem/pull/69
• [ ] https://github.com/libmir/mir-algorithm/pull/464

[dlang-bot]

Thanks for your pull request and interest in making D better, @dkorpel! We are looking forward to reviewing it, and you should be hearing from a maintainer soon. Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	22977	normal	[dip1000] can escape scope pointer returned by nested function

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#14236"

[thewilsonator]

Dependant PR merged

[thewilsonator]

Please rebase

[dkorpel]

automem and kaleidicassociates/lubeck on buildkite currently fail because they rely on nested functions to swallow scope.

[thewilsonator]

cc @atilaneves @9il

[dkorpel]

Looks like the problem has to do with forward / move from core.lifetime.

[dkorpel]

~~Actually, it looks like the problem is that with pragma(inline, true) DMD thinks inlined functions are nested functions in escape.d.~~ Edit: template alias parameters are confusing me :/

[ljmf00-wekaio]

Is there any presented alternative to trust the return value but not the whole function body? Perhaps, making it a function instead of a delegate and trust the lambda?

[dkorpel]

Is there any presented alternative to trust the return value but not the whole function body?

This is a bug fix, returning a scope pointer through a nested function is not supposed to be a way to lose scope, so there's no alternative provided. I presented a way to cast away scope in this thread:

https://forum.dlang.org/post/[email protected]

auto ref assumeNonScope(T)(auto ref scope T arg)
{
    return *(cast(T*) cast(size_t) &arg);
}

Though it's not nothing I can endorse with full confidence. I wonder if Walter and Atila would approve it for inclusion in std.exception.

[ljmf00-wekaio]

Is there any presented alternative to trust the return value but not the whole function body?

This is a bug fix, returning a scope pointer through a nested function is not supposed to be a way to lose scope, so there's no alternative provided. I presented a way to cast away scope in this thread:

https://forum.dlang.org/post/[email protected]

auto ref assumeNonScope(T)(auto ref scope T arg)
{
    return *(cast(T*) cast(size_t) &arg);
}

Though it's not nothing I can endorse with full confidence. I wonder if Walter and Atila would approve it for inclusion in std.exception.

I meant wrapping in a @trusted lambda. AFAIK, @trusted functions doesn't check for scope rules.

[dkorpel]

I meant wrapping in a @trusted lambda. AFAIK, @trusted functions doesn't check for scope rules.

They don't raise errors, but may still infer return scope or scope (see the thread I linked)

[WalterBright]

Can someone restart the buildkite tests?

[dkorpel]

I force pushed to restart buildkite, and I'm working on reducing the lubeck failure with help of dustmite. It's still large, but this is my progress so far:

// Compiles without this PR, but with this PR and -preview=dip1000 you get:
// Error: escaping reference to stack allocated value returned by `autoExpandAndForwardElem()`

template staticMap(alias fun, args...)
{
    alias staticMap = AliasSeq!();
    static foreach (arg; args)
        staticMap = AliasSeq!(staticMap, fun!arg);
}

alias AliasSeq(T...) = T;
alias Iota(size_t j) = Iota!(0, j);
template Iota(size_t i, size_t j)
{
    static if (i == j)
        alias Iota = AliasSeq!();
    else
        alias Iota = AliasSeq!(i, Iota!(i + 1, j));
}

alias _IteratorOf(T : Slice!(Iterator, N), Iterator, size_t N) = Iterator;

struct RefTuple(T...)
{
    T expand;
}

template autoExpandAndForward(alias value)
{
    template autoExpandAndForwardElem(size_t i)
    {
        @property autoExpandAndForwardElem(){ return value.expand[i]; }
    }
    alias autoExpandAndForward = staticMap!(autoExpandAndForwardElem, Iota!(value.expand.length));
}

template _naryAliases(size_t n)
{
    static if (n == 0)
        enum _naryAliases = "";
    else
    {
        enum i = n - 1;
        enum _naryAliases = _naryAliases!i ~ "alias " ~ ('a' + i) ~ " = args[" ~ i.stringof ~ "];\n";
    }
}

template naryFun(functions...)
{
    static foreach (fun; functions)
        alias naryFun = fun;
}

auto sliced(size_t N, Iterator)(Iterator iterator, size_t[N] ...)
{
    alias C = typeof(iterator);
    return Slice!(typeof(C.init), N)();
}

struct Slice(Iterator_, size_t N_ = 1, Labels_...)
{
    alias Iterator = Iterator_;
    Iterator _iterator;
    size_t length()() { assert(0); }
}

struct IotaIterator(I)
{
    I opIndex(ptrdiff_t ) {assert(0);}
}

template _zip_types(Iterators...)
{
    static if (Iterators.length)
    {
        enum i = Iterators.length - 1;
        alias T = typeof(Iterators[i].init[sizediff_t.init]);
            alias _zip_types = AliasSeq!(_zip_types!(Iterators[0 .. i]), T);
    }
    else
        alias _zip_types = AliasSeq!();
}

template _zip_index(Iterators...)
{
    static if (Iterators.length)
    {
        enum i = Iterators.length - 1;
        enum _zip_index = _zip_index!(Iterators[0 .. i]) ~ "_iterators[" ~ i.stringof ~ "][index], ";
    }
    else
        enum _zip_index = "";
}

struct ZipIterator(Iterators...)
{
    Iterators _iterators;
    auto opIndex(ptrdiff_t index) { return mixin("RefTuple!(_zip_types!Iterators)(" ~ _zip_index!Iterators ~ ")"); }
}

struct MapIterator(Iterator, alias _fun)
{
    Iterator _iterator;

    auto opIndex(ptrdiff_t index) scope
    {
        static if (is(typeof(_iterator[0]) : RefTuple!T, T))
        {
            auto t = _iterator[index];
            return _fun(autoExpandAndForward!t);
        }
        else
            return _iterator[index];
    }
}

auto _mapIterator(alias fun, Iterator)(Iterator )
{
    return MapIterator!(Iterator, fun)();
}

struct SliceIterator(Iterator, size_t N)
{
    Iterator _iterator;
    auto opIndex(ptrdiff_t ) { return _iterator; }
}

Slice!(SliceIterator!(Iterator, P)) pack(size_t P, Iterator, size_t N)(Slice!(Iterator, N) )
{
    assert(0);
}

template map(fun...)
{
    static if (__traits(isSame, naryFun!fun, fun))
    {
        alias f = fun;
        auto map(T)(T slice)
        {
            alias MIterator = typeof(_mapIterator!f(slice._iterator));
            return Slice!MIterator();
        }
        auto map() {}
    }
    else
        alias map = map!(staticMap!(naryFun, fun));
}

auto zip(Slices...)(Slices )
{
    alias Iterator = ZipIterator!(staticMap!(_IteratorOf, Slices));
    alias Ret = Slice!Iterator;
    return Ret();
}

void failureTest()
{
    [].sliced(4, 4).pack!1.zip(Slice!(IotaIterator!int).init).map!((a, b) => a[0..b]);
}

[dkorpel]

Reduced further:

struct Tuple(T...)
{
    T expand;
}

auto autoExpandAndForward(alias value)()
{
    return value.expand[0];
}

struct MapIterator
{
    int* p;
    int* foo() scope
    {
        auto t = Tuple!(int*)(p);
        return autoExpandAndForward!t;
    }
}

[PetarKirov]

@dkorpel now that https://github.com/libmir/mir-algorithm/pull/464 is closed, what's the way forward? You mentioned something about improving the inference in the compiler?

[dkorpel]

what's the way forward?

To make dmd able to infer return scope on functions that return a parameter through a nested function.

[pbackus]

I ran into this while working on @safe allocators. I've been using @trusted lambdas to access @system variables in templated code, and it took me a long time to realize that this was completely breaking scope inference.

Is there anything blocking progress on this and/or #14492, or have they just been de-prioritized for now?

[dkorpel]

A bit of both. Since dip1000 by default has been postponed in favor of editions, these bugs have lower urgency. It's also 'blocked' by it being difficult to find an easy path forward: existing quirks are relied on in tests and buildkite projects making it hard to untangle the code.

[pbackus]

existing quirks are relied on in tests and buildkite projects making it hard to untangle the code.

I think the only reasonable way forward here is to bar the use of -preview=dip1000 in tests run by Buildkite. If a project supports running tests without the flag, test without it; otherwise remove the project altogether. The current implementation is so deficient that any project that relies on it is almost certainly broken already.

[rikkimax]

Don't remove it, but certainly make it optional that it passes.

DIP1000 is still behind the preview switch, and we need to close up these big holes like #14492 tries to do.