[Issue 14125] std.file has gotten out of hand

[via Digitalmars-d-bugs]

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

--- Comment #30 from Andrei Alexandrescu <andrei at erdani.com> ---
(In reply to hsteoh from comment #24)
> If that's not how @trusted was intended to be used, then fine, fix it.
> 
> However, that still does not address Dicebot's concern about protecting
> against future change, which I think is a far more important issue.

What is there right now is a false sense of security.

There's no way I can convince myself that 

    static trustedRealloc(void* p, size_t sz, uint ba = 0, const TypeInfo ti =
null) @trusted
    {
        return GC.realloc(p, sz, ba, ti);
    }

can actually be trusted by inspecting it. It's essentially a license for the
code using it to call realloc and pretend it's safe.

So the whole notion that "not one @system line can be added without the
compiler protesting" is wrong. There's plenty of stuff that can be added that
will pass compilation flag free and is completely unsafe - all in a @safe
function because we gave ourself the illusion of safety at the wrong level of
granularity.

A trusted function is what it sounds: I look at it and I can vouch for its
safety, even though the compiler cannot prove it. By availing myself of a
trusted one-liner that is in fact completely unsafe, I can corrupt any amounts
of @safe code with it.

> Currently, other than what's currently in std.file which apparently is not
> "proper use" of @trusted, there is no way to indicate which part of a larger
> function body actually contains the trusted operation.

I must note I disagree with the characterization here - quoted "proper use" and
all that, as if it's some arbitrary party sanction. It's as if what's now in
std.file is okay but not by the party line. No, it's not okay at all! It takes
all of a few seconds to see it for what it is - an emperor's clothes issue.

> For the most part,
> trusted functions tend to only have a small core that's actually trusted,
> and everything else really ought to be enforced to be @safe. When reviewing
> the function, you want to only look at the relevant part, instead of reading
> the whole thing, otherwise nobody would bother reviewing any @trusted code,
> since it's too much work. Which defeats the purpose of @trusted.

Trusted functions must be themselves small and cohesive enough to be verifiable
manually.

> Furthermore, if the entire function body is @trusted, then any arbitrary
> change can introduce more unsafe operations to the function without any
> warning, and subtle bugs can creep into the code this way.

As said above that can easily happen today, and it's a lot worse because it's
all under the pretense some safety is being enforced.

> This is the
> original rationale for factoring these one-liners into @trusted local
> functions -- that forces the main function body to conform to @safe
> requirements, and only those small core bits that actually need to be
> @trusted are marked as @trusted. Or, put another way, the "dung" construct
> you're railing against is a way of pointing out to reviewers "this is the
> bit that requires @trusted, everything else is @safe and you don't have to
> worry about that". It's a way of making the part that needs @trusted stand
> out.

Yes, that all is completely wrong.

> If this isn't "proper use" of @trusted, then fine, but we *do* need *some*
> way of restricting the scope of what's allowed in a @trusted function,
> especially in a large collaborative project like Phobos.

I think we're good as we are. I'm actually in wonder how such amazingly
corrupted use resulted from notions that are simple and easy to use. I don't
see how making matters more complicated would improve the state of affairs.

> Right now, marking
> the entire function @trusted is basically a free-for-all license that
> whatever garbage code you wish to throw into this function, you can do it
> and @safe code will still continue to work (even though it shouldn't).

Apparently garbage code can be introduced easier in @safe functions by means of
@trusted local functions that are not trustworthy at all. It's a net negative -
instead of seeing @trusted on the function and thinking, "alright let's take a
look", I see @safe at the top and nicely get lulled in a sense of security,
just to see a few lines down that the function affords itself any amount of
unsafety by means of @trusted functions impossible to trust. It's just amazing.

> I
> mean, come on, if there's a large PR that touches 50 files and makes 5
> changes in several 500-line functions, are you seriously expecting reviewers
> to have to read the entire 500 lines *and* understand all of its subtleties,
> just because it's @trusted? Most likely what will happen is that unsafe code
> will slip in without anyone noticing, and before you know it, the function
> is completely unsafe yet it's still marked @trusted. For this reason,
> @trusted needs to be kept as restricted as possible.

Please understand you're arguing for something much worse.

--