January 09, 2020
On 09.01.20 20:59, H. S. Teoh wrote:
> On Thu, Jan 09, 2020 at 02:35:36PM -0500, Steven Schveighoffer via Digitalmars-d wrote:
>> On 1/9/20 2:22 PM, Timon Gehr wrote:
> [...]
>>> @safe code can't be trusted. It may be edited by programmers who are
>>> not allowed to write @trusted code.
>>
>> I'm not saying it's safe. I'm saying I want the mechanical checking
>> outside the trusted escape. e.g. I want the compiler to check these
>> parts, but I know this one part needs trusting. D doesn't give a
>> better way to express this other than safe code with trusted escapes.
> [...]
> 
> Yeah, I also consider this to be valuable.  Another way of doing the
> same thing is that @trusted *doesn't* allow unsafe operations by
> default, it just marks that function as needing to be manually verified,
> but within that function you have to explicitly mark out which parts are
> to be trusted:
> ...
> 
> T
> 

I like this proposal (but there should be @system _expressions_ too). It would also fix accidentally trusting your template alias parameters.

Probably this is going a bit far off-topic now though.
January 09, 2020
On 1/9/20 2:59 PM, H. S. Teoh wrote:

> Yeah, I also consider this to be valuable.  Another way of doing the
> same thing is that @trusted *doesn't* allow unsafe operations by
> default, it just marks that function as needing to be manually verified,
> but within that function you have to explicitly mark out which parts are
> to be trusted:
> 
> 	auto myfunc(Args args) @trusted {
> 		... // only @safe code allowed here
> 		@system {
> 			... // @system code allowed here
> 		}
> 		... // only @safe code allowed here
> 	}

Right, that's what I said 2 messages back ;)

> If we could design it again, probably you should have safe and system be what they are today, trusted would be safe, except where you put in unsafe blocks. This allows the code to pick which parts should be able to call system functions.

-Steve
January 09, 2020
On Thursday, 9 January 2020 at 19:35:36 UTC, Steven Schveighoffer wrote:
> If I could mark the whole thing trusted, and turn on the mechanical checking for everything except line X, then I'd do that instead.

Well, you can do that:

    void foo() @trusted
    {
        () @safe { /* ... stuff ... */ } ();
        bar(1, 2, 3); /* line X */
        () @safe { /* ... stuff ... */ } ();
    }

The problem is, of course, that you can't have `foo`'s safety inferred based on "stuff". Which is what we usually want.

I have actually used that pattern once in Phobos:

https://github.com/dlang/phobos/blob/master/std/range/package.d#L1550-L1581

I had to duplicate the code and use `__traits(compiles, ...)` to get inference. So the result isn't exactly pretty. But the use of @trusted is watertight, as far as I can tell.
January 09, 2020
On 1/9/20 3:08 PM, ag0aep6g wrote:
> On Thursday, 9 January 2020 at 19:35:36 UTC, Steven Schveighoffer wrote:
>> If I could mark the whole thing trusted, and turn on the mechanical checking for everything except line X, then I'd do that instead.
> 
> Well, you can do that:
> 
>      void foo() @trusted
>      {
>          () @safe { /* ... stuff ... */ } ();
>          bar(1, 2, 3); /* line X */
>          () @safe { /* ... stuff ... */ } ();
>      }
> 
> The problem is, of course, that you can't have `foo`'s safety inferred based on "stuff". Which is what we usually want.

This turns on its head the lines you have to pay most attention to though :)

Basically, your job of manually checking such a function is to 1. establish which parts are not mechanically checked, and 2. verify they are ALWAYS correct (unlikely) or that they are correct based on the mechanically checked parts of the function.

In order to do that efficiently, I want to tag where the trouble spots may be. And these are likely to be MUCH fewer than the checked ones.

Put it another way, a safe function with no trusted escapes needs no checking. A safe function with one trusted escape needs each safe line checked in reference to the trusted escape. With 2 escapes, you need to check each line against 2 blocks, etc.

e.g. (to build on the previous example):

@safe void someFunction()
{
    int[4] data;
    foo("argument");
    bar();
    @trusted
    {
        data.ptr[3] = 42;
    }
}

Looking at line 2 and 3, I can verify that no parts touch anything that might be affected by the trusted block. I don't need to check foo to see if it's safe when called with "argument", nor do I need to investigate bar(), I just need to know, it doesn't use anything that is currently affected by the trusted block. It makes my job easier as a manual checker. I only need to investigate the declaration of `data`, and the trusted block.

> 
> I have actually used that pattern once in Phobos:
> 
> https://github.com/dlang/phobos/blob/master/std/range/package.d#L1550-L1581
> 
> I had to duplicate the code and use `__traits(compiles, ...)` to get inference. So the result isn't exactly pretty. But the use of @trusted is watertight, as far as I can tell.

Hm... this is an odd thing for sure. I probably would have done it this way though:

ref getR1() @trusted { return r1; }
ref getR2() @trusted { return r2; }

return r1Chosen ? ChooseResult(r1Chosen, getR1().save, getR2()) : ChooseResult(r1Chosen, getR1(), getR2().save);

But actually, is that right? Even if it's safe, it's a bad idea to copy the non-tagged item, as its contents are the contents for the other item. Shouldn't it be:

return r1Chosen ? ChooseResult(r1Chosen, getR1().save, R2.init) : ChooseResult(r1Chosen, R1.init, getR2().save);

-Steve
January 09, 2020
On Thursday, 9 January 2020 at 20:33:00 UTC, Steven Schveighoffer wrote:
> @safe void someFunction()
> {
>     int[4] data;
>     foo("argument");
>     bar();
>     @trusted
>     {
>         data.ptr[3] = 42;
>     }
> }
>
> Looking at line 2 and 3, I can verify that no parts touch anything that might be affected by the trusted block. I don't need to check foo to see if it's safe when called with "argument", nor do I need to investigate bar(), I just need to know, it doesn't use anything that is currently affected by the trusted block. It makes my job easier as a manual checker. I only need to investigate the declaration of `data`, and the trusted block.

Effectively, the @trusted block taints the surrounding function without disabling the mechanical checks on most lines.

For @trusted blocks, I could get on board with that, as long as we limit the scope of the taint to the function. Then it's still fairly obvious what code needs special expertise. I.e., the rule for junior programmers changes from "you may only touch @safe functions" to "you may only touch @safe functions that don't contain @trusted blocks". That's still reasonable.

I don't think tainting should be allowed beyond function scope (e.g., tainting a whole module by accessing one of its globals), because then it becomes hard to identify the safety-critical parts.

I also don't think we should use that pattern with the @trusted function attribute we have at the moment. You're necessarily crossing the function border with that. So it's less clear what level of tainting is acceptable, and it would be harder to formalize. And we'd need to formalize this. Using @trusted in a way that is clearly against the spec is just asking for trouble.

For the given `someFunction`, making `data` an "@system variable" [1] would also be a solution. Then you wouldn't even have to look at lines 2 and 3. I'd be in favor of pursuing that approach first. I can imagine that @system variables would often make tainting functions unnecessary.

>> I have actually used that pattern once in Phobos:
>> 
>> https://github.com/dlang/phobos/blob/master/std/range/package.d#L1550-L1581
>> 
>> I had to duplicate the code and use `__traits(compiles, ...)` to get inference. So the result isn't exactly pretty. But the use of @trusted is watertight, as far as I can tell.
>
> Hm... this is an odd thing for sure. I probably would have done it this way though:
>
> ref getR1() @trusted { return r1; }
> ref getR2() @trusted { return r2; }
>
> return r1Chosen ? ChooseResult(r1Chosen, getR1().save, getR2()) : ChooseResult(r1Chosen, getR1(), getR2().save);

It's done that way in other parts of the code [2]. The problem is that `getR1` and `getR2` are not safe. So they're invalid if we take the spec seriously.

Looking at my own code again, I'm not so sure anymore if it's really "watertight". What is stopping the hypothetical junior programmer from editing my @safe helper functions to leak a reference to the union? It's @safe code, so they're allowed to edit it. But it's @safe code inside @trusted code. Does that mean it's off limits again? Now my head's starting to spin.

> But actually, is that right? Even if it's safe, it's a bad idea to copy the non-tagged item, as its contents are the contents for the other item. Shouldn't it be:
>
> return r1Chosen ? ChooseResult(r1Chosen, getR1().save, R2.init) : ChooseResult(r1Chosen, R1.init, getR2().save);

You're not wrong. It's not even safe. If the non-chosen range contains pointers, they're going to be garbage. If it has a copy constructor, those garbage pointers might be dereferenced.


[1] https://github.com/dlang/DIPs/pull/179
[2] https://github.com/dlang/phobos/blob/f66da1f8c962fbb7ed440caad53d3a978c72ff88/std/range/package.d#L1468-L1481
January 09, 2020
On 1/9/20 6:26 PM, ag0aep6g wrote:
> It's done that way in other parts of the code [2]. The problem is that `getR1` and `getR2` are not safe. So they're invalid if we take the spec seriously.

I'm not sure what you mean by this. They are not safe, nor are they intended to be, they are trusted escapes. But they only live inside the function that uses them, and only called when they are safe.

Your reference is even better, the function is only defined within the context where it is valid.

In any case, I'll look at making a PR in a bit. No need to keep discussing in this thread.

-Steve
January 12, 2020
On Thursday, 2 January 2020 at 09:47:48 UTC, Mike Parker wrote:
> This is the feedback thread for the first round of Community Review for DIP 1028, "Make @safe the Default":

Not sure, if this has allready been mentioned, I didn't read all posts, so forgive me my laziness...

With @safe being the default, a typical hello-world-program would need @system. I don't like this.

So I thought, why couldn't (as a default) the compiler deduce @safe/@system automatically, like he does for templates? One still could write @safe if this is important, so the compiler will complain, if it isn't @safe.

Or, at least, writeln and co should be @trusted... (IMHO, there should be a real good reason, whenever a Phobos function does not work in @safe-code...)
January 12, 2020
On Sunday, 12 January 2020 at 12:28:23 UTC, berni44 wrote:
> With @safe being the default, a typical hello-world-program would need @system. I don't like this.

writeln is @safe as long as you use it with safe arguments.

import std.stdio;
@safe:
void main() {
    writeln("Hello, world!");
}

Compiles without errors.
January 13, 2020
On Thursday, 2 January 2020 at 09:47:48 UTC, Mike Parker wrote:
> This is the feedback thread for the first round of Community Review for DIP 1028, "Make @safe the Default":
>
> https://github.com/dlang/DIPs/blob/1b705f8d4faa095d6d9e3a1b81d6cfa6d688554b/DIPs/DIP1028.md

First, @Walter, thanks for writing this, and @Mike, thanks for taking care of the review process :-)

Since I'm co-author of some key bits of the current reviewer guidelines, I'll try to follow the questions there fairly precisely and see if it makes for some nice structure in my feedback.


(1) Is the proposal acceptable as a language change in principle?

Clearly yes: @safe-by-default fits within an observable trend of languages getting more and more concerned with provable memory safety, and with a related trend of requiring the user to write provably safe code.  Rust in particular has significantly raised the bar in this space, and proven that such constraints are often welcomed by developers.

The should we/shouldn't we questions around this are likely to come down to practicalities.

However, this does seem like a fairly major philosophical shift in the language design.  Up until now D has (possibly only implicitly) followed what amounts to a "permissive by default" design philosophy where (apart from really obvious stupidities) the default setting is that one can do what one likes, and stronger design constraints (including memory safety but also e.g. constness and immutability, purity, nogc, nothrow, final methods, ...) are readily available but all opt-in.

The existence of a parallel DIP for nothrow-by-default seems to confirm this push for a more restrictive-by-default language.  The two DIPs should be considered on their own merits, so I won't try to couple them, beyond noticing the design trend they both contribute to.  However, the impact of that trend should be considered in terms of the impact on the D development experience.  Some thoughts on pros and cons will be covered below.

As a separate issue, it's probably a good idea to take discussion of keyword choices off the table for the purposes of this DIP.  @safe, @trusted and @system have


(2) Is the proposal workable in practice?

-- "Is the proposed feature specified in sufficient detail?" --

There is sufficient spec to make the change happen, per se.  What is missing is a sufficiently detailed overview of the practical problems that will arise from the migration, and explicit proposals for how to deal with them.  This brings us to ...

-- "Are edge cases, flaws, and risks identified and addressed?" --

The identified breaking changes and risks are discussed only at a high level.  The proposed solutions (explicit annotation of all non-templated functions) are correct per se but don't really capture the complexity of the likely reality.  Some examples:

  * @system-by-default means that if a @safe attribute is overlooked,
    that bug can be fixed without breaking change: by contrast, with
    @safe-by-default, an overlooked @system attribute can't be fixed
    without breaking change to the API concerned.

    (Yes, I can hear you now saying that if the function isn't @safe
    then the compiler will object and force the user to add @system,
    so this oversight won't happen.  But this is about intent: the
    function might be safe in practice, as initially implemented,
    but with no intention to preserve that guarantee.  One advantage
    of opt-in constraints is that generally one can be sure that the
    developer means them to be there.)

  * As noted already in others' feedback, the use of `@system:` to tag
    multiple functions has an asymmetric impact compared to `@safe:`.
    In the latter case one likely _wants_ that attribute to apply to
    all subsequent functions, including templated ones.  However, we
    probably do not want catch-all `@system:` to override the inferred
    safety of templated function instantiations.  This edge case should
    be discussed, with suggestions for how to address it.

  * The DIP contains no advice or impact assessment for the case of
    3rd-party libraries that are no longer actively maintained, and
    the consequent risks for obsoleting a large amount of existing
    D code.  Ideally the DIP should contain a robust estimate of the
    numbers of projects this might impact, and some discussion of the
    pros and cons of that impact, and mitigation strategies.

    The migration plan should include clear steps for how to regularly
    remeasure the expected impact on 3rd-party library usability over
    time (e.g. as more and more libraries are adapted to support the
    new feature).  There should also be explicit criteria for deciding
    on what level of impact is (un)acceptable in order to transition
    from `--preview=safedefault` to the feature being on by default.

  * Issues related to taking the address of local variables (mentioned
    several times in this discussion thread) should be discussed, with
    reference to other DIPs that address that concern.  It should be
    made clear whether finalization of those other features is needed
    (or strongly desirable) to finalize @safe-by-default.

  * The impact of @safe-by-default on `extern` APIs should be discussed.
    We have no reasonable grounds to assume these functions are safe by
    default: the DIP should address how to deal with this (which should
    ideally not rely on developer virtue).

  * The impact of @safe-by-default on the ability to write `@nogc` code
    should be covered by the DIP, including appropriate references to
    other relevant DIPs.

  * The advice in the current draft of the DIP to "annotate functions
    that aren't safe with `@trusted` or `@system` should include clear
    guidance as to _when_ to use `@trusted` and when to use `@system`.
    We don't want to "fix" migration problems by blindly slapping
    `@trusted` onto code that hasn't been properly validated.

    We have already had cases of people trying to do that just to get
    stuff to compile with `@safe` <https://github.com/msoucy/dproto/pull/79>
    so we should try to avoid the risk of spreading that kind of code
    around.  (Rust has to deal with similar concerns, of too many devs
    just adding `unsafe` blocks willy-nilly to get the compiler off
    their backs, and I've seen similarly problematic uses of @trusted
    in even some quite prominent D libraries.)

  * It seems likely that @safe-by-default will increase the number of
    occasions that developers have to use @trusted.  The DIP should
    try to make some estimate of the amount of impact, and should
    address whether it is necessary (or at least very desirable) to
    add support for @trusted code blocks as well as functions (cf.
    what Rust allows with `unsafe`, and feedback by Manu and others
    on the problems of needing to define local lambdas to apply the
    @trusted attribute to).

These last few examples touch on another missing risk: there is no assessment of the expected impact on developer experience.  Arguably a very nice productivity feature of D is the ability to hack readily and only worry about introducing strict constraints when one actively wants them.  This is part of what makes D so readily usable for everything from small casual scripts to large-scale libraries and applications.

Those of us who tend to apply `@safe` willingly and regularly may underestimate the impact on users who prefer fast iteration over strictly enforced constraints.  I'm particularly concerned that it may get too many developers into the habit of unthinkingly slapping down `@trusted` on code that doesn't deserve it, rather as some Rust developers just `unsafe` lots of things without really thinking it through.

It's easy to dismiss those people as architects of their own pain, but the problem is how such users can spread bad habits by example.  We should perhaps not underestimate the importance of consent in submitting to constraints ... :-)

OTOH the positive flipside of imposing constraints by default is that it means that the combination of different constraints gets much better battle tested: there is a much lower barrier to discovering (and hopefully fixing) tricky edge cases.

-- "Are there any platform or architecture pitfalls to be aware of?" --

None that I can think of.

-- "Is there an implementation that proves the proposed feature works in practice?" --

The basic implementation is likely trivial.  Questions of proof need to apply more to the migration path (see below).

-- "Does the DIP consider prior work from other languages?" --

Yes, but the consideration is merely of their existence.  It would be good to have a more detailed comparison, discussing how they achieve those outcomes, and what the resulting constraints and developer experiences are.

-- "If the proposed feature is a breaking change, is there a well-defined migration path?" --

This is the crux of the matter.  There are many small finnicky impacts that this change will have.

The basic migration path of using `--preview=safedefault` and ironing out kinks is sound.  However, what needs to be established (which is currently missing) is a clear statement of the criteria that will be used to determine when (and if!) it is appropriate to transition from the `--preview` feature to having @safe-by-default ... by default.

In short, acceptance of moving to the `--preview=safedefault` stage should NOT be taken as acceptance of the @safe-by-default transition in its entirety.  The DIP should define the definite blockers to that transition, and should outline a robust review process for the decision to finalize (or abandon) the change.

The core code migration step (adding `@system` to non-templated functions without an existing `@safe`, `@trusted` or `@system` attribute) ought to be possible to automate: the DIP might mandate the creation of such a tool as a requirement before the transition can be finalized.


(3) Summary

The proposed feature is a significant breaking change.  If we are lucky, the practical impact may be much smaller than one might anticipate, but that needs to be robustly established before approval can be given to the DIP (or at least, to transitioning away from `--preview=safedefault`).

The DIP needs to provide much more detail on the anticipated impacts, the migration paths, and the risks for the existing and future D ecosystem (with particular attention to how many existing codebases may be obsoleted).  It should also clarify if any other DIPs or experimental features need to be finalized before this DIP can be.

The anticipated impact on both developer and maintainer experience should be carefully outlined, with clearly written mitigation strategies for the worst pain points.

In short: since nothing stops anyone who cares from having a `@safe` codebase right now through the existing opt-in features, show us in detail how the pain of transitioning to opt-out is really worth it ;-)
January 13, 2020
On Monday, 13 January 2020 at 15:57:38 UTC, Joseph Rushton Wakeling wrote:
> As a separate issue, it's probably a good idea to take discussion of keyword choices off the table for the purposes of this DIP.  @safe, @trusted and @system have

Whoops, I forgot to complete my train of thought there :-)  The paragraph should read:

@safe, @trusted and @system have well-defined meanings which are entirely compatible with the transition to @safe-by-default.  We should keep our focus on the stuff that needs to change, not the stuff that doesn't.