Jump to page: 1 24  
Page
Thread overview
@trusted considered harmful
Jul 28, 2012
David Nadlinger
Jul 28, 2012
Jonathan M Davis
Jul 28, 2012
Paulo Pinto
Jul 28, 2012
Jesse Phillips
Jul 28, 2012
David Piepgrass
Jul 28, 2012
Jonathan M Davis
Jul 28, 2012
Jesse Phillips
Jul 28, 2012
Jonathan M Davis
Jul 28, 2012
David Nadlinger
Jul 28, 2012
David Nadlinger
Jul 28, 2012
Jonathan M Davis
Jul 28, 2012
David Nadlinger
Jul 28, 2012
David Nadlinger
Jul 28, 2012
Jonathan M Davis
Jul 29, 2012
David Piepgrass
Jul 29, 2012
Artur Skawina
Jul 28, 2012
Michel Fortin
Jul 28, 2012
Michel Fortin
Jul 28, 2012
Jonathan M Davis
Jul 28, 2012
Dmitry Olshansky
Jul 28, 2012
Artur Skawina
Jul 28, 2012
Artur Skawina
Jul 28, 2012
Artur Skawina
Jul 28, 2012
David Nadlinger
Jul 28, 2012
Artur Skawina
Jul 28, 2012
David Nadlinger
Jul 28, 2012
David Nadlinger
Jul 28, 2012
Jonathan M Davis
Jul 29, 2012
deadalnix
Jul 29, 2012
deadalnix
Feb 15, 2013
Martin Nowak
Feb 15, 2013
Martin Nowak
Feb 15, 2013
deadalnix
Feb 15, 2013
Timon Gehr
Feb 15, 2013
deadalnix
July 28, 2012
@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
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
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
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
> 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
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
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
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
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
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