Add annotations for functions which `return &this`

[don-clugston-sociomantic]

From dmd2.092 onwards, functions which use the return &this method chaining idiom, must be marked with the return annotation. This makes it impossible for @safe code to have references to destructed stack objects (see DIP25).

[don-clugston-sociomantic]

LGTM, but isn't the Sociomantic compiler < 2.075 ? Yeah it might be 2.073 or something. I think you meant to comment on a different PR though :)

[Geod24]

It applies to all 3 PRs in the proto. According to the CI it's 2.071. But it looks like STCreturn existed before 2.069, so all good.

[don-clugston-sociomantic]

Yeah, I did check that it existed, before making the PR. More interesting, though, are the remaining messages. ./submodules/swarm/src/swarm/neo/connection/RequestOnConnBase.d(416): Deprecation: copying (cast(const(void*))&elem)[0..4] into allocated memory escapes a reference to parameter variable elem

The code is:

            public void add ( T ) ( ref T elem )
            {
                static assert(!hasIndirections!(T));
                this.outer.outer.send_payload ~= (cast(const(void*))&elem)[0..T.sizeof];
            }

And actually I'm horrified that such a nasty bit of code has no comments. Now look at what this is doing. Suppose T is an int. This creates a void[] slice of the raw bytes of elem, and appends them to send_payload. That's a hacky way of serializing but OK.

But here's the problem. send_payload is not an array of bytes! It is actually:

         private const(void)[][] send_payload;

So though the code was intended to append the bytes, that's not what it's doing. It's appending the slices. Ie it is saving pointers to stack variables. Compiler is correct. Every single swarm neo request is broken...

Now that's interesting. I have never found a bug through the const system. However dip25 has paid for itself immediately. Congrats to everyone involved in it.

[Geod24]

Ie it is saving pointers to stack variables.

Actually you can't know from this snipped. add takes its argument by ref, so as long as send_payload's lifetime does not exceed's that of the element passed by ref you're fine. And that's a very good reason why comments are needed here.

One usage example is here: https://github.com/sociomantic-tsunami/swarm/blob/531545767cdef83a3ce03e64b735768aa311025f/src/swarm/neo/connection/RequestOnConnBase.d#L703-L716

And even though it's hackish, it seems correct. Good luck inspecting all call sites though.