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

hsteoh@quickfur.ath.cx changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hsteoh@quickfur.ath.cx

--- Comment #1 from hsteoh@quickfur.ath.cx ---
I didn't look closely at std.file, but I think the consensus over the past several months is that @trusted should not be used if avoidable, and if it cannot be avoided, it should be limited to small chunks of code, usually factored out as small lambdas in the main function body which is marked @safe. Example:

-----
auto mySafeFunc(T...)(T args) @safe {
    int trustedHelper(int x) @trusted {
        // Here, it should be explained exactly why the particular value(s) of
        // x here will never trigger un-@safe behaviour in the following call:
        return potentiallyDangerousFunc(x);
    }
    doStuff(...);
    auto x = trustedHelper(...);
    doMoreStuff();
    return result;
}
-----

The idea is that doStuff() and doMoreStuff() can be non-trivial, convoluted code, that we don't want to be reviewing every single time we review usage of @trusted; the actually-trusted bit of code should be confined to trustedHelper(), which is small enough that should the need arise, we can review it within a reasonable amount of time.

I don't know if this is the best way to do it, but this particular construct is quickly becoming an idiom in Phobos.

--

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

Dicebot <public@dicebot.lv> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |public@dicebot.lv

--- Comment #2 from Dicebot <public@dicebot.lv> ---
Andrei, you may have been too quick to give a judgement without having a close look what actually happens there.

All those @trusted functions exist there exact for the purpose of NOT marking everything as @trusted but keeping actual function @safe. Those are used in contexts where operation in general can't be verified to be @safe (and thus C binding can't be marked @trusted itself) but it is verified in specific context where it is called. Wrapping such call in trivial @trusted function allows to keep everything else @safe and enjoy compiler verification.

I do immediately see that some of those posix functions could have been marked as @trusted in extern(C) binding itself - and should probably be removed to reduce visual noise and simplify maintenance.

However, there is nothing fundamentally broken with that code in context of type system and language itself.

--

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

Orvid King <blah38621@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |blah38621@gmail.com

--- Comment #3 from Orvid King <blah38621@gmail.com> ---
I would actually argue that the way it's implemented currently is better than
what you're suggesting.
If I'm understanding correctly, you want std.file.read to be @trusted rather
than @safe.

The way it's implemented currently limits the amount of code in the @trusted section significantly, and the rest is in the @safe version. This is done so that the only parts of the method that need to be @trusted are in @trusted blocks.

--

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

--- Comment #4 from Orvid King <blah38621@gmail.com> ---
I would actually argue that the way it's implemented currently is better than
what you're suggesting.
If I'm understanding correctly, you want std.file.read to be @trusted rather
than @safe.

The way it's implemented currently limits the amount of code in the @trusted section significantly, and the rest is in the @safe version. This is done so that the only parts of the method that need to be @trusted are in @trusted blocks.

--

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

--- Comment #5 from Andrei Alexandrescu <andrei@erdani.com> ---
>I didn't look closely at std.file, but I think the consensus over the past several months is that @trusted should not be used if avoidable, and if it cannot be avoided, it should be limited to small chunks of code, usually factored out as small lambdas in the main function body which is marked @safe
[...]
>I don't know if this is the best way to do it, but this particular construct is quickly becoming an idiom in Phobos.

As I said, there's something to be said about the letter of the "law" and its respective spirit. Anything good can be done in poor taste, and sadly std.file is chock full of things done in perfectly poor taste. We must fix it asap, it's a disaster area. If that is how D is meant to be used I don't want to use it. Look a this pile of dung:

S readText(S = string)(in char[] name) @safe if (isSomeString!S)
{
    import std.utf : validate;
    static auto trustedCast(void[] buf) @trusted { return cast(S)buf; }
    auto result = trustedCast(read(name));
    validate(result);
    return result;
}

How exactly does that scaffolding help a four liner? It takes longer to read all that crap than to actually figure the function is correct!

Or look at this one:

ulong getSize(in char[] name) @safe
{
    version(Windows)
    {
        with (getFileAttributesWin(name))
            return makeUlong(nFileSizeLow, nFileSizeHigh);
    }
    else version(Posix)
    {
        static auto trustedStat(in char[] path, stat_t* buf) @trusted
        {
            return stat(path.tempCString(), buf);
        }
        static stat_t* ptrOfLocalVariable(return ref stat_t buf) @trusted
        {
            return &buf;
        }
        stat_t statbuf = void;
        cenforce(trustedStat(name, ptrOfLocalVariable(statbuf)) == 0, name);
        return statbuf.st_size;
    }
}

How in the world is all that scaffolding help anyone with anything?

There is a place for the spirit. Clearly large unchecked swaths of @trusted code work against quality, but there's a limit to it that std.file went way beyond.

--

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

--- Comment #6 from Andrei Alexandrescu <andrei@erdani.com> ---
>Andrei, you may have been too quick to give a judgement without having a close look what actually happens there.

I think I have a good understanding of what happened: a good intent has been applied in a way that got it thoroughly corrupted.

--

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

--- Comment #7 from Andrei Alexandrescu <andrei@erdani.com> ---
(In reply to Orvid King from comment #4)
> I would actually argue that the way it's implemented currently is better
> than what you're suggesting.
> If I'm understanding correctly, you want std.file.read to be @trusted rather
> than @safe.

Correct.

> The way it's implemented currently limits the amount of code in the @trusted section significantly, and the rest is in the @safe version. This is done so that the only parts of the method that need to be @trusted are in @trusted blocks.

I understand the intent. The moment I need to define dung like:

    static trustedRef(T)(ref T buf) @trusted
    {
        return &buf;
    }

I know there's a problem with the application of the idiom.

--

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

FG <home@fgda.pl> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |home@fgda.pl

--- Comment #8 from FG <home@fgda.pl> ---
"There is nothing to be gained by making each operation @trusted without due verification."  Trust but verify?  :)

@trusted is one thing, but it's sad how little code uses @nogc in both std.file and std.stdio.  Have a look at the latest discussion in http://forum.dlang.org/thread/sxxqrgkzthxzcvuogcpr@forum.dlang.org

--

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

--- Comment #9 from Andrei Alexandrescu <andrei@erdani.com> ---
(In reply to FG from comment #8)
> "There is nothing to be gained by making each operation @trusted without due verification."  Trust but verify?  :)
> 
> @trusted is one thing, but it's sad how little code uses @nogc in both std.file and std.stdio.  Have a look at the latest discussion in http://forum.dlang.org/thread/sxxqrgkzthxzcvuogcpr@forum.dlang.org

Yah, good point but let's not derail this issue though. Thanks.

--

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

Andrei Alexandrescu <andrei@erdani.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Hardware|x86                         |All
                 OS|Mac OS X                    |All

--