Feedback Thread: DIP 1035-- at system Variables--Community Review Round 1

[Timon Gehr]

On 10.06.20 10:39, Mike Parker wrote:
> This is the feedback thread for the first round of Community Review of 
> DIP 1035, "@system Variables".
> ...

I think this is a very well-written DIP that addresses an important problem.

However, I don't think the examples should be using `assert` to validate 
input data. At the very least, those asserts should be in `in` 
contracts, but even then, I am not sure if the semantics of `assert` 
supports your use case. In particular, does `-release` mean "disable 
memory safety checks" like `-noboundscheck` does?

(Besides that, probably `assert` should not be used at all, at least 
outside of contracts, if you care about memory safety:

https://dlang.org/spec/contracts.html

"Undefined Behavior: The subsequent execution of the program after an 
assert contract is false.")


Also, making initialization of `@system` variables `@safe` is not sound. 
`@system` variables are variables that need to satisfy additional 
invariants. The constructor has to establish those invariants. Memory 
safety cannot depend on the correctness of a `@safe` constructor.

Consider the following slightly adapted example from the DIP:

enum Opcode : ubyte {
     decrement, increment, print,
}

struct VmInstruction {
     @system Opcode opcode; // this need not be private, just a valid 
enum member
     this(Opcode opcode) @safe {
         this.opcode = opcode; // forgot to check
     }
}

int gCounter;
void decrementImpl() {gCounter++;};
void incrementImpl() {gCounter--;};
void printImpl() {import std; writeln(gCounter);};

immutable void function()[3] jumpTable = [
    &decrementImpl, &incrementImpl, &printImpl,
];

void execute(VmInstruction[] code) @trusted {
     foreach(instruction; code) {
         // indexing using .ptr to avoid bounds checks
         jumpTable.ptr[instruction.opcode]();
     }
}

void main() @safe {
     auto code = [VmInstruction(cast(Opcode)20)];
     execute(code);
}



Minor:
- "Ownsership and borrowing in D"
- "static initializtion"