Improve scoped implementation

[monarchdodra]

This changes the scoped(T, Args...) function, into a template parametrized on T, that returns a function parametrized on Args. EG:

template scoped(T)
    if (is(T == class))
{
    private static struct Scoped;
    @system auto scoped(Args...)(auto ref Args args);
}

The advantages are:

1. The Scoped struct definition depends only on the type T (previously, a new definition existed for each and every combination of Args)
2. It allows aliasing and instantiating the template, to get a template function that is ready for future use.

Finally, I gave Scoped an @disabled this() to prevent our smarter users from trying to build a Scoped using typeof.

Also, moved example into a documented unittest.

[ghost]

What's wrong with passing Scoped by ref?

[monarchdodra]

What's wrong with passing Scoped by ref?

I'm confused about what you are refering to...?

[ghost]

Actually nevermind, I used to experiment with scoped before while trying to recreate Qt's stack-based events. For example instantiating from the heap every time you move your mouse to generate a MouseMove event is expensive, so using scoped could do the trick, while keeping event classes polymorphic. But I think I ran into too much problems to make it viable.

So, nevermind. :]

[9rnsr]

Adding private is not necessary, because they cannot be accessible from outer scope of Eponymous Template.

[monarchdodra]

That's what I though, but Andrei had told me to put them in one of my previous pulls. I'll remove them if they are useless.

[9rnsr]

I think this is good direction.

[monarchdodra]

I think this is good direction.

TY for the review. All points addressed.

[9rnsr]

Thanks!

[denis-sh]

Why did you add this line?

[monarchdodra]

Because Scoped_store = void didn't actually guarantee that Scoped_store was 0 initialized. writing:

Scoped result; //Fine

but

//@disable this(), init initialialize
Scoped result = Scoped.init;

In this case, result.Scoped_store was not 0 initialized. This is particularly problematic, because it holds the value of d, which you need to be 0 initialized.

In the end, I changed result to be void initialized anyways. The advantage here I guess is that you only initialize the part of Scoped you need initialized.

[denis-sh]

I mean this line has no effect as because of line 3356: *cast(size_t*) &result.Scoped_store[$ - size_t.sizeof] = d;

[monarchdodra]

You're on to something, but it's the other way around: calculating d entails calling cast(void*) result.Scoped_payload - result.Scoped_store.ptr, which calls Scoped_payload. Scoped payload will make a read to that area to verify alignment, so that variable must be initialized before calling that line.

That said, the line 3356, as you mention, is itself useless, and has always been, since Scoped_payload will have already done the job. I'll open an improvement to remove it.

I was wondering: I seem to recall you were the one that wrote that code originally. Is any of this code necessary on a 64 bit run-time? Do you think it could be possible to place that code in a version block?

[denis-sh]

Damn, forget my own code...
Sorry, but my code isn't good because memmove is called without any need at first access to Scoped_payload. This is the right way to initialize &result.Scoped_store[$ - size_t.sizeof] (instead of this line and the next one):

void* alignedStore = cast(void*) aligned(cast(size_t) Scoped_store.ptr);
immutable size_t d = alignedStore - Scoped_store.ptr;

[denis-sh]

Is any of this code necessary on a 64 bit run-time?

AFAIK, there is nothing special with 64 bit. If you sure every struct on platform X is properly n-bytes aligned and the class requires m-bytes alignment where m <= n, you just tell compiler that Scoped struct is m-bytes aligned (not sure about the proper way to do it, maybe U[0] will help but note about Issue 5332) and branch out alignment code.

[monarchdodra]

Alright. I didn't want to touch that code too much, but I think I fully understand it now, so I'll tweak it as you are suggesting.

Regarding the 64 bit thing, I'm not comfortable with changing anything there, so I'll just leave it as is, and maybe leave a TODO in there or something.

[monarchdodra]

Done: #1218