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

--- Comment #58 from Andrei Alexandrescu <andrei@erdani.com> ---
(In reply to Dicebot from comment #57)
> You shouldn't consider those wrapper really @trusted - they have _never_ supposed to be ones. It is simply a workaround for bunch of language limitations. I'd love to have something better to address the issue - not regress to worse solution because workaround was not good enough.

We do have something better: @trusted functions that offer safe interfaces with unsafe implementations. This is the right thing to do. This whole fine-granularity of trusted mini-functions adding no safety is a gross abuse of a good notion.

--

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

--- Comment #59 from David Nadlinger <code@klickverbot.at> ---
(In reply to Andrei Alexandrescu from comment #53)
> (In reply to David Nadlinger from comment #51)
> > (In reply to Andrei Alexandrescu from comment #42)
> > > Sigh, std.array has become another disaster area we need to clean.
> > 
> > What about just giving it a try?
> 
> Give what a try? The cleanup?

Yes. Or think about why e.g. marking a template function that iterates over a range @trusted in its entirety would be almost always wrong, at least without explicitly constraining the range regarding @safe-ty of its primitives.

To me, it seems rather obvious that @trusted blocks would be A Good Thing. To you and/or Walter, it seems to be clear that they are The Root of All Evil. I figure it would save us a lot of discussion if you had a look at real-world code where their conceptual equivalent is useful right now.

(Just to be clear: Yes, nested functions that just pretend to be safe are ugly, but immediately invoked delegates or real @trusted blocks aren't an option right now. It's the concept that is important here.)

--

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

--- Comment #60 from Dicebot <public@dicebot.lv> ---
(In reply to Andrei Alexandrescu from comment #58)
> (In reply to Dicebot from comment #57)
> > You shouldn't consider those wrapper really @trusted - they have _never_ supposed to be ones. It is simply a workaround for bunch of language limitations. I'd love to have something better to address the issue - not regress to worse solution because workaround was not good enough.
> 
> We do have something better: @trusted functions that offer safe interfaces with unsafe implementations. This is the right thing to do. This whole fine-granularity of trusted mini-functions adding no safety is a gross abuse of a good notion.

"those aren't the droids you're looking for" approach does not work on me, sorry.

@trusted functions longer than few lines are inherently unmaintainable unless implementation is supposed to be read-only (like with bindings). Saying "no, you don't really have the problem" doesn't make it go away. Especially when I have already tried approach you suggest and experienced how it fails in practice.

--

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

--- Comment #61 from Andrei Alexandrescu <andrei@erdani.com> ---
(In reply to David Nadlinger from comment #59)
> (In reply to Andrei Alexandrescu from comment #53)
> > (In reply to David Nadlinger from comment #51)
> > > (In reply to Andrei Alexandrescu from comment #42)
> > > > Sigh, std.array has become another disaster area we need to clean.
> > > 
> > > What about just giving it a try?
> > 
> > Give what a try? The cleanup?
> 
> Yes. Or think about why e.g. marking a template function that iterates over a range @trusted in its entirety would be almost always wrong, at least without explicitly constraining the range regarding @safe-ty of its primitives.
> 
> To me, it seems rather obvious that @trusted blocks would be A Good Thing. To you and/or Walter, it seems to be clear that they are The Root of All Evil.

I just think they'd be more open to misuse and abuse. I do agree they can be used well, too.

> I figure it would save us a lot of discussion if you had a look at real-world code where their conceptual equivalent is useful right now.

Well I just did - std.file does count for real-world code. It's just not right.

> (Just to be clear: Yes, nested functions that just pretend to be safe are ugly, but immediately invoked delegates or real @trusted blocks aren't an option right now. It's the concept that is important here.)

The concept is: best use of @trusted is to mark a safe function that cannot be automatically proven @safe. All that wrapping realloc as trustedRealloc and then cheering that we got safe code going is nonsense.

--

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

--- Comment #62 from Andrei Alexandrescu <andrei@erdani.com> ---
(In reply to Dicebot from comment #60)
> (In reply to Andrei Alexandrescu from comment #58)
> > (In reply to Dicebot from comment #57)
> > > You shouldn't consider those wrapper really @trusted - they have _never_ supposed to be ones. It is simply a workaround for bunch of language limitations. I'd love to have something better to address the issue - not regress to worse solution because workaround was not good enough.
> > 
> > We do have something better: @trusted functions that offer safe interfaces with unsafe implementations. This is the right thing to do. This whole fine-granularity of trusted mini-functions adding no safety is a gross abuse of a good notion.
> 
> "those aren't the droids you're looking for" approach does not work on me, sorry.

I don't see how this is pushing your argument further. Make a technical point and it'll be well appreciated.

> @trusted functions longer than few lines are inherently unmaintainable unless implementation is supposed to be read-only (like with bindings). Saying "no, you don't really have the problem" doesn't make it go away. Especially when I have already tried approach you suggest and experienced how it fails in practice.

I see the good code in std.file has gone bonkers.

--

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

--- Comment #63 from Walter Bright <bugzilla@digitalmars.com> ---
(In reply to David Nadlinger from comment #49)
> No. I want the "...safe code...", which is in fact "...code that might be safe depending on the template arguments...", to trigger attribute inference as usual, while the compiler should trust me on the potentially-unsafe-but-manually-verified part (such as the cast, or a temporary allocation, etc.).

There's simply no way that:

    static U trustedCast(U, V)(V v) @trusted { return cast(U) v; }

can pass muster as trusted code. Trusted code must provide a safe interface, and that template does not. For example, it could be used to convert ints to pointers. The trusted part needs to go up a level in the logic.

Looking at join(), it is used thus:

    static U trustedCast(U, V)(V v) @trusted { return cast(U) v; }
    return trustedCast!RetType(result);

To fix this, I'd start with a rewrite to:

    static RetType myCast(typeof(result) result) { return cast(RetType)result;
}

That isn't right, either, but at least it is a step forwards in being constrained to only have to deal with two types instead of any types. I.e. we must not allow converting ints to pointers, or pointers to ints to pointers to doubles, etc.

I.e. in order to be safe, result must be safely convertible to RetType. Where's the check for that? If I look upcode, I see:

    auto result = uninitializedArray!(RetTypeElement[])(length);

Whoa! We have result[] being an array of garbage, which is then pretended to be safely convertible to perhaps an array of pointers? Examining the code further, we see an array to initialize result[].

How do we know that every member of result[] is initialized exactly once? Examining the code, I see that we are TRUSTING that code to do the right thing. I.e. the whole thing is, literally, @trusted code. Pretending to infer it as @safe is wrong, it is not @safe code, and not checkable as @safe code. It's @trusted.

Also, uninitializedArray() is marked as @trusted. Returning arrays of pointers
that consist of unknown garbage cannot be @trusted. uninitializedArray() isn't
any more trustworthy than malloc(), and should be marked as @system.

Given the state of this code I worry that the entire std.array needs to be reviewed for memory safety. The apparent necessity of:

    static U trustedCast(U, V)(V v) @trusted { return cast(U) v; }

looks like the canary died trying to tell the programmer something was wrong, and got fed to the cat instead of fixed.

I am not saying that std.array is broken as far as the users of it are concerned. I am saying that it is broken in that it sidesteps the language's attempts to check safety, and serves as a wrong example on how to use D's features to write memory safe code. (uninitializedArray() is definitely broken, however.)

It reminds me of the early days of D2 where people were arguing that the compiler was too strict in its const checking. I believe we finally did reach consensus that it was doing the right thing, and I hope history will repeat itself :-)

Yes, some strong words in this message. Makes me hope I didn't make a mistake - but surely if I did it'll get pointed out :-) en garde.

--

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

--- Comment #64 from Andrei Alexandrescu <andrei@erdani.com> ---
First step: https://github.com/D-Programming-Language/phobos/pull/2963

--

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

--- Comment #65 from Dicebot <public@dicebot.lv> ---
(In reply to Andrei Alexandrescu from comment #62)
> I don't see how this is pushing your argument further. Make a technical point and it'll be well appreciated.

Big chunks of @trusted are maintenance disaster. @trusted completely disabled all compiler safety verification and because of that every time a single line changes in @trusted function it must be completely re-evaluated as a whole during the review to ensure that safety promise stays. Situation becomes worse because compiler is very conservative in what it considers reliably @safe and thus amount of necessary@trusted functions is very big. Combined, that makes using @trusted in a way you propose so impractical that resource-wise it is more effective to completely ignore @safe feature at all - it does not provide enough benefits for such investment.

Does that sounds technical enough?

> > @trusted functions longer than few lines are inherently unmaintainable unless implementation is supposed to be read-only (like with bindings). Saying "no, you don't really have the problem" doesn't make it go away. Especially when I have already tried approach you suggest and experienced how it fails in practice.
> 
> I see the good code in std.file has gone bonkers.

I hope _this_ is not your understanding of a "technical point" ;)

--

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

--- Comment #66 from Dicebot <public@dicebot.lv> ---
Thanks, Walter! This comment alone was more useful and meaningful than rest of this thread combined :)

Let me use example from the very same std.file to oppose:

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;
}

There are two important bits here:

1) This is verified to be safe only if call to `validate` is safe. Making it all @trusted would allow to change `validate` to @system and silently break the promise.

2) Cast is restricted to string types. Pretty much only way it can go wrong is if function signature changes to allow other types - it is effectively @safe function that compiler fails to identify as one.

Wrapper is marked @trusted incorrectly but that is intentional. @trusted here is not used to tell function can actually be trusted but to keep everything else plain @safe.

Alternative solution is to invert it:

S readText(S = string)(in char[] name) @trusted if (isSomeString!S)
{
    () @safe {
        import std.utf : validate;
        auto s = read(name);
    } ();

    auto result = cast(S) s;

    () @safe {
        validate(result);
    } ();

    return result;
}

Which also solves same problem but it result in creation of much more wrappers and makes reading the source even harder (even despite the fact I cheated here and used lambdas instead of nested functions).

--

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

--- Comment #67 from Andrei Alexandrescu <andrei@erdani.com> ---
(In reply to Dicebot from comment #65)
> (In reply to Andrei Alexandrescu from comment #62)
> > I don't see how this is pushing your argument further. Make a technical point and it'll be well appreciated.
> 
> Big chunks of @trusted are maintenance disaster.

Agreed.

> @trusted completely
> disabled all compiler safety verification and because of that every time a
> single line changes in @trusted function it must be completely re-evaluated
> as a whole during the review to ensure that safety promise stays.

I understand. To summarize: there is merit in marking a function as @safe even though it has a few non-safe portions because the compiler can at least verify those non-safe portions. So, the argument goes, we get a bit more interstitial verification.

That's also the case with functions such as trustedOpen, trustedFstat, trustedRead, trustedRealloc, trustedPtrAdd (urgh!), trustedPtrSlicing. Any code added may use them and needs to be checked for safety. I hope this is understood as well.

> Situation
> becomes worse because compiler is very conservative in what it considers
> reliably @safe and thus amount of necessary@trusted functions is very big.
> Combined, that makes using @trusted in a way you propose so impractical that
> resource-wise it is more effective to completely ignore @safe feature at all
> - it does not provide enough benefits for such investment.
> 
> Does that sounds technical enough?

Yes, thanks. It is also not convincing, as it seems my arguments are not convincing to you.

Again it is getting clear we are reaching irreducible positions on this so arguing the point is not productive. New arguments might change this, so feel free to bring any fresh perspective on the subject. I believe I have a good understanding of your current points.

This must go one way or another; it can't go both ways.

If you were responsible I trust you would do what you think is best over my unconvincing arguments, and let me decide how to handle your decision in good form. As things are Walter and I must do what we believe is best (starting with https://github.com/D-Programming-Language/phobos/pull/2963) and I trust you will handle our decision in good form. Thank you.

--