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
