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

Walter Bright <bugzilla@digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bugzilla@digitalmars.com

--- Comment #10 from Walter Bright <bugzilla@digitalmars.com> ---
Consider the following code:

    @trusted void* trustedMalloc(size_t n) { return malloc(n); }
    @trusted void trustedFree(void* p) { free(p); }

    @safe void foo() {
        auto p = trustedMalloc(5);
        trustedFree(p);
        trustedFree(p);
    }

foo() passes @safe checks, yet is able to corrupt memory. The fault is that the @trusted functions failed to encapsulate what they're doing and present a safe interface.

@trusted functions must be reviewed to determine if they present a safe interface or not. Merely wrapping an unsafe operation is not good enough and must not pass review.

--

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

--- Comment #11 from Dicebot <public@dicebot.lv> ---
(In reply to Andrei Alexandrescu from comment #5)
> 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!

I don't care how long it takes to read that crap. What is important is that if anyone later will add some @system line to that function by an accident, compiler will immediately complain about it.

Correct comes first and pretty comes later. If you want that code to be more pretty, please forward this issue to DMD developers and show them how hardly usable @safe is sometimes in practice because compiler is simply not clever enough to reason that something is @safe (for example, declaring _all_ casts @system is clearly an overkill)

We may move some of @trusted to actual core.stdc signatures, that is true. But once you start marking whole functions (even 4 lines long) as @trusted, you immediately destroy all trust in @trusted (pun intended) and becomes a non-feature.

I completely refuse to accept your suggested coding style on this matter.

--

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

--- Comment #12 from Dicebot <public@dicebot.lv> ---
(In reply to Walter Bright from comment #10)
> Consider the following code:
> 
>     @trusted void* trustedMalloc(size_t n) { return malloc(n); }
>     @trusted void trustedFree(void* p) { free(p); }
> 
>     @safe void foo() {
>         auto p = trustedMalloc(5);
>         trustedFree(p);
>         trustedFree(p);
>     }
> 
> foo() passes @safe checks, yet is able to corrupt memory. The fault is that the @trusted functions failed to encapsulate what they're doing and present a safe interface.
> 
> @trusted functions must be reviewed to determine if they present a safe interface or not. Merely wrapping an unsafe operation is not good enough and must not pass review.

This is why such wrapper functions are wlays kept private and as long as possible - local to functions those are used in.

It would help a lot if `() @trusted { foo(); }` lambdas could be 100% inlined -
then those could be used instead to prevent accidentla reusage of wrapper in
wrong context.

--

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

--- Comment #13 from Andrei Alexandrescu <andrei@erdani.com> ---
(In reply to Dicebot from comment #11)
> I completely refuse to accept your suggested coding style on this matter.

I'm sorry we've reached irreducible positions on this. I'm not suggesting - we need to fix this.

--

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

--- Comment #14 from Dicebot <public@dicebot.lv> ---
Do as you wish. I have removed myself from Phobos developer list due to inability to comply to demanded policies.

--

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

--- Comment #15 from Andrei Alexandrescu <andrei@erdani.com> ---
(In reply to Dicebot from comment #14)
> Do as you wish. I have removed myself from Phobos developer list due to inability to comply to demanded policies.

If this is that important to you, understood. I trust you do what you think is best, as I do.

--

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

David Nadlinger <code@klickverbot.at> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |code@klickverbot.at

--- Comment #16 from David Nadlinger <code@klickverbot.at> ---
(In reply to Dicebot from comment #12)
> It would help a lot if `() @trusted { foo(); }` lambdas could be 100%
> inlined - then those could be used instead to prevent accidentla reusage of
> wrapper in wrong context.

This. Solves the visual noise problem while keeping the good parts (only mark trusted what really needs to be).

In fact this has already been brought up in the "@trusted considered harmful" thread I started a couple of years (?) ago.

--

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

--- Comment #17 from David Nadlinger <code@klickverbot.at> ---
(In reply to Walter Bright from comment #10)
> @trusted functions must be reviewed to determine if they present a safe interface or not. Merely wrapping an unsafe operation is not good enough and must not pass review.

This isn't relevant to the issue discussed here. The helper functions in std.file are local to their callers.

While I agree that the code looks odd and a visually less noisy alternative is to be preferred (e.g. lambdas), no @safe-ty issues are exposed to client code.

--

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

--- Comment #18 from Andrei Alexandrescu <andrei@erdani.com> ---
(In reply to David Nadlinger from comment #16)
> (In reply to Dicebot from comment #12)
> > It would help a lot if `() @trusted { foo(); }` lambdas could be 100%
> > inlined - then those could be used instead to prevent accidentla reusage of
> > wrapper in wrong context.
> 
> This. Solves the visual noise problem while keeping the good parts (only mark trusted what really needs to be).
> 
> In fact this has already been brought up in the "@trusted considered harmful" thread I started a couple of years (?) ago.

Clearly there has got to be reason and good judgment to this all. We can't possibly decorate every other line of code with `() @trusted { ... }` and feel good about it. `@trusted` means "verified by hand to be safe", and planting it for arbitrary and arbitrarily small unsafe operations (such as pointer additions) hinders proving, doesn't help it.

Any nice paint can be applied too thickly, and this is no exception.

--

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

--- Comment #19 from Andrei Alexandrescu <andrei@erdani.com> ---
(In reply to David Nadlinger from comment #17)
> (In reply to Walter Bright from comment #10)
> > @trusted functions must be reviewed to determine if they present a safe interface or not. Merely wrapping an unsafe operation is not good enough and must not pass review.
> 
> This isn't relevant to the issue discussed here. The helper functions in std.file are local to their callers.

I find it relevant - abundant small @trusted functions that are obviously not safe unless the larger context is known is undesirable.

> While I agree that the code looks odd and a visually less noisy alternative is to be preferred (e.g. lambdas), no @safe-ty issues are exposed to client code.

Again: my point here is putting our heads together and doing some good judgment. It is strikingly obvious that the application of @trusted to std.file has transgressed into a monster of complexity.

--