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

--- Comment #20 from hsteoh@quickfur.ath.cx ---
It seems that there's a disconnect here between @trusted as marking a trusted *interface* (i.e., behind the interface is code that's potentially dangerous, but you cannot trigger the dangerous behaviour using that interface), and @trusted as marking a potentially dangerous *operation* that is actually safe because the surrounding context ensures that it is never used in an unsafe way.

The former is for presenting an interface to the user that we can verify won't trigger unsafe behaviour; the latter is for preventing accidental breakage of the trusted code by careless change. What Walter said pertains to the former; what Dicebot said pertains to the latter.

Neither are directly related to syntax, though, which is apparently what provoked this backlash. If it's the syntax that's the problem, then we should be looking at dedicated syntax for @trusted blocks inside functions, or always inlining lambdas that are immediately invoked. Putting std.file on the pedestal as the whipping boy isn't actually solving the core issues IMO.

--

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

--- Comment #21 from Andrei Alexandrescu <andrei@erdani.com> ---
(In reply to hsteoh from comment #20)
> It seems that there's a disconnect here between @trusted as marking a trusted *interface* (i.e., behind the interface is code that's potentially dangerous, but you cannot trigger the dangerous behaviour using that interface), and @trusted as marking a potentially dangerous *operation* that is actually safe because the surrounding context ensures that it is never used in an unsafe way.

Well put. If I have a free-for-all trusted address taking at the top of a function:

https://github.com/D-Programming-Language/phobos/blob/master/std/file.d#L198

then what good does that do to anybody?

> The former is for presenting an interface to the user that we can verify won't trigger unsafe behaviour; the latter is for preventing accidental breakage of the trusted code by careless change. What Walter said pertains to the former; what Dicebot said pertains to the latter.
> 
> Neither are directly related to syntax, though, which is apparently what provoked this backlash. If it's the syntax that's the problem, then we should be looking at dedicated syntax for @trusted blocks inside functions, or always inlining lambdas that are immediately invoked.

Syntax is the least of the problems here.

> Putting std.file on
> the pedestal as the whipping boy isn't actually solving the core issues IMO.

I see fixing std.file as an important and urgent matter. Phobos code is how industrial-strength code should be written, and should stand as an example of idiomatic use of D. Right now std.file is anything but a good example to follow.

--

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

--- Comment #22 from Andrei Alexandrescu <andrei@erdani.com> ---
(In reply to Andrei Alexandrescu from comment #21)
> I see fixing std.file as an important and urgent matter. Phobos code is how industrial-strength code should be written, and should stand as an example of idiomatic use of D. Right now std.file is anything but a good example to follow.

To clarify: the larger matter here is fixing the review process to make sure there's enough scrutiny before things happen, not after.

--

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

--- Comment #23 from Walter Bright <bugzilla@digitalmars.com> ---
(In reply to hsteoh from comment #20)
> It seems that there's a disconnect here between @trusted as marking a trusted *interface* (i.e., behind the interface is code that's potentially dangerous, but you cannot trigger the dangerous behaviour using that interface), and @trusted as marking a potentially dangerous *operation* that is actually safe because the surrounding context ensures that it is never used in an unsafe way.

If code marked as @safe is not mechanically checkable as being safe, it must not be marked as safe. It should be marked as trusted. Trusted code is, by definition, checked to be safe by the user. Safe code is, by definition, checked by the compiler.

Hence "and @trusted as marking a potentially dangerous *operation* that
is actually safe because the surrounding context ensures that it is never
used in an unsafe way" is not correct. It would be correctly written as "and
@system as marking a potentially dangerous *operation* that is actually safe
because the surrounding @trusted context ensures that it is never used in an
unsafe way".

--

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

--- Comment #24 from hsteoh@quickfur.ath.cx ---
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. 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. 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.

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. 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.

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. 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).  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.

--

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

hsteoh@quickfur.ath.cx changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |safe

--

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

--- Comment #25 from hsteoh@quickfur.ath.cx ---
@Walter: mechanical verification of @safe is precisely what we're trying to leverage here. Given a large function body with 1 or 2 lines that need manual review, would you rather mark the entire function as @trusted (thus requiring review of the whole function *and* disabling mechanical @safe checks for the other 498 lines of code that the compiler *could* have verified automatically), or would you rather that the function is marked as @safe -- which imposes mechanical verification of safety -- with the exception of a small @trusted part that needs to be verified by hand?

--

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

--- Comment #26 from Walter Bright <bugzilla@digitalmars.com> ---
@trusted is intended to encapsulate unsafe code, not rely on the caller to call @trusted code in a safe manner.

Simply using @trusted to bypass checks, and rely on supposedly @safe code to then use those @trusted sequences correctly, defeats the purpose of @safe code.

The way @trusted is being used in std.file makes the supposedly @safe code calling it uncheckable by the compiler.

@safe code must be 100% mechanically checkable, not correct by convention.

--

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

--- Comment #27 from Walter Bright <bugzilla@digitalmars.com> ---
For example, Line 213:

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

All this is doing is redefining an @system function as being @trusted, without changing the interface to it, or checking, or doing anything at all to justify it now being trusted.

It may be true that CloseHandle() actually is trustworthy - if that is the case, the declaration of CloseHandle() needs to change from @system to @trusted. On the other hand, if it is not trustworthy, papering it over with such a wrapper and relying on the caller to use it correctly by convention has the effect of totally defeating the compiler's ability to check code.

--

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

--- Comment #28 from hsteoh@quickfur.ath.cx ---
I agree that what's happening with std.file needs to be fixed.

But still, the nagging issue is, how do we restrict the scope of @trusted inside a function body? This is pertinent because your mechanical verification of @safe hinges upon @trusted functions being actually trustworthy -- since @safe code can call @trusted functions no matter how unsafe the latter are, otherwise the mechanical verification proves nothing and is useless.

If I have a @trusted function that only performs 1 or 2 potentially unsafe operations, I'd like to mark those specific operations as needing manual verification, while the rest of the function body ought to be under the scrutiny of the compiler under @safe requirements. The idea is, if somebody else comes along later and changes the code, I don't want him to introduce new unsafe operations to the function body -- at least, not without requiring the use of the same markup that designates said operations as unsafe. In other words, besides those 1 or 2 potentially unsafe operations, I want everything else in the function to be mechanically enforced to conform to @safe requirements. If any new unsafe operations are to be introduced to the function, I want them clearly marked as such so that they can be thoroughly scrutinized before the code change is accepted. Any new code that isn't thus marked ought to be rejected by the compiler outright as a potential breach of trust.

Otherwise, once a function is marked @trusted, a careless code change can easily introduce safety violations without any warning, and then percolate throughout the entire @safe system.

--