Fix Issue 3672 - Read-modify-write operations should not be allowed f…

…or shared variables.

src/expression.c

@@ -2127,6 +2127,11 @@ int Expression::checkModifiable(Scope *sc, int flag)
     return type ? 1 : 0;    // default modifiable
 }
 
+bool Expression::checkReadModifyWrite()
+{
+    return !type->isShared();
+}
+
 Expression *Expression::modifiableLvalue(Scope *sc, Expression *e)
 {
     //printf("Expression::modifiableLvalue() %s, type = %s\n", toChars(), type->toChars());
@@ -2168,6 +2173,36 @@ Expression *Expression::modifiableLvalue(Scope *sc, Expression *e)
     return new ErrorExp();
 }
 
+Expression *Expression::readModifyWrite(TOK rmwOp, Expression *ex)
+{
+    //printf("Expression::readModifyWrite() %s %s", toChars(), ex ? ex->toChars() : "");
+    if (checkReadModifyWrite())
+        return this;
+
+    // atomicOp uses opAssign (+=/-=) rather than opOp (++/--) for the CT string literal.
+    switch (rmwOp)
+    {
+        case TOKplusplus: case TOKpreplusplus:
+            rmwOp = TOKaddass;
+            break;
+
+        case TOKminusminus: case TOKpreminusminus:
+            rmwOp = TOKminass;
+            break;
+
+        default:
+            break;
+    }
+
+    deprecation("Read-modify-write operations are not allowed for shared variables. "
+                "Use core.atomic.atomicOp!\"%s\"(%s, %s) instead.",
+                Token::tochars[rmwOp], toChars(), ex ? ex->toChars() : "1");
+    return this;
+
+    // note: enable when deprecation becomes an error.
+    // return new ErrorExp();
+}
+
 
 /************************************
  * Detect cases where pointers to the stack can 'escape' the
@@ -5417,6 +5452,12 @@ int VarExp::checkModifiable(Scope *sc, int flag)
     return var->checkModify(loc, sc, type, NULL, flag);
 }
 
+bool VarExp::checkReadModifyWrite()
+{
+    //printf("VarExp::checkReadModifyWrite %s", toChars());
+    return (var->storage_class & STCshared) == 0;
+}
+
 Expression *VarExp::modifiableLvalue(Scope *sc, Expression *e)
 {
     //printf("VarExp::modifiableLvalue('%s')\n", var->toChars());
@@ -6641,6 +6682,8 @@ Expression *BinAssignExp::semantic(Scope *sc)
     if (e)
         return e;
 
+    e1->readModifyWrite(op, e2);
+
     if (e1->op == TOKarraylength)
     {
         // arr.length op= e2;
@@ -7565,6 +7608,12 @@ int DotVarExp::checkModifiable(Scope *sc, int flag)
     return e1->checkModifiable(sc, flag);
 }
 
+bool DotVarExp::checkReadModifyWrite()
+{
+    //printf("DotVarExp::checkReadModifyWrite %s", toChars());
+    return (var->storage_class & STCshared) == 0;
+}
+
 Expression *DotVarExp::modifiableLvalue(Scope *sc, Expression *e)
 {
 #if 0
@@ -10542,6 +10591,8 @@ Expression *PostExp::semantic(Scope *sc)
     if (e)
         return e;
 
+    e1->readModifyWrite(op);  // check whether rmw operation is allowed
+
     if (e1->op == TOKslice)
     {
         const char *s = op == TOKplusplus ? "increment" : "decrement";
@@ -10618,6 +10669,8 @@ PreExp::PreExp(TOK op, Loc loc, Expression *e)
 Expression *PreExp::semantic(Scope *sc)
 {
     Expression *e = op_overload(sc);
+    // printf("PreExp::semantic('%s')\n", toChars());
+
     if (e)
         return e;
 
@@ -11730,6 +11783,8 @@ Expression *PowAssignExp::semantic(Scope *sc)
     if (e)
         return e;
 
+    e1->readModifyWrite(op, e2);  // check whether rmw operation is allowed
+
     assert(e1->type && e2->type);
     if (e1->op == TOKslice || e1->type->ty == Tarray || e1->type->ty == Tsarray)
     {

src/expression.h

@@ -208,6 +208,12 @@ class Expression : public RootObject
     void checkNogc(Scope *sc, FuncDeclaration *f);
     bool checkPostblit(Scope *sc, Type *t);
     virtual int checkModifiable(Scope *sc, int flag = 0);
+
+    // check whether the expression allows RMW operations, error with rmw operator diagnostic if not.
+    // exp is the RHS expression, or NULL if ++/-- is used (for diagnostics)
+    Expression *readModifyWrite(TOK rmwOp, Expression *exp = NULL);
+    virtual bool checkReadModifyWrite();  // return true if the expression allows RMW operations.
+
     virtual Expression *checkToBoolean(Scope *sc);
     virtual Expression *addDtorHook(Scope *sc);
     Expression *checkToPointer();
@@ -653,6 +659,7 @@ class VarExp : public SymbolExp
     void checkEscape();
     void checkEscapeRef();
     int checkModifiable(Scope *sc, int flag);
+    bool checkReadModifyWrite();
     int isLvalue();
     Expression *toLvalue(Scope *sc, Expression *e);
     Expression *modifiableLvalue(Scope *sc, Expression *e);
@@ -882,6 +889,7 @@ class DotVarExp : public UnaExp
     DotVarExp(Loc loc, Expression *e, Declaration *var, bool hasOverloads = false);
     Expression *semantic(Scope *sc);
     int checkModifiable(Scope *sc, int flag);
+    bool checkReadModifyWrite();
     int isLvalue();
     Expression *toLvalue(Scope *sc, Expression *e);
     Expression *modifiableLvalue(Scope *sc, Expression *e);

test/fail_compilation/diag3672.d

@@ -0,0 +1,63 @@
+// PERMUTE_ARGS:
+// REQUIRED_ARGS: -de
+/*
+TEST_OUTPUT:
+---
+fail_compilation/diag3672.d(38): Deprecation: Read-modify-write operations are not allowed for shared variables. Use core.atomic.atomicOp!"+="(x, 1) instead.
+fail_compilation/diag3672.d(39): Deprecation: Read-modify-write operations are not allowed for shared variables. Use core.atomic.atomicOp!"+="(x, 1) instead.
+fail_compilation/diag3672.d(40): Deprecation: Read-modify-write operations are not allowed for shared variables. Use core.atomic.atomicOp!"-="(x, 1) instead.
+fail_compilation/diag3672.d(41): Deprecation: Read-modify-write operations are not allowed for shared variables. Use core.atomic.atomicOp!"-="(x, 1) instead.
+fail_compilation/diag3672.d(42): Deprecation: Read-modify-write operations are not allowed for shared variables. Use core.atomic.atomicOp!"+="(x, 1) instead.
+fail_compilation/diag3672.d(43): Deprecation: Read-modify-write operations are not allowed for shared variables. Use core.atomic.atomicOp!"+="(x, 2) instead.
+fail_compilation/diag3672.d(44): Deprecation: Read-modify-write operations are not allowed for shared variables. Use core.atomic.atomicOp!"-="(x, 3) instead.
+fail_compilation/diag3672.d(45): Deprecation: Read-modify-write operations are not allowed for shared variables. Use core.atomic.atomicOp!"|="(x, y) instead.
+fail_compilation/diag3672.d(46): Deprecation: Read-modify-write operations are not allowed for shared variables. Use core.atomic.atomicOp!"*="(x, y) instead.
+fail_compilation/diag3672.d(47): Deprecation: Read-modify-write operations are not allowed for shared variables. Use core.atomic.atomicOp!"/="(x, y) instead.
+fail_compilation/diag3672.d(48): Deprecation: Read-modify-write operations are not allowed for shared variables. Use core.atomic.atomicOp!"%="(x, y) instead.
+fail_compilation/diag3672.d(49): Deprecation: Read-modify-write operations are not allowed for shared variables. Use core.atomic.atomicOp!"&="(x, y) instead.
+fail_compilation/diag3672.d(50): Deprecation: Read-modify-write operations are not allowed for shared variables. Use core.atomic.atomicOp!"^="(x, y) instead.
+fail_compilation/diag3672.d(51): Deprecation: Read-modify-write operations are not allowed for shared variables. Use core.atomic.atomicOp!"<<="(x, y) instead.
+fail_compilation/diag3672.d(52): Deprecation: Read-modify-write operations are not allowed for shared variables. Use core.atomic.atomicOp!">>="(x, y) instead.
+fail_compilation/diag3672.d(53): Deprecation: Read-modify-write operations are not allowed for shared variables. Use core.atomic.atomicOp!">>>="(x, y) instead.
+fail_compilation/diag3672.d(54): Deprecation: Read-modify-write operations are not allowed for shared variables. Use core.atomic.atomicOp!"^^="(x, y) instead.
+fail_compilation/diag3672.d(55): Deprecation: Read-modify-write operations are not allowed for shared variables. Use core.atomic.atomicOp!"+="(ptr, 1) instead.
+fail_compilation/diag3672.d(56): Deprecation: Read-modify-write operations are not allowed for shared variables. Use core.atomic.atomicOp!"+="(ptr, 1) instead.
+fail_compilation/diag3672.d(57): Deprecation: Read-modify-write operations are not allowed for shared variables. Use core.atomic.atomicOp!"-="(ptr, 1) instead.
+fail_compilation/diag3672.d(58): Deprecation: Read-modify-write operations are not allowed for shared variables. Use core.atomic.atomicOp!"-="(ptr, 1) instead.
+---
+*/
+shared int x;
+shared int y;
+shared int* ptr;
+shared static this() { ptr = new int; } // silence null-dereference errors
+class NS { shared int x; }
+shared class S { int sx; }
+
+void main()
+{
+    ++x;
+    x++;
+    --x;
+    x--;
+    x += 1;
+    x += 2;
+    x -= 3;
+    x |= y;
+    x *= y;
+    x /= y;
+    x %= y;
+    x &= y;
+    x ^= y;
+    x <<= y;
+    x >>= y;
+    x >>>= y;
+    x ^^= y;
+    ++ptr;
+    ptr++;
+    --ptr;
+    ptr--;
+    NS ns = new NS;
+    ns.x++;
+    S s = new S;
+    s.sx++;
+}

test/fail_compilation/fail3672.d

@@ -0,0 +1,38 @@
+/*
+TEST_OUTPUT:
+---
+fail_compilation/fail3672.d(28): Deprecation: Read-modify-write operations are not allowed for shared variables. Use core.atomic.atomicOp!"+="(*p, 1) instead.
+fail_compilation/fail3672.d(32): Deprecation: Read-modify-write operations are not allowed for shared variables. Use core.atomic.atomicOp!"+="(*sfp, 1) instead.
+fail_compilation/fail3672.d(32): Error: '*sfp += 1' is not a scalar, it is a shared(SF)
+fail_compilation/fail3672.d(32): Error: incompatible types for ((*sfp) += (1)): 'shared(SF)' and 'int'
+---
+*/
+struct SF  // should fail
+{
+    void opOpAssign(string op, T)(T rhs)
+    {
+    }
+}
+
+struct SK  // ok
+{
+    void opOpAssign(string op, T)(T rhs) shared
+    {
+    }
+}
+
+void main()
+{
+    shared int x;
+    auto p = &x;
+    *p += 1;  // fail
+
+    shared SF sf;
+    auto sfp = &sf;
+    *sfp += 1;  // fail
+
+    shared SK sk;
+    auto skp = &sk;
+    sk += 1;  // ok
+    *skp += 1;  // ok
+}