As we all should know, std.conv.emplace does not play well with closures:

void main()
{
    int x = 42;
    struct S
    {
        auto getX() { return x; }
    }

    S s;
    assert(s.getX == x);

    auto ps = someBuffer.emplace!S();
    assert(s.getX == x);                // SEGFAULT!
}

As this is not fixable (we cannot pass closures around in D), we should IMHO forbid such usages of emplace (i.e. static assert(!isNested!S))

I was already working on this, when I stumbled upon this unittest in std.conv (simplified):

unittest
{
    int i = 0;
    struct S1
    {
        void foo(){++i;}
    }
    S1 sa = void;
    S1 sb;
    emplace(&sa, sb);   // automagically works
    sa.foo();
    assert(i == 1);
}

Of course there's no way to distinguish between this (legitimate?) use case and the former one, so preventing those segfaults will also prohibit this kind of usage.

What I'd like to know: is this usage widespread? Should we forbid it for the sake of security?

On 09/19/2016 01:27 PM, Lodovico Giaretta wrote:
> As we all should know, std.conv.emplace does not play well with closures:
>
> void main()
> {
>     int x = 42;
>     struct S
>     {
>         auto getX() { return x; }
>     }
>
>     S s;
>     assert(s.getX == x);
>
>     auto ps = someBuffer.emplace!S();
>     assert(s.getX == x);                // SEGFAULT!
> }

Should probably be `assert(ps.getX == x);`.

Note that it would also segfault with `auto ps = S.init;`, and for the same reason: missing context pointer.

> As this is not fixable (we cannot pass closures around in D), we should
> IMHO forbid such usages of emplace (i.e. static assert(!isNested!S))
>
> I was already working on this, when I stumbled upon this unittest in
> std.conv (simplified):
>
> unittest
> {
>     int i = 0;
>     struct S1
>     {
>         void foo(){++i;}
>     }
>     S1 sa = void;
>     S1 sb;
>     emplace(&sa, sb);   // automagically works
>     sa.foo();
>     assert(i == 1);
> }
>
> Of course there's no way to distinguish between this (legitimate?) use
> case and the former one, so preventing those segfaults will also
> prohibit this kind of usage.

There is a difference, though: You're copying an existing object here, including the context pointer. So maybe we could disallow the variant above that writes the .init value, and still allow the copying variant.

On Monday, 19 September 2016 at 11:45:25 UTC, ag0aep6g wrote:
> Note that it would also segfault with `auto ps = S.init;`, and for the same reason: missing context pointer.

Oh. I didn't thought about that. This means that in the following example, the initialization of `s` is more than a simple call to `S.init`. I was under the impression that in D `S.init` should represent a valid state for whatever type `S`.

void main()
{
    int x = 42;
    struct S
    {
        auto getX() { return x; }
    }

    S s; // this line does not simply call S.init,
         // but also creates a closure and puts it inside s.
}

> There is a difference, though: You're copying an existing object here, including the context pointer. So maybe we could disallow the variant above that writes the .init value, and still allow the copying variant.

Yeah, I will try that.

On 09/19/2016 02:24 PM, Lodovico Giaretta wrote:
> Oh. I didn't thought about that. This means that in the following
> example, the initialization of `s` is more than a simple call to
> `S.init`. I was under the impression that in D `S.init` should represent
> a valid state for whatever type `S`.

Yeah, .init and nested structs don't really fit together. On the one hand .init is supposed to be a valid value. On the other hand it must be a static value. Can't have both with nested structs. Maybe they shouldn't have .init at all.

On 19.09.2016 14:35, ag0aep6g wrote:
> On 09/19/2016 02:24 PM, Lodovico Giaretta wrote:
>> Oh. I didn't thought about that. This means that in the following
>> example, the initialization of `s` is more than a simple call to
>> `S.init`. I was under the impression that in D `S.init` should represent
>> a valid state for whatever type `S`.
>
> Yeah, .init and nested structs don't really fit together. On the one
> hand .init is supposed to be a valid value. On the other hand it must be
> a static value. Can't have both with nested structs. Maybe they
> shouldn't have .init at all.

This works:
import std.stdio;
void main(){
    int x=3;
    enum l=()=>x;
    writeln(x);
}

I.e. l has the correct runtime context even though it is a static value.

On Monday, 19 September 2016 at 11:27:03 UTC, Lodovico Giaretta wrote:
> What I'd like to know: is this usage widespread? Should we forbid it for the sake of security?

A big issue I'm finding is that inside most Phobos unittests classes and structs are not marked static even when they could, so putting any restriction on emplace will break most of them.

This of course is not a problem for Phobos (we can easily put static everywhere it's needed), but most DUB packages will probably have the same poor usage of static without the maintainance effort of Phobos (so will remain broken forever).

I already found many (direct or indirect) uses of emplace for non-static nested structs and classes in std.conv, std.typecons and std.experimental.allocator.

On 09/19/2016 02:55 PM, Timon Gehr wrote:
> This works:
> import std.stdio;
> void main(){
>     int x=3;
>     enum l=()=>x;
>     writeln(x);
> }
>
> I.e. l has the correct runtime context even though it is a static value.

Enums are a special kind of static value, though. Can't do that with `static` instead of `enum`. Enums (often?) don't manifest until they're used. Could .init work that way?

On 9/19/16 7:27 AM, Lodovico Giaretta wrote:

> What I'd like to know: is this usage widespread? Should we forbid it for
> the sake of security?

No. There is no security concern here. You are dereferencing a null pointer, which is perfectly safe.

-Steve

On Monday, 19 September 2016 at 14:22:16 UTC, Steven Schveighoffer wrote:
> On 9/19/16 7:27 AM, Lodovico Giaretta wrote:
>
>> What I'd like to know: is this usage widespread? Should we forbid it for
>> the sake of security?
>
> No. There is no security concern here. You are dereferencing a null pointer, which is perfectly safe.
>
> -Steve

Ok, wrong wording. I meant "should we forbid it to avoid long hours of debugging and unexpected behaviours? One uses emplace expecting that it Just Works(TM), which is not true for nested things."

On 9/19/16 10:26 AM, Lodovico Giaretta wrote:
> On Monday, 19 September 2016 at 14:22:16 UTC, Steven Schveighoffer wrote:
>> On 9/19/16 7:27 AM, Lodovico Giaretta wrote:
>>
>>> What I'd like to know: is this usage widespread? Should we forbid it for
>>> the sake of security?
>>
>> No. There is no security concern here. You are dereferencing a null
>> pointer, which is perfectly safe.
>>
> Ok, wrong wording. I meant "should we forbid it to avoid long hours of
> debugging and unexpected behaviours? One uses emplace expecting that it
> Just Works(TM), which is not true for nested things."

Maybe we can disable the emplace that will certainly cause a Null pointer segfault when used, and allow the copying version. But there may still be code that uses the struct without needing the context pointer, that would then be broken.

My opinion is just not to worry about it. We don't get much traffic here complaining of this issue, you may be the one hardest hit by it (and you likely won't have it happen any more).

For instance, we have way way more complaints of people using classes without allocating them, and that is a similar problem.

However, a sufficient warning in the docs is certainly in order, at least as long as we aren't going to forbid it.

-Steve