View mode: basic / threaded / horizontal-split · Log in · Help
July 28, 2012
@trusted considered harmful
@trusted in its current form needs to go. Its design is badly 
broken, as it leaks implementation details and encourages writing 
unsafe code.

The Problem
———————————

First, there is no point in having @trusted in the function 
signature. Why? From the perspective of the caller of the 
function in question, @safe and @trusted mean exactly the same 
thing. If you are not convinced about that, just consider that 
you can wrap any @trusted function into a @safe function to make 
it @safe, and vice versa.

So the current situation is similar to having two keywords `pure` 
and `pure2` in the language, which are completely equivalent for 
the consumer of an API. This is in itself a problem, since it is 
a pitfall for writing generic code. To stick with the example, 
it's easy to check only for `pure` in a template constraint when 
you really meant to accept both `pure` and `pure2` – and you 
_always_ want to accept both.

But is this alone enough to warrant a change to the language at 
this point? Probably not. But besides that, the current design 
also leads to problems for the implementation side:

One issue is that the distinction unnecessarily restricts the 
implementation in terms of interface stability. Yes, @safe and 
@trusted are equivalent from the caller's perspective, but they 
are mangled differently. This means that changing a function 
signature from one to the other is a breaking change to the ABI, 
and as the mangled name is available in the program (which is 
e.g. what std.traits.FunctionAttributes), also to the API.

Thus, you can't just change @trusted to @safe or vice versa on 
the implementation side if you make changes code which require 
@trusted, resp. cause it to be no longer needed. Sure, you can 
always move the implementation into a new, properly marked 
function, and make the original function just a wrapper around 
it. But this is kludgy at best, and might be unacceptable in the 
case of @safe -> @trusted for performance optimizations, if the 
inliner doesn't kick in. So, the only reasonable choice if you 
want to provide a stable interface in this case is to mark all 
functions which could ever possibly need to access unsafe code as 
@trusted, forgoing all the benefits of automatic safety checking.

But the much bigger problem is that @trusted doesn't play well 
with template attribute inference and makes it much too easy to 
accidentally mark a function as safe to call if it really isn't. 
Both things are a consequence of the fact that it can be applied 
at the function level only; there is no way to apply it 
selectively to only a part of the function.

As an example how this is problematic, consider that you are 
writing a function which takes some generic input data, and needs 
to do (unsafe) low-level buffer handling internally to 
efficiently do its job. You come up with a first implementation, 
maybe only accepting arrays for the sake of getting it working 
quickly, and add @trusted as your dirty buffer magic isn't 
visible from the outside, but does break attribute inference. 
Later, you decide that there is no reason not to take other range 
types as input. Fortunately, the actual implementation doesn't 
require any changes, so you just modify the template constraint 
as needed, and you are good. Well, no – you've just completely 
broken all safety guarantees for every program which calls your 
function, because empty/front/popFront of the passed range might 
be @system.

Now, you might argue that this is a contrived scenario. Yes, the 
mistake could have easily be avoided, @trusted on a template 
declaration should always raise a red flag. But cases like this 
_do_ occur in real-world code, and are easy to miss: The recently 
added std.uuid originally had a similar bug, which went unnoticed 
until during the vote [1] – at that point, a number of people, 
mostly experienced contributors, had reviewed the code. A safety 
system which is easy to break by accident is somewhat of a futile 
exercise.

Can you correctly implement such a template function with today's 
@trusted? Yes, there are workarounds, but it's not quite easy. 
One way is to explicitly detect the @safe-ty of the code accessed 
via template arguments and switch function prototypes using 
static ifs and string mixins to avoid code duplication. For an 
example of this, see Jonathan's new std.range.RefRange [2]. It 
works, but it isn't pretty. The average programmer will, just as 
done in the revised version of std.uuid [3], likely give up and 
accept the fact that the function isn't callable from safe code. 
Which is a pity, as we should really utilize the unique asset we 
got in SafeD to the fullest, but this only works if everything 
that could be @safe is marked as such. The situation won't get 
better as we continue to advocate the use of ranges, either.

To summarize, there are, at least as far as I can see, no 
advantages in distinguishing between @safe and @trusted in 
function signatures, and the function-level granularity of 
@trusted yields to avoidable bugs in real-world code. 
Fortunately, we should be able to resolve both of these issues 
fairly easily, as described below.


A Solution
——————————

Let me make something clear first: I am _not_ intending to remove 
@trusted from the language. As a bridge between the @safe and 
@system worlds, it is an integral part of SafeD. What I'm 
proposing is:

 1) Remove the distinction between @safe and @trusted at the 
interface (ABI, API) level. This implies changing the name 
mangling of @trusted to Nf, and consequently removing the 
distinction in DMD altogether (at least in user-facing parts like 
.stringof and error messages). In theory, this is a breaking 
change, but as any code that doesn't treat them the same is buggy 
anyway, it shouldn't be in practice. As for 
std.traits.FunctionAttribute, we could either make trusted an 
alias for safe, or just remove documentation for the former and 
keep it around for some time (there is no way to deprecate an 
enum member).

 2) The first step is necessary, but mainly of cosmetic nature 
(think `pure`, `pure2`). We still need to address for the 
granularity and attribute inference problem. The obvious solution 
is to add a "@trusted" declaration/block, which would allow 
unsafe code in a certain region. Putting @trusted in the function 
header would still be allowed for backwards compatibility (but 
discouraged), and would have the same effect as marking the 
function @safe and wrapping its whole body in a @trusted block. 
It could e.g. look something like this (the @ prefix definitely 
looks weird, but I didn't want to introduce a new keyword):

---
 void foo(T)(T t) {
   t.doSomething();
   @trusted {
     // Do something dirty.
   }
   t.doSomethingElse();
   @trusted phobosFunctionWhichHasNotBeenMarkedSafeYet();
 }
---

This is similar to other constructs we have today, for example 
debug {}, which allows impure code. It can be debated whether a 
block »argument« should introduce a new scope or not (like 
static if). The latter currently seems much more attractive to 
me, but I suppose it could be confusing for some.

In any case, while there is probably quite a bit of bikeshedding 
to be done for 2), I don't think there is much controversy about 
1). So, let's try to get this done shortly after the 2.060 
release – as discussed above, it is very unlikely that the 
change will break something, but the odds still increase over 
time. Also, there is currently a Phobos pull request [4] which 
will be influenced by the outcome of this discussion.

David



[1] 
http://forum.dlang.org/thread/jrpni1$rt$1@digitalmars.com?page=2#post-pcaaoymspzelodvmnbvc:40forum.dlang.org
[2] 
https://github.com/D-Programming-Language/phobos/blob/c005f334ec34d3b0295d5dbf48212972e17823d0/std/range.d#L7327
[3] 
https://github.com/D-Programming-Language/phobos/blob/c005f334ec34d3b0295d5dbf48212972e17823d0/std/uuid.d#L1193
[4] https://github.com/D-Programming-Language/phobos/pull/675
July 28, 2012
Re: @trusted considered harmful
On Saturday, July 28, 2012 02:08:28 David Nadlinger wrote:
> This is similar to other constructs we have today, for example
> debug {}, which allows impure code. It can be debated whether a
> block »argument« should introduce a new scope or not (like
> static if). The latter currently seems much more attractive to
> me, but I suppose it could be confusing for some.

I'd definitely vote for it _not_ introducing a new scope. It will be much more 
useful that way.

> In any case, while there is probably quite a bit of bikeshedding
> to be done for 2), I don't think there is much controversy about
> 1). So, let's try to get this done shortly after the 2.060
> release – as discussed above, it is very unlikely that the
> change will break something, but the odds still increase over
> time. Also, there is currently a Phobos pull request [4] which
> will be influenced by the outcome of this discussion.

I'm all for this. Templates and @trusted just don't get along very well, which 
has a huge impact on ranges. The only real saving grace there is that 
attribute inferrence solves a lot of the problem if you just never use @safe 
or @trusted on a templated function, but when you have to do @system stuff 
within such a template, it's a real problem (as RefRange shows).

As for the syntax, I think that @trusted{ /+@system code+/ } makes perfect 
sense.

- Jonathan M Davis
July 28, 2012
Re: @trusted considered harmful
On Fri, Jul 27, 2012 at 5:08 PM, David Nadlinger <see@klickverbot.at> wrote:
>  2) The first step is necessary, but mainly of cosmetic nature (think
> `pure`, `pure2`). We still need to address for the granularity and attribute
> inference problem. The obvious solution is to add a "@trusted"
> declaration/block, which would allow unsafe code in a certain region.
> Putting @trusted in the function header would still be allowed for backwards
> compatibility (but discouraged), and would have the same effect as marking
> the function @safe and wrapping its whole body in a @trusted block. It could
> e.g. look something like this (the @ prefix definitely looks weird, but I
> didn't want to introduce a new keyword):
>

Agreed. This is very similar to how Rust works. In Rust all the
functions are assumed to be @safe. Unsafe code can only be performed
in clearly marked blocks. Note: I am not suggesting D should implement
Rust's solution as David already pointed out.

-Jose
July 28, 2012
Re: @trusted considered harmful
On Saturday, 28 July 2012 at 00:08:30 UTC, David Nadlinger wrote:

>  2) [...] The obvious solution is to add a "@trusted" 
> declaration/block, which would allow unsafe code in a certain 
> region. Putting @trusted in the function header would still be 
> allowed for backwards compatibility (but discouraged), and 
> would have the same effect as marking the function @safe and 
> wrapping its whole body in a @trusted block. It could e.g. look 
> something like this (the @ prefix definitely looks weird, but I 
> didn't want to introduce a new keyword):
>
> ---
>  void foo(T)(T t) {
>    t.doSomething();
>    @trusted {
>      // Do something dirty.
>    }
>    t.doSomethingElse();
>    @trusted phobosFunctionWhichHasNotBeenMarkedSafeYet();
>  }
> ---

I don't see flaw with 1.

However 2 doesn't sound right.

    @trusted {
      // Do something dirty.
    }

You aren't supposed to do dirty things in @trusted code. You're 
supposed to  safely wrap a system function to be usable by a safe 
function. The system function is supposed to be short and getting 
its hands dirty. Remember this is about memory safety and not 
lack of bugs safety.

The template issue needs fixed, but maybe it is the inference 
which needs expanded? Maybe a template is only inferred as safe 
or trusted and require explicitly system?

I think I was going to say more, but I'm not versed in the 
problems for this area, which I'm sure there are many, so this is 
probably good enough self butchering.
July 28, 2012
Re: @trusted considered harmful
> I don't see flaw with 1.
>
> However 2 doesn't sound right.
>
>     @trusted {
>       // Do something dirty.
>     }
>
> You aren't supposed to do dirty things in @trusted code. You're 
> supposed to  safely wrap a system function to be usable by a 
> safe function. The system function is supposed to be short and 
> getting its hands dirty.

True, but since the proposal is that all functions should be 
either @safe or @system, a @trusted block is necessary in a @safe 
function in order to call @system functions. Perhaps you would 
suggest that a @trusted block should be able to _call_ @system 
code but not actually do anything unsafe directly? That sounds 
interesting, but it's not how @trusted currently works.
July 28, 2012
Re: @trusted considered harmful
On Saturday, July 28, 2012 04:05:14 Jesse Phillips wrote:
> On Saturday, 28 July 2012 at 00:08:30 UTC, David Nadlinger wrote:
> >  2) [...] The obvious solution is to add a "@trusted"
> > 
> > declaration/block, which would allow unsafe code in a certain
> > region. Putting @trusted in the function header would still be
> > allowed for backwards compatibility (but discouraged), and
> > would have the same effect as marking the function @safe and
> > wrapping its whole body in a @trusted block. It could e.g. look
> > something like this (the @ prefix definitely looks weird, but I
> > didn't want to introduce a new keyword):
> > 
> > ---
> > 
> >  void foo(T)(T t) {
> >  
> >    t.doSomething();
> >    @trusted {
> >    
> >      // Do something dirty.
> >    
> >    }
> >    t.doSomethingElse();
> >    @trusted phobosFunctionWhichHasNotBeenMarkedSafeYet();
> >  
> >  }
> > 
> > ---
> 
> I don't see flaw with 1.
> 
> However 2 doesn't sound right.
> 
>      @trusted {
>        // Do something dirty.
>      }
> 
> You aren't supposed to do dirty things in @trusted code. You're
> supposed to  safely wrap a system function to be usable by a safe
> function. The system function is supposed to be short and getting
> its hands dirty. Remember this is about memory safety and not
> lack of bugs safety.

The whole point of @trusted is to mark @system code as being @safe. The 
programmer is certifying that the code is @safe, because they know that what 
it's doing is actually @safe in spite of the fact that the compiler can't 
verify that. It's perfectly acceptable to put "dirty code" in an @trusted 
function. The difference between @trusted and @system is that with @trusted, 
the programmer is guaranteeing that it's actually @safe regardless of what 
arguments it's given, whereas @system is not only doing unsafe things, but 
whether they're ultimately @safe or not depends on arguments and/or other 
code, so the programmer _can't_ guarantee that it's @safe.

> The template issue needs fixed, but maybe it is the inference
> which needs expanded? Maybe a template is only inferred as safe
> or trusted and require explicitly system?
> 
> I think I was going to say more, but I'm not versed in the
> problems for this area, which I'm sure there are many, so this is
> probably good enough self butchering.

The main problem is when you have code that is @system in a templated function 
and while you can guarantee that that code is actually @safe and therefore 
mark it as @trusted, you can't guarantee that for the rest of the function.

With the current situation, you can't mark that function as @trusted, because 
that would be certifying that _everything_ in the function was @safe, which 
isn't necessarily true. Ideally, you would be able to mark the operations that 
are @system but you know are really @safe with @trusted and let whether the 
function as a whole is @safe or @system be inferred by the compiler. But right 
now, you either have to break up that @trusted code into a separate function 
(which isn't always reasonable) or play games with static if and having two 
versions of the same function where one is @safe and the other is @system. For 
instance, in the new std.range.RefRange, save is defined something like this:

private static void _testSave(R)(R* range)
{
   (*range).save;
}

static if(isSafe!(_testSave!R))
{
   @property auto save() @trusted
   {
       mixin(_genSave());
   }
}
else
{
   @property auto save()
   {
       mixin(_genSave());
   }
}

private static string _genSave() @safe pure nothrow
{
   return `import std.conv;` ~
          `alias typeof((*_range).save) S;` ~
          `static assert(isForwardRange!S, S.stringof ~ " is not a forward 
range.");` ~
          `auto mem = new void[S.sizeof];` ~
          `emplace!S(mem, cast(S)(*_range).save);` ~
          `return RefRange!S(cast(S*)mem.ptr);`;
}

The problem is that the emplace stuff is @system, and I know that it's really 
@safe and want to mark it with @trusted, but I _don't_ know that _range's save 
function is @safe, so I can't mark RefRange's save as @trusted. I'm stuck 
either doing nonsense like the code above or giving up on making it possible 
for save to be @safe. With David's suggestion, all of that can be reduced to 
this:

@property auto save()
{
   import std.conv;
   alias typeof((*_range).save) S;
   static assert(isForwardRange!S, S.stringof ~ " is not a forward range.");

   @trusted
   {
       auto mem = new void[S.sizeof];`
       emplace!S(mem, cast(S)(*_range).save);`
       return RefRange!S(cast(S*)mem.ptr);
   }
}

That is _way_ cleaner.

Now, the actual code of in RefRange is actually even _more_ complicated 
because of the need to handle the possibility of making save const, but if 
enhancement request# 8407 were implemented, then that would be solved. I 
believe that there is at least _some_ buy in on Walter with regards to issue# 
8407 based on some comments by Andrei, but I have no idea what Walter will 
think of David's proposal. With both, functions like RefRange's save become 
reasonably small. Without them, you either give up on const and/or @safe, make 
your templated code require that the functions it uses be const and/or @safe 
(which can be way too restrictive), or you do what I did with RefRange, which 
works but is downright ugly.

- Jonathan M Davis
July 28, 2012
Re: @trusted considered harmful
On 2012-07-28 00:08:28 +0000, "David Nadlinger" <see@klickverbot.at> said:

> @trusted in its current form needs to go. Its design is badly broken, 
> as it leaks implementation details and encourages writing unsafe code.

@trusted is a dangerous thing. Since the first time I tried to use it, 
I always found it troublesome. I agree it needs to be a scope. And for 
backward compatibility, a @trusted function should be exactly the same 
as a @safe function where the whole body is wrapped in a @trusted scope.

-- 
Michel Fortin
michel.fortin@michelf.ca
http://michelf.ca/
July 28, 2012
Re: @trusted considered harmful
On 2012-07-28 03:24:58 +0000, Michel Fortin <michel.fortin@michelf.ca> said:

> On 2012-07-28 00:08:28 +0000, "David Nadlinger" <see@klickverbot.at> said:
> 
>> @trusted in its current form needs to go. Its design is badly broken, 
>> as it leaks implementation details and encourages writing unsafe code.
> 
> @trusted is a dangerous thing. Since the first time I tried to use it, 
> I always found it troublesome. I agree it needs to be a scope. And for 
> backward compatibility, a @trusted function should be exactly the same 
> as a @safe function where the whole body is wrapped in a @trusted scope.

And when I say it "needs to be a scope", what I meat is a block. I 
don't think it should be a new scope unless someone finds a good reason 
for it (unsafe struct destructors?)


-- 
Michel Fortin
michel.fortin@michelf.ca
http://michelf.ca/
July 28, 2012
Re: @trusted considered harmful
On Friday, July 27, 2012 23:28:17 Michel Fortin wrote:
> On 2012-07-28 03:24:58 +0000, Michel Fortin <michel.fortin@michelf.ca> said:
> > On 2012-07-28 00:08:28 +0000, "David Nadlinger" <see@klickverbot.at> said:
> >> @trusted in its current form needs to go. Its design is badly broken,
> >> as it leaks implementation details and encourages writing unsafe code.
> > 
> > @trusted is a dangerous thing. Since the first time I tried to use it,
> > I always found it troublesome. I agree it needs to be a scope. And for
> > backward compatibility, a @trusted function should be exactly the same
> > as a @safe function where the whole body is wrapped in a @trusted scope.
> 
> And when I say it "needs to be a scope", what I meat is a block. I
> don't think it should be a new scope unless someone finds a good reason
> for it (unsafe struct destructors?)

If you need another scope, you can always just use a second set of braces, but 
if an @trusted block introduces another scope, there's no way to make it _not_ 
introduce one (not without adding more syntax to make it not introduce one 
anyway).

- Jonathan M Davis
July 28, 2012
Re: @trusted considered harmful
On Saturday, 28 July 2012 at 00:53:24 UTC, José Armando García 
Sancio wrote:
> On Fri, Jul 27, 2012 at 5:08 PM, David Nadlinger 
> <see@klickverbot.at> wrote:
>>  2) The first step is necessary, but mainly of cosmetic nature 
>> (think
>> `pure`, `pure2`). We still need to address for the granularity 
>> and attribute
>> inference problem. The obvious solution is to add a "@trusted"
>> declaration/block, which would allow unsafe code in a certain 
>> region.
>> Putting @trusted in the function header would still be allowed 
>> for backwards
>> compatibility (but discouraged), and would have the same 
>> effect as marking
>> the function @safe and wrapping its whole body in a @trusted 
>> block. It could
>> e.g. look something like this (the @ prefix definitely looks 
>> weird, but I
>> didn't want to introduce a new keyword):
>>
>
> Agreed. This is very similar to how Rust works. In Rust all the
> functions are assumed to be @safe. Unsafe code can only be 
> performed
> in clearly marked blocks. Note: I am not suggesting D should 
> implement
> Rust's solution as David already pointed out.
>
> -Jose

C#'s approach is also similar with the unsafe keyword.
« First   ‹ Prev
1 2 3 4
Top | Discussion index | About this forum | D home