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

--- Comment #29 from hsteoh@quickfur.ath.cx ---
One idea that occurred to me (though it may be a bit too late to implement) is that @trusted functions remain under @safe requirements except for @system blocks within the function body, e.g.:

-----
// This is hypothetical syntax, the exact syntax is not important,
// it's the idea behind it.
int myTrustedFunc(int x) @trusted {
    int x = *cast(int*)null; // Compile error: unmarked unsafe operation in
@trusted function
    @system {
        enum magicAddress = 0x900D1DEA;
        int y = *cast(int*)magicAddress; // OK, unsafe operation allowed in
@system block
    }

    free(null); // Compile error: cannot call @system function outside @system
block
    return ...;
}
-----

This way, reviewers know to scrutinize everything inside the @system block, while the code outside is mechanically verified not to introduce more @system operations to the function.

--

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

--- Comment #30 from Andrei Alexandrescu <andrei@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.

--

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

--- Comment #31 from Andrei Alexandrescu <andrei@erdani.com> ---
(In reply to hsteoh from comment #29)
> One idea that occurred to me (though it may be a bit too late to implement) is that @trusted functions remain under @safe requirements except for @system blocks within the function body, e.g.:
> 
> -----
> // This is hypothetical syntax, the exact syntax is not important,
> // it's the idea behind it.
> int myTrustedFunc(int x) @trusted {
>     int x = *cast(int*)null; // Compile error: unmarked unsafe operation in
> @trusted function
>     @system {
>         enum magicAddress = 0x900D1DEA;
>         int y = *cast(int*)magicAddress; // OK, unsafe operation allowed in
> @system block
>     }
> 
>     free(null); // Compile error: cannot call @system function outside
> @system block
>     return ...;
> }
> -----
> 
> This way, reviewers know to scrutinize everything inside the @system block, while the code outside is mechanically verified not to introduce more @system operations to the function.

No, please. Let's not make matters even more complicated and more opened to abuse.

--

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

--- Comment #32 from David Nadlinger <code@klickverbot.at> ---
(In reply to hsteoh from comment #28)
> But still, the nagging issue is, how do we restrict the scope of @trusted inside a function body?

Agreed. This is especially necessary for template functions where the @trusted-ness of the whole function might depend on the template arguments. Many of them *need* something to the effect of what std.file is doing (or () @trusted {} ()) so that they can be @trusted at all, as it would be wrong to apply the attribute at the function level.

Nota bene: Both Go and Rust offer finer granularity than whole functions for marking code as unsafe, the former using special intrinsics, the latter using "unsafe" blocks.

--

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

--- Comment #33 from Walter Bright <bugzilla@digitalmars.com> ---
(In reply to hsteoh from comment #28)
> But still, the nagging issue is, how do we restrict the scope of @trusted inside a function body?

By encapsulating the unsafe part in an @trusted local function. But that function must STILL present a safe interface. Merely wrapping it with "here is an unsafe operation" may fly with other languages, but it is not acceptable in D.

Apply this rule:

"If the compiler cannot verify memory safety in code marked @safe, then you did it wrong."

This code, for example, flunks that rule:

  static trustedCloseHandle(HANDLE hObject) @trusted
  {
    return CloseHandle(hObject);
  }

--

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

--- Comment #34 from Walter Bright <bugzilla@digitalmars.com> ---
This also flunks that rule:

   @safe T* func(T* p) {
       @trusted {
          p += 5;
       }
       return p;
   }

If it is not clear why, please let me know.

--

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

--- Comment #35 from hsteoh@quickfur.ath.cx ---
@Andrei: any @safe function can call a @trusted function that may contain arbitrary unsafe operations. Just because something is marked @safe at the top guarantees nothing.

--

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

--- Comment #36 from David Nadlinger <code@klickverbot.at> ---
(In reply to Walter Bright from comment #33)
> By encapsulating the unsafe part in an @trusted local function. But that function must STILL present a safe interface. Merely wrapping it with "here is an unsafe operation" may fly with other languages, but it is not acceptable in D.

May I suggest that you try to understand the issue at hand instead of repeating the same non-solution over and over again? For a small sampling of code that can't easily be rewritten to follow your demands, check std.array.

(Again, "() @trusted {…}()" would arguably be the better solution, but that's
an inliner problem waiting to be fixed.)

--

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

David Nadlinger <code@klickverbot.at> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|std.file has gotten out of  |@trusted nested helper
                   |hand                        |functions in std.file

--

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

--- Comment #37 from Walter Bright <bugzilla@digitalmars.com> ---
(In reply to hsteoh from comment #35)
> @Andrei: any @safe function can call a @trusted function that may contain arbitrary unsafe operations. Just because something is marked @safe at the top guarantees nothing.

This is a misunderstanding of what @trusted is. It's very important that we clear this up.

Your misunderstanding seems to be that the CALLER of @trusted code must be careful to use it safely. This is incorrect. @trusted code needs to be reviewable for safety by ONLY looking at the @trusted code body. NOT the way the @trusted code is used. For example:

  @trusted void foo() {
    auto p = malloc(3);
    free(p);
  }

is correct use of trust. The following is incorrect:

  @trusted void* tmalloc(size_t n) { return malloc(n); }
  @trusted void tfree(void* p) { return free(p);

  @safe void foo() {
    auto p = tmalloc(3);
    tfree(p);
  }

Make sense?

--