This is the discussion thread for the second round of Community Review of DIP 1035, "@system Variables":

https://github.com/dlang/DIPs/blob/c39f6ac62210e0604dcee99b0092c1930839f93a/DIPs/DIP1035.md

The review period will end at 11:59 PM ET on March 11, or when I make a post declaring it complete. Discussion in this thread may continue beyond that point.

Here in the discussion thread, you are free to discuss anything and everything related to the DIP. Express your support or opposition, debate alternatives, argue the merits, etc.

However, if you have any specific feedback on how to improve the proposal itself, then please post it in the feedback thread. The feedback thread will be the source for the review summary that I will write at the end of this review round. I will post a link to that thread immediately following this post. Just be sure to read and understand the Reviewer Guidelines before posting there:

https://github.com/dlang/DIPs/blob/master/docs/guidelines-reviewers.md

And my blog post on the difference between the Discussion and Feedback threads:

https://dlang.org/blog/2020/01/26/dip-reviews-discussion-vs-feedback/

Please stay on topic here. I will delete posts that are completely off-topic.

On Thursday, 25 February 2021 at 09:21:20 UTC, Mike Parker wrote:

>
> However, if you have any specific feedback on how to improve the proposal itself, then please post it in the feedback thread. The feedback thread will be the source for the review summary that I will write at the end of this review round. I will post a link to that thread immediately following this post.

The Feedback Thread is here:

https://forum.dlang.org/post/eivezuohaejnvvlhaeec@forum.dlang.org

On 25.02.21 10:21, Mike Parker wrote:
> This is the discussion thread for the second round of Community Review of DIP 1035, "@system Variables":
> 
> https://github.com/dlang/DIPs/blob/c39f6ac62210e0604dcee99b0092c1930839f93a/DIPs/DIP1035.md 

Big thanks to Dennis and Paul for championing this! I've only given the DIP a cursory read, but it looks great to me.

> Example: User-Defined Slice

Since ptr and length are private, they are only accessible to member functions of IntSlice. I don't think there is any way to prove memory safety of the implementation of this.

>  Instead, every function that touches ptr and length, including the @safe constructor, must be manually checked.

Right. So the idea is to minimize access to .ptr/.length for member functions of IntSlice. This can be done with:

  struct IntSlice
  {
    private static struct Slice
    {
        private int* ptr;
        private size_t length;

      @trusted:

        this(int[] src)
        {
             ptr = src.ptr;
             length = src.length;
        }

        int[] opSlice()
        {
             return ptr[0 .. length];
        }
     }

     // Now, the only access is via @trusted functions

   @safe:

    private Slice slice;

    this(int[] src)
    {
        slice = Slice(src);
    }

    ref int opIndex(size_t i)
    {
        return slice[][i]; // note no assert(i < length) needed
    }

    // ... every function goes here ...
  }

So, the general idea is to restrict access to `@system` fields by encapsulating them separately, and providing a minimized @trusted interface to them.

> Example: Short String

This example boils down to an invariant not being maintained if the struct is void-initialized. A couple simple solutions come to mind:

1. disallow void initialization in @safe code if the struct has a constructor

2. disallow void initialization if the struct has an invariant

I suggest that the ShortString struct is not properly written, because it has a hidden invariant that was not made clear with an explicit:

    invariant { assert(length <= data.length); }

On 2/26/2021 1:57 AM, Walter Bright wrote:
> 2. disallow void initialization if the struct has an invariant

Thinking about this some more.

An invariant specifies a subset of values that the fields could have, while a void initialization means any random values are acceptable.

Therefore (2) makes sense.

On 26.02.21 10:49, Walter Bright wrote:
>  > Example: User-Defined Slice
> 
> Since ptr and length are private, they are only accessible to member functions of IntSlice. I don't think there is any way to prove memory safety of the implementation of this.
> 
>  >  Instead, every function that touches ptr and length, including the @safe constructor, must be manually checked.
> 
> Right. So the idea is to minimize access to .ptr/.length for member functions of IntSlice. This can be done with:
> 
>    struct IntSlice
>    {
>      private static struct Slice
>      {
>          private int* ptr;
>          private size_t length;
> 
>        @trusted:
> 
>          this(int[] src)
>          {
>               ptr = src.ptr;
>               length = src.length;
>          }
> 
>          int[] opSlice()
>          {
>               return ptr[0 .. length];
>          }
>       }
> 
>       // Now, the only access is via @trusted functions
> 
>     @safe:
> 
>      private Slice slice;
> 
>      this(int[] src)
>      {
>          slice = Slice(src);
>      }
> 
>      ref int opIndex(size_t i)
>      {
>          return slice[][i]; // note no assert(i < length) needed
>      }
> 
>      // ... every function goes here ...
>    }
> 
> So, the general idea is to restrict access to `@system` fields by encapsulating them separately, and providing a minimized @trusted interface to them.


Yes, that's the idea, but this is a convention. It's not possible now to make the compiler check your convention. @system fields address this limitation.

On 26.02.21 11:08, Walter Bright wrote:
> On 2/26/2021 1:57 AM, Walter Bright wrote:
>> 2. disallow void initialization if the struct has an invariant
> 
> Thinking about this some more.
> 
> An invariant specifies a subset of values that the fields could have, while a void initialization means any random values are acceptable.
> 
> Therefore (2) makes sense.


An invariant is anything that remains true throughout the execution, and it's not always easy to check. (2) makes sense, but it's not a substitute for @system fields. @safe code shouldn't be able to mess with unsafe values at all, not only through `void` initialization, and @system code should be able to define its own unsafe values.

On Thursday, 25 February 2021 at 09:21:20 UTC, Mike Parker wrote:
> This is the discussion thread for the second round of Community Review of DIP 1035, "@system Variables":
>
> [...]

Well done. Everything in the direction of safety if very important to our company. Great work.

On Friday, 26 February 2021 at 09:57:03 UTC, Walter Bright wrote:
> > Example: Short String
>
> This example boils down to an invariant not being maintained if the struct is void-initialized.

Note that it's not just void initialization. Overlap in a union and reinterpreting casts can also create instances with broken invariants; e.g.,

union U
{
    int n;
    ShortString s;
}

@safe
void main()
{
    U u = { n: 0xDEADBEEF };
    writeln(u.s[]); // undefined behavior
}