@trust is an encapsulation method, not an escape

[Steven Schveighoffer via Digitalmars-d]

First, I want to say that I didn't want to cause a huge rift between D 
developers with this, I didn't think this was such a drastic issue, just 
a possible idea. But here we are. I hope we can mend this, and move 
forward. But on to the discussion.

On 2/5/15 6:39 PM, Walter Bright wrote:
> Consider the following code excerpted from std.array.join:
>
>    static U trustedCast(U, V)(V v) @trusted { return cast(U) v; }
>    return trustedCast!RetType(result);
>
> This is because the compiler would complain that the following line
> would not be @safe:
>
>    return cast(RetType)(result);
>
> The rationale is "I know it's safe, so I'll create an @trusted wrapper
> to eliminate the error." What comes next is "that's cumbersome. How
> about a better syntax:"
>
>    trusted {
>       return cast(RetType)(result);
>    }

No. This was NEVER THE ARGUMENT.

Now, let me explain why the latter is BETTER.

It's better because I know where it is used. It's used in one place, and 
I can squash it right there saying "No, you can't do this in this one 
place." Instead of reviewing an API in ALL POSSBILE CONTEXTS (which if 
trustedCast is a public API, would be a lot), I have to review one call 
in ONE CONTEXT.

The former is WORSE because it can be used in 100 places. Now I have to 
go through and fix ALL THOSE FUNCTIONS that use it, because its 
interface was exposed to the whole of phobos.

The problem, as we have said many times, is maintenance. Sure, in both 
cases they never should have shown up in the first place. But there they 
are, and we now have to fix them. All of them.

And let's also talk about long term maintenance. Any time a @trusted 
function is amended or tweaked, we have to reconsider all the contexts. 
If you mark a single line as @trusted, then additional lines to the same 
function would not need as much scrutiny, especially if you warn when 
@trusted tainted data is touched.

> The trouble with it is, what if the cast is converting from an integer
> to a pointer? That could lead to memory corruption. The code allows a
> potentially memory corrupting operation to be inserted into code that is
> otherwise @safe.

And so, you reject that code in both cases, except in the case where you 
don't define a callable API, you don't have to worry about any other 
code or contexts. This has to be the worst example to explain your point.

> The only way to deal with it is to then manually review everything about
> 'RetType' and 'result' to prove to oneself that it is not converting
> random bit patterns into pointers. In other words, one is manually
> reviewing @safe code for memory corruption errors.

@trusted code is not @safe code. You are reviewing that one line in its 
current context, not the whole function to see where it is called in 
various contexts.

> The solution is to regard @trusted as a means of encapsulating unsafe
> operations, not escaping them. Encapsulating them means that the
> interface from the @trusted code is such that it is usable from safe
> code without having to manually review the safe code for memory safety.
> For example (also from std.array):
>
>    static void trustedMemcopy(T[] dest, T[] src) @trusted
>    {
>      assert(src.length == dest.length);
>      memcpy(dest.ptr, src.ptr, src.length * T.sizeof);
>    }
>
> I don't have to review callers of trustedMemory() because it
> encapsulates an unsafe operation (memcpy) with a safe interface.

Sure. But let's consider it's called in one place:

trustedMemcopy(array[pos..pos+to_insert], tmp);

What if that became:

assert(tmp.length == to_insert); // same as src.length == dest.length
@trusted memcpy(array.ptr + pos, tmp.ptr, tmp.length);

What is missing here is, make sure the type of array and tmp is the 
same. All one has to do is review the function to see that. You could 
put in a static assert if you wish:

static assert(is(typeof(array) == typeof(tmp)));

If there are multiple places that it's called from, sure we can 
encapsulate that in a function:

static void trustedMemcopy(T[] dest, T[] src)
{
    assert(src.length == dest.length);
    @trusted memcpy(dest.ptr, src.ptr, src.length * T.sizeof);
}

There is very little difference here. Except if I add code to my version 
of trustedMemcopy that is @system, I have to properly mark that too, and 
that should imply greater scrutiny. I fail to see why making extra 
unintended @system calls inside a "trusted" function shouldn't require 
extra marking. Consider if this is a github review, and the context for 
the new lines is missing (i.e. you don't see that the new line is inside 
a @trusted function because github doesn't give you that line).

> The reason @trusted applies only to functions, and not to blocks of
> code, is that functions are the construct in D that provides an
> interface. Arbitrary blocks of code do not have a structured interface.
> Adding @trusted { code } support will encourage incorrect uses like the
> opening example. The existence of @trusted blocks will require review of
> every line of code in the function that encloses it, and transitively
> every function that calls it!

This is like opposite land! You don't have to review every function that 
calls a @safe function with @trusted escapes any more than you have to 
review every function that calls a @trusted function! That is the point 
of having the attribute on the function call.

However, if @trusted is improperly placed on a function, then I have to 
break every function that calls it if it now becomes @system. While the 
same would be for a @safe function that has incorrectly escaped @trusted 
calls, the temptation to make those small calls into a nice neat public 
API without context is not there.

Having @trusted as a function attribute *encourages* bad encapsulation 
of unsafe calls WITHOUT CONTEXT. This is so easy to do with templates.

> Adding @trusted as a function attribute, on the other hand, only
> requires review of the function's interface to determine if it is
> acceptable to use in safe code. Safety review of its callers is
> unnecessary.

I see zero difference between a @trusted function and a @safe function 
with @trusted escapes. You have to review both with the same scrutiny. 
Except less so for the one with escapes because you only have to 
scrutinize the @trusted calls.

You know, I'm surprised you have rejected H.S. Teoh's proposal because 
it seems like you are stuck on the concept that nobody should have to 
review @safe code for memory problems. Perhaps marking functions 
@trusted and then having escapes with @system is a better way of looking 
at it, because you do have to review that code for memory problems.

The bottom line of my reasoning is that code changes over time, by 
different people. Context is forgotten. It's much better to have the 
compiler verify you know what you are doing when working with @trusted 
than it is to just allow anyone to inject code anywhere.

-Steve