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

[via Digitalmars-d-bugs]

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

--- Comment #63 from Walter Bright <bugzilla at digitalmars.com> ---
(In reply to David Nadlinger from comment #49)
> No. I want the "...safe code...", which is in fact "...code that might be
> safe depending on the template arguments...", to trigger attribute inference
> as usual, while the compiler should trust me on the
> potentially-unsafe-but-manually-verified part (such as the cast, or a
> temporary allocation, etc.).

There's simply no way that:

    static U trustedCast(U, V)(V v) @trusted { return cast(U) v; }

can pass muster as trusted code. Trusted code must provide a safe interface,
and that template does not. For example, it could be used to convert ints to
pointers. The trusted part needs to go up a level in the logic.

Looking at join(), it is used thus:

    static U trustedCast(U, V)(V v) @trusted { return cast(U) v; }
    return trustedCast!RetType(result);

To fix this, I'd start with a rewrite to:

    static RetType myCast(typeof(result) result) { return cast(RetType)result;
}

That isn't right, either, but at least it is a step forwards in being
constrained to only have to deal with two types instead of any types. I.e. we
must not allow converting ints to pointers, or pointers to ints to pointers to
doubles, etc.

I.e. in order to be safe, result must be safely convertible to RetType. Where's
the check for that? If I look upcode, I see:

    auto result = uninitializedArray!(RetTypeElement[])(length);

Whoa! We have result[] being an array of garbage, which is then pretended to be
safely convertible to perhaps an array of pointers? Examining the code further,
we see an array to initialize result[].

How do we know that every member of result[] is initialized exactly once?
Examining the code, I see that we are TRUSTING that code to do the right thing.
I.e. the whole thing is, literally, @trusted code. Pretending to infer it as
@safe is wrong, it is not @safe code, and not checkable as @safe code. It's
@trusted.

Also, uninitializedArray() is marked as @trusted. Returning arrays of pointers
that consist of unknown garbage cannot be @trusted. uninitializedArray() isn't
any more trustworthy than malloc(), and should be marked as @system.

Given the state of this code I worry that the entire std.array needs to be
reviewed for memory safety. The apparent necessity of:

    static U trustedCast(U, V)(V v) @trusted { return cast(U) v; }

looks like the canary died trying to tell the programmer something was wrong,
and got fed to the cat instead of fixed.

I am not saying that std.array is broken as far as the users of it are
concerned. I am saying that it is broken in that it sidesteps the language's
attempts to check safety, and serves as a wrong example on how to use D's
features to write memory safe code. (uninitializedArray() is definitely broken,
however.)

It reminds me of the early days of D2 where people were arguing that the
compiler was too strict in its const checking. I believe we finally did reach
consensus that it was doing the right thing, and I hope history will repeat
itself :-)

Yes, some strong words in this message. Makes me hope I didn't make a mistake -
but surely if I did it'll get pointed out :-) en garde.

--