DIP 1028---Make @safe the Default---Community Review Round 1

ag0aep6g anonymous at example.com
Thu Jan 9 23:26:00 UTC 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


More information about the Digitalmars-d mailing list