[Issue 14125] @trusted nested helper functions in std.file

[via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=14125

--- Comment #76 from Steven Schveighoffer <schveiguy at yahoo.com> ---
After reading this quite long thread (and I'm not going to CC myself because of
this), I want to summarize the viewpoints here. I think I see that everyone is
in slight agreement, but does not realize the other's *complete* viewpoint.

1. We all agree that a public function such as:

@trusted auto tmalloc(size_t x) {return malloc(x);}

Is no good, should never pass review.

2. There are occasions where inside a very lengthy function one wishes to be
called from @safe code, that a small portion of code which uses unsafe
functions, but in a safe manner, is needed. One would wish that in such a
function all the actual @safe calls are still checked by the compiler, and any
additions to the @safe portion should be checked for future maintainers.

3. A static nested function inside such a function is a license to commit sin
at will. In other words, such a function allows abuse by current and future
maintainers of that function. Therefore, even nested functions like the one in
point 1 above should probably be rejected. However, @trusted static nested
functions that do NOT leak memory details can be allowed.

4. A possible restriction (but not bulletproof) is to use an immediately-called
lambda to execute a trusted call. The issue here is that it won't be inlined.
Hence the desire to use static nested functions.

5. There is a general issue that adding valid @trusted calls inside a @safe
function is simply allowed in review without too much consideration for what it
means for future maintenance.

-------------

Now, point 4 is not bullet proof. Walter brought up a case where it is not, and
it's not hard to see it can be abused. When a @trusted call affects variables
inside a @safe function, it can mean that depending on the calls, any code that
then *uses* those variables, even in a safe way, is suspect. This means that
@trusted must extend to all those uses. Such a position makes for an incorrect
assumption about the entire function.

However, I think even worse is the idea that we add a static callable @trusted
function nested inside a @safe function, that can be called in an unsafe way.
Such a function cannot guarantee calls to it are safe. A lambda is much better
because review of its uses involves one line. But only if its details do not
leak!

So I would say, as a review requirement, we should limit internal @trusted
calls ONLY to lambdas, unless the static nested function does not leak memory
details. And inside those lambdas, the code should not affect any variables in
@safe code in a way that has the potential for abuse. I think this is doable,
because you can ask that question objectively by reading the one call. It
doesn't matter if code *doesn't* use that variable in an un at safe way, just the
*potential* of using it is enough to cause the review to fail.

There is the issue with not inlining lambdas. Tough. Get that fixed, it's not
worth corrupting memory to save some performance.

That's my $.02. I'll see about reviewing std.file.read and try and apply this
methodology to fix the issues.

--