[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 Iain Buclaw changed: What|Removed |Added Priority|P1 |P4 --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 --- Comment #80 from Walter Bright --- (In reply to Walter Bright from comment #79) > This pull request: > > https://github.com/dlang/phobos/pull/4786 has been pulled. --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 --- Comment #79 from Walter Bright --- This pull request: https://github.com/dlang/phobos/pull/4786 will help simplify things in std.file by making some of the machinations to work with both arrays and ranges unnecessary. --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 Denis Shelomovskij changed: What|Removed |Added CC||verylonglogin@gmail.com --- Comment #78 from Denis Shelomovskij --- One more argument to the issue: Ugly paradigms we show for our library code readers. And yes, this does matter, as when a person starts with the language he may start from reading its library code (that's exactly what I did). And when you (a newbie) see all these wrappers around system API you starting to ask yourself: is it that "easy way to communicate with C that was stated as a language advantage?" P.S. I tried to start this discussion half a year ago [1] but looks like nobody noticed. I'm happy the problem is finally stated. [1] https://github.com/D-Programming-Language/phobos/pull/2465#issuecomment-53950146 --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 --- Comment #77 from Andrei Alexandrescu --- Thanks Steve for the nice summary! --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125
--- Comment #76 from Steven Schveighoffer ---
After reading this quite long thread (and I'm not going to CC myself because of
this), I want to summarize the viewpoints here. I think I see that everyone is
in slight agreement, but does not realize the other's *complete* viewpoint.
1. We all agree that a public function such as:
@trusted auto tmalloc(size_t x) {return malloc(x);}
Is no good, should never pass review.
2. There are occasions where inside a very lengthy function one wishes to be
called from @safe code, that a small portion of code which uses unsafe
functions, but in a safe manner, is needed. One would wish that in such a
function all the actual @safe calls are still checked by the compiler, and any
additions to the @safe portion should be checked for future maintainers.
3. A static nested function inside such a function is a license to commit sin
at will. In other words, such a function allows abuse by current and future
maintainers of that function. Therefore, even nested functions like the one in
point 1 above should probably be rejected. However, @trusted static nested
functions that do NOT leak memory details can be allowed.
4. A possible restriction (but not bulletproof) is to use an immediately-called
lambda to execute a trusted call. The issue here is that it won't be inlined.
Hence the desire to use static nested functions.
5. There is a general issue that adding valid @trusted calls inside a @safe
function is simply allowed in review without too much consideration for what it
means for future maintenance.
-
Now, point 4 is not bullet proof. Walter brought up a case where it is not, and
it's not hard to see it can be abused. When a @trusted call affects variables
inside a @safe function, it can mean that depending on the calls, any code that
then *uses* those variables, even in a safe way, is suspect. This means that
@trusted must extend to all those uses. Such a position makes for an incorrect
assumption about the entire function.
However, I think even worse is the idea that we add a static callable @trusted
function nested inside a @safe function, that can be called in an unsafe way.
Such a function cannot guarantee calls to it are safe. A lambda is much better
because review of its uses involves one line. But only if its details do not
leak!
So I would say, as a review requirement, we should limit internal @trusted
calls ONLY to lambdas, unless the static nested function does not leak memory
details. And inside those lambdas, the code should not affect any variables in
@safe code in a way that has the potential for abuse. I think this is doable,
because you can ask that question objectively by reading the one call. It
doesn't matter if code *doesn't* use that variable in an un@safe way, just the
*potential* of using it is enough to cause the review to fail.
There is the issue with not inlining lambdas. Tough. Get that fixed, it's not
worth corrupting memory to save some performance.
That's my $.02. I'll see about reviewing std.file.read and try and apply this
methodology to fix the issues.
--
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 John Colvin changed: What|Removed |Added CC||john.loughran.colvin@gmail. ||com --- Comment #75 from John Colvin --- Walter and Andrei are completely right here. If you can't factor out the @system code to a function providing a truly safe interface (marked with @trusted), then the code clearly depends on its surrounding context to make it safe. So *all* of that code needs to be manually verified with the same scrutiny, together with the core bit that actually appeared to be @system. It becoming a maintenance nightmare is just unveiling the true difficulty of safely using @system code, as opposed to papering over it. It might become good practice in robust @trusted code to add static asserts to ensure that changes to @system (explicit or inferred) further down the call tree aren't accidentally missed. static assert(isTrusted!func) or similar. --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125
--- Comment #74 from Walter Bright ---
(In reply to hsteoh from comment #68)
> Then there's Andrei's PR that contains a 50+-line @trusted function. I had a
> headache trying to figure out whether it was truly @trusted -- it's wa
> too big to be adequately reviewed in a satisfactory way. And this review
> effort has to be incurred every single time somebody so much as touches a
> single line in that function, because @trusted disables all compiler safety
> checks and therefore we can never be too sure whether the change has
> introduced subtle breakage in safety.
See my review of std.array.join() upthread. The idea that one could mark a
function with an unsafe interface as @trusted and thereby avoid having to check
the rest of the code for safety is destroyed by that example. The rest was
correct, but could not be checked for safety by the compiler. It had to be
reviewed. The @trusted workaround only provided an illusion of safety. No work
was avoided - extra work is caused because I no longer trust the code in
std.array and it all needs reviewing again.
I propose a review rule - functions of the following patterns will be
automatically rejected by reviewers:
1. any function that trivially wraps a call to an @system function so it can be
marked as @trusted. For example:
@trusted void* trustedMalloc(size_t n) { return malloc(n); }
2. any function that is a trivial wrapper around an operation that would
otherwise be flagged as unsafe, for example:
@trusted void* incPtr(void* p) { return p + 1; }
--
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 --- Comment #73 from Andrei Alexandrescu --- (In reply to David Nadlinger from comment #71) > (In reply to Andrei Alexandrescu from comment #61) > > Well I just did - std.file does count for real-world code. It's just not > > right. > > Hold on a second. I'm suggesting that trying to clean up the std.array > functions might lead to an insight or two (interplay between the need for > @system code and template attribute inference), and you reply by claiming > that std.file is bad code? Prolly some misunderstanding in the going back and forth. > I never tried to convince you that all real-world code would unambiguously > benefit from finer-grained @trusted-ness. But there is a good amount of code > where, as I wrote earlier, "[its] conceptual equivalent is useful right > now". Generic range algorithms that do some @system optimizations > internally, such as std.array, are an example. > > This discussion would be much easier if you didn't assume that I don't know > what I'm talking about. How in the world did you acquire that idea? > In fact, I'm rather confident that I have a better > understanding of @safe-ty than most other people. :) This is fantastic. I think the right way to go here is to leverage your good understanding in finding better solutions. It takes me all of three seconds to look at std.file.read and figure out that's definitely a bad way to go about things. > You are just not seeing > a part of the problem that is there, right now, in real world generic code. Well here I'm again counting on your excellent understanding of the topic for good explanations and proposed solutions. I'm not being sarcastic. If you have a good grasp over the matter at hand, there must be a way you can prove that by concrete proposals and solutions, as in DIPs and code and great pull requests (as opposed to relativelt unproductive general conversation). Even though I am not as creative as you, I'm pretty sure I can identify a brilliant solution when I see it. I think you'd agree the code that https://github.com/D-Programming-Language/phobos/pull/2963 fixes is quite the opposite of it. Please do come with something better. > Whatever the solution to this issue will be—nested functions as currently > used in parts of Phobos, @trusted lambdas, @trusted blocks or something else > entirely—, discussing design decisions doesn't make a lot of sense without a > good grasp of the problem domain. > > I'm rather certain that you calling std.array a disaster area is a > consequence of that. So, please consider how the need for attribute > inference might influence the picture before we discuss this any further. > Once we agree on a non-disastrous solution for the cases where the safety of > a function depends on a template parameter we can discuss how we might or > might not want to apply it as a general coding guideline in situations like > std.file. For generic functions safety would be deduced (which is indeed an area that needs improvement - a very high-impact potential). There is of course the possibility of forking the implementations by using template constraints and then using different attributes for the forks. --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 --- Comment #72 from Andrei Alexandrescu --- (In reply to hsteoh from comment #68) > Then there's Andrei's PR that contains a 50+-line @trusted function. I had a > headache trying to figure out whether it was truly @trusted -- it's wa > too big to be adequately reviewed in a satisfactory way. 50 lines is very little code, and code that interfaces with low-level libraries and optimizes the bejesus out of system calls and memory allocations - obviously there must be some manual verification going on, and I don't see it as onerous. It also replaces code that is in no way more provably safe because it uses escape hatches literally almost at every line! How is that drivel better when it has functions that allow escaping addresses and adding pointers and all that stuff? I am sorry I am not buying this line of argumentation for one second. https://github.com/D-Programming-Language/phobos/pull/2963 replaces utter drivel with sensible code. --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 --- Comment #71 from David Nadlinger --- (In reply to Andrei Alexandrescu from comment #61) > Well I just did - std.file does count for real-world code. It's just not > right. Hold on a second. I'm suggesting that trying to clean up the std.array functions might lead to an insight or two (interplay between the need for @system code and template attribute inference), and you reply by claiming that std.file is bad code? I never tried to convince you that all real-world code would unambiguously benefit from finer-grained @trusted-ness. But there is a good amount of code where, as I wrote earlier, "[its] conceptual equivalent is useful right now". Generic range algorithms that do some @system optimizations internally, such as std.array, are an example. This discussion would be much easier if you didn't assume that I don't know what I'm talking about. In fact, I'm rather confident that I have a better understanding of @safe-ty than most other people. :) You are just not seeing a part of the problem that is there, right now, in real world generic code. Whatever the solution to this issue will be—nested functions as currently used in parts of Phobos, @trusted lambdas, @trusted blocks or something else entirely—, discussing design decisions doesn't make a lot of sense without a good grasp of the problem domain. I'm rather certain that you calling std.array a disaster area is a consequence of that. So, please consider how the need for attribute inference might influence the picture before we discuss this any further. Once we agree on a non-disastrous solution for the cases where the safety of a function depends on a template parameter we can discuss how we might or might not want to apply it as a general coding guideline in situations like std.file. --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 --- Comment #70 from Andrei Alexandrescu --- Loosely related: https://github.com/D-Programming-Language/druntime/pull/1157. That and more like it is definitely good to do. --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 --- Comment #69 from Dicebot --- Sure, I don't hold any bad feeling about it - won't be the first time something very uncomfortable for me happens in language upstream :) Only thing I wanted to clarify is that existing code is not written that way out of ignorance or reviewer sloppinness - it was an attempt of active Phobos reviewers to address a specific problem we encountered. Having a different vision of how this needs to be designed is OK, but original NG thread implied we did our "job" badly and that wasn't really true. --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125
--- Comment #68 from hst...@quickfur.ath.cx ---
(In reply to Dicebot from comment #66)
[...]
> 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.
I think this is precisely what Walter & Andrei are objecting against.
> 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).
This would be the pedantically-correct version of the code. However, it is even
worse in readability than the current code.
Then there's Andrei's PR that contains a 50+-line @trusted function. I had a
headache trying to figure out whether it was truly @trusted -- it's wa too
big to be adequately reviewed in a satisfactory way. And this review effort has
to be incurred every single time somebody so much as touches a single line in
that function, because @trusted disables all compiler safety checks and
therefore we can never be too sure whether the change has introduced subtle
breakage in safety.
Not to mention, if any line in the 50+-line function is a call to another
function, then every time any one of *those* functions change (e.g., they were
@safe before but now an unrelated PR has made them @system), you have to
re-review the whole 50 lines again. Plus ALL other @trusted functions that may
have called those functions. This is clearly impractical beyond trivial
textbook examples.
I would truly love to know how Walter/Andrei propose to solve this maintenance
nightmare, since I'm clearly missing something that's blatantly obvious to
them.
--
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 --- Comment #67 from Andrei Alexandrescu --- (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. --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125
--- Comment #66 from Dicebot ---
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).
--
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 --- Comment #65 from Dicebot --- (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" ;) --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 --- Comment #64 from Andrei Alexandrescu --- First step: https://github.com/D-Programming-Language/phobos/pull/2963 --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125
--- Comment #63 from Walter Bright ---
(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.
--
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 --- Comment #62 from Andrei Alexandrescu --- (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. --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 --- Comment #61 from Andrei Alexandrescu --- (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. --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 --- Comment #59 from David Nadlinger --- (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.) --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 --- Comment #60 from Dicebot --- (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. --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 --- Comment #58 from Andrei Alexandrescu --- (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. --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 --- Comment #57 from Dicebot --- 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. --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 --- Comment #56 from Andrei Alexandrescu --- (In reply to Dicebot from comment #55) > I don't see any counterarguments, only trying to explain what @trusted is > suppposed to mean (which I am perfectly aware of). It does not address > "break @trusted by adding code" concern which is a different thing - and > which we were trying to fix by this idiom within available language > limitations. The illusion that @trusted wrappers around essentially unsafe operations make any difference has been shattered. --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 --- Comment #55 from Dicebot --- I don't see any counterarguments, only trying to explain what @trusted is suppposed to mean (which I am perfectly aware of). It does not address "break @trusted by adding code" concern which is a different thing - and which we were trying to fix by this idiom within available language limitations. --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 --- Comment #54 from Andrei Alexandrescu --- (In reply to Dicebot from comment #52) > (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? You might even discover that there actually > > is a language issue to be solved here (assuming that the current situation > > is unacceptable). > > This. > > Judging by this thread there seems to be a huge distance between language > authors and people trying to actually use those language features in > practice. It is hard to communicate when it is simpyl assumed that I don't > understand how @trusted and @safe work (and are supposed to work) instead of > addressing actual real-world problems that we encounter. Do you have any response to the counterarguments made to yours? I think it's rather clear how Walter and I stand, and also where you are wrong. --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 --- Comment #53 from Andrei Alexandrescu --- (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? --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 --- Comment #52 from Dicebot --- (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? You might even discover that there actually > is a language issue to be solved here (assuming that the current situation > is unacceptable). This. Judging by this thread there seems to be a huge distance between language authors and people trying to actually use those language features in practice. It is hard to communicate when it is simpyl assumed that I don't understand how @trusted and @safe work (and are supposed to work) instead of addressing actual real-world problems that we encounter. --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 --- Comment #51 from David Nadlinger --- (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? You might even discover that there actually is a language issue to be solved here (assuming that the current situation is unacceptable). --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 --- Comment #50 from Zach the Mystic --- (In reply to Zach the Mystic from comment #48) > I think it would be funny if a @trusted function with fewer than two > statements was automatically rejected by the compiler. Since an > unverifiable, but @safe operation requires two actions, one to dive into > unsafe territory, and a second one to dive back out, any sound use of > @trusted *must* have two or more statements. Destroy! :-) I'm kidding, but unfortunately, it does seem like @trusted's purpose is never going to be easily communicated. Maybe something in the documentation saying that a @trusted function must have more than one action to be used correctly? --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 --- Comment #49 from David Nadlinger --- (In reply to Walter Bright from comment #44) > I understand you want the ...safe code... to be checked for safety. 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.). The hypothetical @safe blocks won't help there. @trusted blocks would. I seriously don't see how you could view the latter as more dangerous than applying the attribute to the whole function. In fact, the difference might even boil down to just setting STCtrusted on a smaller scope instead of the function scope. There is still a big, red "@trusted" token in the code to catch the reviewer's eye (well, the color obviously depends on your syntax highlighter settings). In fact, I think that @trusted blocks make it easier to write and review high-quality code, as they help to reduce the amount of lines that need to be reviewed. (Of course, having two @trusted blocks in what would otherwise be a three-liner might not be worth it in that regard, as Andrei pointed out.) --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125
Zach the Mystic changed:
What|Removed |Added
CC||reachz...@gmail.com
--- Comment #48 from Zach the Mystic ---
(In reply to Walter Bright from comment #47)
> (In reply to Andrei Alexandrescu from comment #45)
> > That's beautiful. Too bad it doesn't work :o).
>
> I did say I paraphrased it. I've come out against supporting
>
>@trusted { ... code ... }
>
> because it made it too easy to do bad things with it. But I did use it to
> make for a simpler example :-)
I think it would be funny if a @trusted function with fewer than two statements
was automatically rejected by the compiler. Since an unverifiable, but @safe
operation requires two actions, one to dive into unsafe territory, and a second
one to dive back out, any sound use of @trusted *must* have two or more
statements. Destroy! :-)
--
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125
--- Comment #47 from Walter Bright ---
(In reply to Andrei Alexandrescu from comment #45)
> That's beautiful. Too bad it doesn't work :o).
I did say I paraphrased it. I've come out against supporting
@trusted { ... code ... }
because it made it too easy to do bad things with it. But I did use it to make
for a simpler example :-)
--
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125
--- Comment #46 from Walter Bright ---
I was a bit curious where these misunderstandings came from. Turns out,
example code for C# 'unsafe' code presents not only unsafe code, but an unsafe
interface:
https://msdn.microsoft.com/en-us/library/aa288474(v=vs.71).aspx#vcwlkunsafecode_readfileexample
public unsafe int Read(byte[] buffer, int index, int count)
{
int n = 0;
fixed (byte* p = buffer)
{
if (!ReadFile(handle, p + index, count, &n, 0))
return 0;
}
return n;
}
Note that there's no guarantee that index is within the length of buffer[]. The
poor sot cannot simply review Read(), he's got to review EVERY SINGLE CALLER of
read(). Since this is taught as correct usage of 'unsafe', it's something we're
going to have to regularly auger against.
--
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125
--- Comment #45 from Andrei Alexandrescu ---
(In reply to Walter Bright from comment #44)
> I understand you want the ...safe code... to be checked for safety. Here's
> how to do it:
>
> @trusted join() {
> @safe {
> ... safe code ...
> }
> return cast(U) v;
> }
That's beautiful. Too bad it doesn't work :o).
--
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125
--- Comment #44 from Walter Bright ---
(In reply to David Nadlinger from comment #40)
> grep for "trusted[A-Z]". For example, trustedCast() in join().
For reference:
static U trustedCast(U, V)(V v) @trusted { return cast(U) v; }
That's a fine example of what I'm talking about as a complete misunderstanding
of what trust is supposed to be doing. It's putting the entire onus of using
the cast correctly on the CALLER. It is not checkable without checking the
supposedly @safe code that uses it, which has defeated the purpose of @safe.
The use in join() looks like this (paraphrasing):
@safe join() {
... safe code ...
return trustedCast();
}
I understand you want the ...safe code... to be checked for safety. Here's how
to do it:
@trusted join() {
@safe {
... safe code ...
}
return cast(U) v;
}
--
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 --- Comment #43 from Andrei Alexandrescu --- (In reply to David Nadlinger from comment #38) > What is your point? > > @trusted can be used incorrectly. Nothing new and exciting so far. His point is that gainful use of @trusted is for safe interfaces with unsafe implementations, not make-believe that unsafe functions are actually safe. It's a very good point. --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 --- Comment #42 from Andrei Alexandrescu --- (In reply to David Nadlinger from comment #40) > (In reply to Walter Bright from comment #39) > > (In reply to David Nadlinger from comment #36) > > > 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. > > > > std.array is a big module. You'll need to be much more specific. > > grep for "trusted[A-Z]". For example, trustedCast() in join(). Sigh, std.array has become another disaster area we need to clean. --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 --- Comment #41 from Andrei Alexandrescu --- (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. Agreed - and that's exactly why we shouldn't abuse it. --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 --- Comment #40 from David Nadlinger --- (In reply to Walter Bright from comment #39) > (In reply to David Nadlinger from comment #36) > > 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. > > std.array is a big module. You'll need to be much more specific. grep for "trusted[A-Z]". For example, trustedCast() in join(). --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 --- Comment #39 from Walter Bright --- (In reply to David Nadlinger from comment #36) > 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. std.array is a big module. You'll need to be much more specific. --
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125
--- Comment #38 from David Nadlinger ---
(In reply to Walter Bright from comment #34)
> This also flunks that rule:
>
>@safe T* func(T* p) {
>@trusted {
> p += 5;
>}
>return p;
>}
So does this:
---
@trusted void claimsToBeSafe(ref T* p) {
p += 5;
}
@safe T* func(T* p) {
claimsToBeSafe(p);
return p;
}
---
And this:
---
@safe T* func(T* p) {
@trusted void claimsToBeSafe(ref T* p) {
p += 5;
}
claimsToBeSafe(p);
return p;
}
---
What is your point?
@trusted can be used incorrectly. Nothing new and exciting so far.
--
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125
--- Comment #37 from Walter Bright ---
(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?
--
[Issue 14125] @trusted nested helper functions in std.file
https://issues.dlang.org/show_bug.cgi?id=14125 David Nadlinger changed: What|Removed |Added Summary|std.file has gotten out of |@trusted nested helper |hand|functions in std.file --
