This is the feedback thread for the Final Review of DIP 1035, "@system Variables".

===================================
THIS IS NOT A DISCUSSION THREAD

Posts in this thread must adhere to the feedback thread rules outlined in the Reviewer Guidelines (and listed at the bottom of this post).

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

That document also provides guidelines on contributing feedback to a DIP review. Please read it before posting here. If you would like to discuss this DIP, please do so in the discussion thread:

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

==================================

You can find DIP 1035 here:

https://github.com/dlang/DIPs/blob/4d73e17901a3a620bf59a2a5bfb8c433069c5f52/DIPs/DIP1035.md

The review period will end at 11:59 PM ET on March 5, or when I make a post declaring it complete. Feedback posted to this thread after that point may be ignored.

At the end of the Final Review, the DIP will be forwarded to the language maintainers for Formal Assessment.

==================================
Posts in this thread that do not adhere to the following rules will be deleted at the DIP manager's discretion:

• All posts must be a direct reply to the DIP manager's initial post, with only two exceptions:
  • Any commenter may reply to their own posts to retract feedback contained in the original post
  • The DIP author may (and is encouraged to) reply to any feedback solely to acknowledge the feedback with agreement or disagreement (preferably with supporting reasons in the latter case)
• Feedback must be actionable, i.e., there must be some action the DIP author can choose to take in response to the feedback, such as changing details, adding new information, or even retracting the proposal.
• Feedback related to the merits of the proposal rather than to the contents of the DIP (e.g., "I'm against this DIP.") is allowed in Community Review, but not in Final Review, and must be backed by supporting arguments (e.g., "I'm against this DIP because..."). The supporting arguments must be reasonable. Obviously frivolous arguments waste everyone's time.
• Feedback should be clear and concise, preferably listed as bullet points (those who take the time to do an in-depth review and provide feedback in the form of answers to the questions in the reviewer guidelines will receive much gratitude). Information that is irrelevant to the DIP or is not provided in service of clarifying the feedback is unwelcome.

On Saturday, 19 February 2022 at 12:26:26 UTC, Mike Parker wrote:

In the "Example: int as pointer" section, the following sentence appears:

This sentence correctly recognizes that (absent incorrect @trusted code elsewhere) there is no memory-safety risk in allowing a value without indirections to escape from a function.

It also completely undermines the example's motivation. If there is no benefit to memory-safety from applying scope checking to data without indirections, then there is no justification for enabling such checks in all @safe code, even if they may occasionally be "desirable" for other, non-memory-safety-related reasons.

Later, in the "Description" section, we find the following sentence:

The only justification for this appears to be the example discussed above.

Both this sentence, and the example that attempts to support it, should be removed from the DIP.

On Saturday, 19 February 2022 at 12:26:26 UTC, Mike Parker wrote:

Wouldn't putting the handle in union with void[1] work?

This part concerns me:

Doesn't that mean that this is allowed:

ref const identity(T)(return ref const T var){return var;}

@safe void main()
{ auto x = someContainer.internalRepresentation.identity;
}

? It defeats the purpose of disallowing reading @system variables in @safe code.

struct T
{
    @system int y;
    @system int z = 3; // allowed
    this(int y, int z) @safe
    {
        this.y = y; // allowed, this is initialization
        this.y = y; // second time disallowed, this is assignment to a `@system` variable
        this.z = z; // disallowed, this is assignment
    }
}

The third comment is wrong according to my test:

import std.stdio;

struct Int
{ int quantity;
  this(int q, string unused){quantity = q;}
  this(int q)
  { quantity = q;
    writeln("constructed");
  }
  void opAssign(int q)
  { quantity = q;
    writeln("assigned");
  }
}

struct T
{ Int y;
  Int z = Int(3, ""); // allowed
  this(int y, int z)
  { writeln(1);
    this.y = y; // allowed, this is initialization
    writeln(2);
    this.y = y; // second time disallowed, this is assignment to a `@system` variable
    writeln(3);
    this.z = z; // disallowed, this is assignment
  }
}

void main(){auto x = T(5, 10);}

outputs

1
constructed
2
assigned
3
constructed

On Saturday, 19 February 2022 at 13:01:23 UTC, rikki cattermole wrote:

The original idea was to be consistent with the @safe by default DIP, which is now dead, so I'll change it.

On Friday, 25 February 2022 at 21:46:25 UTC, Dukc wrote:

No, void[1] is not a type with unsafe values.

Yes, I'll fix that.

Ditto.

On Monday, 21 February 2022 at 20:02:04 UTC, Paul Backus wrote:

I'll summarize my response from the Discussion Thread here. The idea is not to add scope checking to plain integers for non-memory-safety-related reasons, but to be able to create custom types that represent an indirection backed by an integer field. A pointer is also just an integer under the hood.

But indeed, the DIP does not demonstrate what kind of @trusted code this enables, and there are cases where you might want unsafe values without scope checking (such as bool), so maybe it shouldn't be an effect of @system members.

On Saturday, 19 February 2022 at 12:26:26 UTC, Mike Parker wrote:

This review round has ended. Thanks to everyone who provided feedback.