On 6/17/20 2:14 AM, Timon Gehr wrote:
> The documentation says @trusted code has to be audited, not all code in modules that contain a bit of @trusted code or no @trusted but both @system and @safe code.

That seems to be a problem with the documentation. The unit of review is the module. A module with system or trusted code needs to be reviewed as a unit.

On 6/17/20 2:14 AM, Timon Gehr wrote:
>> * "even outside the module, __traits(getMember, ) bypasses private" -> that's an obvious bug that needs to be fixed.
> 
> It's not obvious that it is a bug (it would need to be defined to be a bug)

https://issues.dlang.org/show_bug.cgi?id=20941

> nor is it obvious that making bypassing of `private` unsafe obviates the need for @system variables.

It's an objection to that particular argument.

On 6/17/20 2:14 AM, Timon Gehr wrote:
> 
>> Embellishments are immediate (ctors, allowing void init, opAssign, etc). To the extent this type can be pried open in @safe code, these are definitely bugs in the existing language.
> 
> Not all of them, though the fact that it has a default constructor `this(T value){ this.value = value; }` is a bug.

Ehm. The structure given was a minimal sketch, not an attempt at a complete implementation. An implementation could choose to disable the constructor or define it as @system etc.

> Maybe you can make it work the way you envision, but what is to stop someone from coming along and adding some more @safe code to that module?

A code review.

On Wednesday, 17 June 2020 at 12:41:57 UTC, Andrei Alexandrescu wrote:
> On 6/17/20 2:14 AM, Timon Gehr wrote:
>> 
>>> Embellishments are immediate (ctors, allowing void init, opAssign, etc). To the extent this type can be pried open in @safe code, these are definitely bugs in the existing language.
>> 
>> Not all of them, though the fact that it has a default constructor `this(T value){ this.value = value; }` is a bug.
>
> Ehm. The structure given was a minimal sketch, not an attempt at a complete implementation. An implementation could choose to disable the constructor or define it as @system etc.

...or the language would do all that work for you, under this DIP; no new user types required, no ctors/dtors/sets/gets, merely a @system in front of a variable. That something *can* be a library solution does not mean that it *should* be one (*cough* emplace *cough* move *cough*). The DIP *is* a solution to many of the "bugs" with @safe.

Writing @safe code in a module should be no different to writing @safe code outside that module.

@system int x;

Done. Private or no private, writing to it is not @safe no matter where you do it.
No SysVar!insert_all_your_types_here all over the binary; no associated runtime costs (i.e. copies for and in get/set); no associated implementation costs (at least in current language, until such time that moves do get realized).

On Wednesday, 17 June 2020 at 12:41:57 UTC, Andrei Alexandrescu wrote:
>> Maybe you can make it work the way you envision, but what is to stop someone from coming along and adding some more @safe code to that module?
>
> A code review.

I thought the whole premise of @safe was that code review is inadequate for catching memory corruption bugs.

On 6/17/20 9:19 AM, Stanislav Blinov wrote:
> On Wednesday, 17 June 2020 at 12:41:57 UTC, Andrei Alexandrescu wrote:
>> On 6/17/20 2:14 AM, Timon Gehr wrote:
>>>
>>>> Embellishments are immediate (ctors, allowing void init, opAssign, etc). To the extent this type can be pried open in @safe code, these are definitely bugs in the existing language.
>>>
>>> Not all of them, though the fact that it has a default constructor `this(T value){ this.value = value; }` is a bug.
>>
>> Ehm. The structure given was a minimal sketch, not an attempt at a complete implementation. An implementation could choose to disable the constructor or define it as @system etc.
> 
> ...or the language would do all that work for you, under this DIP; no new user types required, no ctors/dtors/sets/gets, merely a @system in front of a variable. That something *can* be a library solution does not mean that it *should* be one (*cough* emplace *cough* move *cough*). The DIP *is* a solution to many of the "bugs" with @safe.
> 
> Writing @safe code in a module should be no different to writing @safe code outside that module.
> 
> @system int x;
> 
> Done.

Not even close. The crux of the matter is that forgetting to add @system to that variable makes @safe code do unsafe things with no diagnostic for the compiler. That's a problem with the safety system, regardless of the adoption of this DIP. We can't say "@safe D code is safe, except of course if you forget to insert @system on key variables, in which case it won't be with no warning."

The main merit of this DIP is to bring attention to the problem and collect a list of issues to look at.

Coming from the other side: assume all bugs in @safe mentioned in the DIP are fixed (as they should anyway) - the question is what would be the usefulness of it then. I think a DIP listing the bugs and then motivating its necessity without assuming they won't be fixed would be stronger.

On Wednesday, 17 June 2020 at 06:14:25 UTC, Timon Gehr wrote:
> On 17.06.20 03:12, Andrei Alexandrescu wrote:
>> [snip]
>> 
>> * And indeed the example with getPtr() illustrates an obvious bug. Safe code has no business calling into @system code.
>
> Under current language rules, it's not @safe code. That's the problem. Variable initializers have no safety annotations.

It seems to me as if this is the most important point that has been made so far.

There are two ways to interpret what Andrei is saying:

1) There is a bug with @safe that should be fixed. However, Timon notes that this is not part of the definition of @safe and the whole point of the DIP.

2) There is a bug in the program. In this case, he has argued elsewhere that @safe-ty reviews should happen on a module basis. That means the review would need to check variable initialization to be sure they are not calling @system code. This is because the compiler is not checking them for you. However, the whole point of this DIP is so that the compiler would do those checks for you.

Regardless, it means that @safe is not currently safe.

On 6/17/20 9:30 AM, Dennis wrote:
> On Wednesday, 17 June 2020 at 12:41:57 UTC, Andrei Alexandrescu wrote:
>>> Maybe you can make it work the way you envision, but what is to stop someone from coming along and adding some more @safe code to that module?
>>
>> A code review.
> 
> I thought the whole premise of @safe was that code review is inadequate for catching memory corruption bugs.

Modules that contain @trusted code need to be reviewed manually. We need to make clear in the documentation that it's not only the @trusted bits in the module; it's the entire module. (That is the case independently on the adoption of the DIP.) Modules that have only @safe code (no @trusted, no @system) should provide safety guarantees. The DIP improves on that in that it points to a number of issues with @safe that need fixing.

On 6/17/20 10:30 AM, jmh530 wrote:
> Regardless, it means that @safe is not currently safe.

Affirmative. Clearly this is the problem. Opting into @system by means of an variable annotation that is NOT FLAGGED IF MISSING is not helping.

Making @system opt-in is completely backwards.

On Wednesday, 17 June 2020 at 14:27:17 UTC, Andrei Alexandrescu wrote:
> [snip]
>
> Not even close. The crux of the matter is that forgetting to add @system to that variable makes @safe code do unsafe things with no diagnostic for the compiler. That's a problem with the safety system, regardless of the adoption of this DIP. We can't say "@safe D code is safe, except of course if you forget to insert @system on key variables, in which case it won't be with no warning."
>

That is a fair point.

One potential resolution would be to allow for @safe/@trusted/@system (with @system the default) variable initialization and prevent taking the address of a @system variable in a @safe function.