don't use @trusted functions nested in @safe functions

[WalterBright]

Experience with this has led to the conclusion it is not a good practice.

[dlang-bot]

Thanks for your pull request, @WalterBright!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

[pbackus]

This should probably specify "non-static @trusted nested functions," since those are the ones that have access to the enclosing scope.

[dnadlinger]

What is this "experience"? Immediately invoked @trusted lambdas are often used to emulate @trusted blocks, which are helpful for writing correct templated trusted code, and have also worked out nicely in Rust.

[ljmf00]

This should probably specify "non-static @trusted nested functions," since those are the ones that have access to the enclosing scope.

@RazvanN7 this needs to be fixed in order to be technically correct according to non-static nested function behaviour and the rationale presented in this best practice.

[RazvanN7]

@ljmf00 Yes, that is why I did not merge it already.

[dnadlinger]

Where is the DIP for this?

[ljmf00]

Where is the DIP for this?

I don't think a "Best Practice" need a DIP.

[dnadlinger]

I don't think this suggestion is very well-motivated, though, particularly as it goes against what some others (including myself) would consider best practice for templated code.

[ibuclaw]

I don't think this suggestion is very well-motivated, though, particularly as it goes against what some others (including myself) would consider best practice for templated code.

The growing consensus is that system code should only be done in @system blocks/functions. The idea is to make @trusted have the same safety checking as @safe currently does, and to not allow unsafe operations in @trusted code.

This is being hammered out in a DIP, to which will hopefully show up soon.

[dnadlinger]

The idea is to make @trusted have the same safety checking as @safe currently does, and to not allow unsafe operations in @trusted code.

Well, that's then definitely a change of a magnitude that should be well-researched and motivated (i.e. go through the DIP process). (If I understand your point correctly, @trusted lambdas are @system blocks!)

[ibuclaw]

The idea is to make @trusted have the same safety checking as @safe currently does, and to not allow unsafe operations in @trusted code.

Well, that's then definitely a change of a magnitude that should be well-researched and motivated (i.e. go through the DIP process). (If I understand your point correctly, @trusted lambdas are @system blocks!)

Yes, and because of that, it is far too easy to whitewash your code with @safe when it is anything but.

This thread provides some additional context. https://forum.dlang.org/thread/[email protected]

[ibuclaw]

There is one place where you do not want to obey this: high-quality templates. There are often cases where you want to do something @trusted, but if you marked the whole function that way it'd also be @trusted when the function uses unsafe member function from types it's instanced with. And trying to design an internal @safe interface for a library like Phobos to avoid the @trusted nested function pattern would like double the code size.

I'm afraid that this advice in the spec would mislead Phobos / Vibe.D / Mir developers. Please be more specific.

Again, the plan is for @trusted to have the same safe-ty checking as @safe. Meaning - if accepted - doing unsafe things in @trusted code will (eventually) be an error unless explicitly put in @system.

[dukc]

There is one place where you do not want to obey this: high-quality templates.

Illustration of the problem:

import std.range;

auto right(Range)(Range r)
   if(isInputRange!Range && is(ElementType!Range : int))
{  import core.stdc.stdlib;
   
   auto ptr = (() @trusted => malloc(10))();
   scope(exit) () @trusted {ptr.free;}();
   
   int result;
   foreach(e; r)
   {  /+ some calculations using mallocated buffer... +/
   }
   
   return result;
}

auto wrong(Range)(Range r) @trusted
   if(isInputRange!Range && is(ElementType!Range : int))
{  import core.stdc.stdlib;
   
   auto ptr = malloc(10);
   scope(exit) ptr.free;
   
   int result;
   foreach(e; r)
   {  /+ some calculations using mallocated buffer... +/
   }
   
   return result;
}

struct UnsafeRange
{  private int* ptr;
   
   ref front(){return *ptr;}
   auto popFront(){ptr++;}
   auto empty(){return *ptr == 0;}
}

@safe void main()
{  int goodThisCompiles = [1,2,3,4].right;
   int goodAlsoCompiles = [1,2,3,4].wrong;
   
   //int goodDoesNotCompile = UnsafeRange(new int(5)).right;
   int uhOh = UnsafeRange(new int(5)).wrong;
}

[dukc]

Again, the plan is for @trusted to have the same safe-ty checking as @safe. Meaning - if accepted - doing unsafe things in @trusted code will (eventually) be an error unless explicitly put in @system.

Okay, this works, albeit it causes a lot of need to migrate code with relatively little benefit.

But this is not a very public plan yet - It's the first time I even heard of it, and it isn't accepted yet. Having an advice like the one discussed given on spec is a bit early IMO.

[aG0aep6G]

auto right(Range)(Range r)
   if(isInputRange!Range && is(ElementType!Range : int))
{  import core.stdc.stdlib;
   
   auto ptr = (() @trusted => malloc(10))();
   scope(exit) () @trusted {ptr.free;}();
   
   int result;
   foreach(e; r)
   {  /+ some calculations using mallocated buffer... +/
   }
   
   return result;
}

That code is invalid. You're effectively just marking free as @trusted. But free does not have a safe interface, so it cannot be @trusted. It can only be @system. Walter is calling out that kind of @trusted as wrong.

There is obviously a desire (maybe a need) to use @trusted like that, but it's not currently allowed by the language. Which is why people are exploring @system blocks, changing the semantics of @trusted, etc.

[pbackus]

There is obviously a desire (maybe a need) to use @trusted like that, but it's not currently allowed by the language.

This is a problem with the language spec, not the code. An immediately-invoked @trusted lambda does not need to have a safe interface in order to be memory-safe at all possible call sites because it only has one call site, and the safety of that single call site can be proven based on local knowledge.

(The same logic applies to all nested functions: since all of their call sites are known at compile time, the memory safety of those call sites can be proven by exhaustion.)

[aG0aep6G]

This is a problem with the language spec, not the code. An immediately-invoked @trusted lambda does not need to have a safe interface in order to be memory-safe at all possible call sites because it only has one call site, and the safety of that single call site can be proven based on local knowledge.

That's fine. But someone has to do the work and write that down in the spec (and get it past Walter). Before then, the language doesn't allow it. The rules for @trusted must be solid, not open for interpretation.

Also, I'm not convinced that treating @trusted that way is a good idea. It makes @trusted even more complicated. It adds a gotcha: one cannot simply refactor a nested @trusted function to a free function. Some version of the @system blocks idea can probably provide the feature more cleanly.

[dukc]

Also, I'm not convinced that treating @trusted that way is a good idea. It makes @trusted even more complicated. It adds a gotcha: one cannot simply refactor a nested @trusted function to a free function. Some version of the @system blocks idea can probably provide the feature more cleanly.

You may well be right with regular functions, perhaps even with your average template. But for Phobos, or other such high-quality template libraries, the alternatives are worse:

1: no @safe for the client code even when the code can obviously be. 2: false @trusted 3: Horrible code bloat to design an internal @safe interface. Just consider how much more complicated the example I provided would be that way.

[aG0aep6G]

Also, I'm not convinced that treating @trusted that way is a good idea. It makes @trusted even more complicated. It adds a gotcha: one cannot simply refactor a nested @trusted function to a free function. Some version of the @system blocks idea can probably provide the feature more cleanly.

You may well be right with regular functions, perhaps even with your average template. But for Phobos, or other such high-quality template libraries, the alternatives are worse:

1: no @safe for the client code even when the code can obviously be. 2: false @trusted 3: Horrible code bloat to design an internal @safe interface. Just consider how much more complicated the example I provided would be that way.

You forgot the one true alternative even though you quoted it: A new, safer @trusted with @system blocks (or some other iteration of that general idea).

You don't like @trusted as it is in the spec, and for good reason. But if we're going to change it, we might as well do it right.

[dukc]

You forgot the one true alternative even though you quoted it: A new, safer @trusted with @system blocks (or some other iteration of that general idea).

You don't like @trusted as it is in the spec, and for good reason. But if we're going to change it, we might as well do it right.

In the long run I agree. I'd hope for a more backwards-compatible change than the one they're currently planning, but otherwise the system block idea sounds good.

But since it's still just a plan, I think in the short run it's just best to say that the safe interface requirement applies only for non-local symbols. That's how @trusted and @safe are used in practice anyway.

And Walter is backing this PR up with experience, not with spec compliance. This is why I don't want this change to be merged in the present form, at least not before the @system blocks idea has been accepted as a DIP.

[aG0aep6G]

But since it's still just a plan, I think in the short run it's just best to say that the safe interface requirement applies only for non-local symbols. That's how @trusted and @safe are used in practice anyway.

I'm afraid that the devil is in the details, and even a quick fix would need significant effort to make any sense. That effort would be better spent on the long run. But I wouldn't mind seeing a spec PR that proves me wrong. If allowing the current usage of @trusted is actually easy, go for it.

And Walter is backing this PR up with experience, not with spec compliance. This is why I don't want this change to be merged in the present form, at least not before the @system blocks idea has been accepted as a DIP.

I think I get what you're saying: This "best practice" is basically telling everyone that they're doing @trusted wrong. So if this gets merged and @system blocks never materialize, you're stuck with a spec that condemns the only way you can actually use @trusted in practice. And that's of course not where we want to be.

But as far as I'm concerned, we're already there. The spec already forbids all the nested @trusted functions that don't have safe interfaces. Repeating it as a "best practice" doesn't change anything. All "best practices" are ultimately meaningless.

[pbackus]

That's fine. But someone has to do the work and write that down in the spec (and get it past Walter). Before then, the language doesn't allow it. The rules for @trusted must be solid, not open for interpretation.

I agree, and in fact I am working on writing up an alternative formulation of the rules which handles both nested and non-nested functions sensibly. I'll post something on the forums when I have a complete draft ready.

[dukc]

I think I get what you're saying: This "best practice" is basically telling everyone that they're doing @trusted wrong. So if this gets merged and @system blocks never materialize, you're stuck with a spec that condemns the only way you can actually use @trusted in practice. And that's of course not where we want to be.

Yes, that is roughly what I meant.

But as far as I'm concerned, we're already there. The spec already forbids all the nested @trusted functions that don't have safe interfaces.

I think you are right about that, unfortunately.

Repeating it as a "best practice" doesn't change anything. All "best practices" are ultimately meaningless.

Hmm. It's rarely bad to advocate for spec compliance in best practices. But it increases the risk that people mark their templates @system with no technical need to do so. And there is no way here for a spec-breaking program to break in future, because Phobos would break also with no realistic migration path.

So as dirty as the conclusion is, the spec must advocate against itself here, not for. Until Paul's changes and/or @system blocks get through of course.