Thread overview
[Issue 18985] bad error message for += operation on shared Object
Jun 14, 2018
ag0aep6g
Jun 14, 2018
OlegZ
Jun 14, 2018
OlegZ
Jun 15, 2018
RazvanN
Jun 15, 2018
RazvanN
June 14, 2018
https://issues.dlang.org/show_bug.cgi?id=18985

ag0aep6g <ag0aep6g@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |diagnostic
                 CC|                            |ag0aep6g@gmail.com
            Summary|shared variable of          |bad error message for +=
                   |user-defined type with      |operation on shared Object
                   |opOpAssign                  |
           Severity|major                       |normal

--- Comment #1 from ag0aep6g <ag0aep6g@gmail.com> ---
(In reply to OlegZ from comment #0)
> class Some {
> 	static immutable Some One = new immutable Some();
> 	auto opOpAssign( op )( Some val ) { writefln( "Some.op" ~ op ); }
> }
> auto var = new shared Some();
> var += Some.One;
> // and we'v got here: Error: read-modify-write operations are not allowed
> for `shared` variables. Use `core.atomic.atomicOp!"+="(var, One)` instead.

There a couple errors in your opOpAssign. If you implement it correctly, you don't get an error:

----
class Some
{
    static immutable Some One = new immutable Some();
    auto opOpAssign( string op )( const Some val ) shared
    {
        import std.stdio: writefln;
        writefln( "Some.op" ~ op );
    }
}

void main()
{
    auto var = new shared Some();
    var += Some.One; /* no error */
}
----

But the "read-modify-write" error you got is wrong/confusing. It should be fixed.

A simplified test for the bad error message:

----
Object foo;
shared Object bar;

void main()
{
    foo += 1; /* Error: foo += 1 is not a scalar, it is a object.Object */
    bar += 1; /* Error: read-modify-write operations are not allowed for shared
variables. Use core.atomic.atomicOp!"+="(bar, 1) instead. */
}
----

The first error message is a bit weird (shouldn't it say "foo is not a scalar"?), but one can figure out the meaning.

The second error message is bad. `shared` is a red herring. Would be better if the first message would be displayed here, too.


> 1) this error should be a warning only, cause not a beginners usually know what they do. and compiler should allow  to do what they want.

By that logic these should also be warnings instead of errors:

1) assignment as a condition: int x; if (x = 1) {}
2) narrowing conversions: long x; int y; y = x;
3) all the @safe checks: void main() @safe { int* p = void; }

You see that's not how D rolls. But you can usually tell the compiler to "just do it" by using the unsafe tools `cast` and `@trusted`. In the case of `shared`, you can always cast it away and do whatever you want then.


> 2) this error is stupid cause we can't use atomicOp for user defined type
> (in most cases)

Yup. The error message is bad.


> 3) "shared" keyword is stupid thing in D.

Bugzilla is not particularly suited for rants about how D sucks. The General section of the forum is the right place for that.

https://forum.dlang.org/group/general


> 4) synchronized method(became shared) differ from methods with
> auto meth(...) { synchronized( this ) { ... } }
> meaning is same but behavior is not same - WTF?

This isn't directly related to the opOpAssign thing anymore, is it? If there's something wrong with `synchronized`, please file a separate issue. Or maybe make a thread on the forum first to find out if it's worth filing. Might just be a design decision that you don't like.


I'm adding the "diagnostic" keyword here, because the error message you got is
confusing and could be improved.
I'm changing the title reflect that the error message is the problem.
I'm reducing the severity to "normal", because the error message is all that
needs fixing here. The code works if opOpAssign is written correctly.

--
June 14, 2018
https://issues.dlang.org/show_bug.cgi?id=18985

--- Comment #2 from OlegZ <black80@bk.ru> ---
yes, the error disappeared.
for non-shared vars should be used non-shared method overloading (when it
needed) or non-shared another class. not easy.
and about to change wrong error text I filled before another bug/enhancement
https://issues.dlang.org/show_bug.cgi?id=18961 (second comment)

--
June 14, 2018
https://issues.dlang.org/show_bug.cgi?id=18985

--- Comment #3 from OlegZ <black80@bk.ru> ---
yes, the error disappeared.
for non-shared vars should be used non-shared method overloading (when it
needed) or non-shared another class. not easy.
and about to change wrong error text I filled before another bug/enhancement
https://issues.dlang.org/show_bug.cgi?id=18961 (second comment)

--
June 15, 2018
https://issues.dlang.org/show_bug.cgi?id=18985

RazvanN <razvan.nitu1305@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |razvan.nitu1305@gmail.com

--- Comment #4 from RazvanN <razvan.nitu1305@gmail.com> ---
AnyObject foo;
shared AnyObject bar;

void main()
{
    foo += 1; /* Error: foo += 1 is not a scalar, it is a object.Object */
    bar += 1; /* Error: read-modify-write operations are not allowed for shared
variables. Use core.atomic.atomicOp!"+="(bar, 1) instead. */
}

We can distinguish 4 situations here:

1. AnyObject does not define any opOpAssign methods => error message should be in both situations "$object is not scalar"

2. Anyobject does define opOpAssign but it's not shared => foo passes compilation, but bar does not and the most helpful error message would be "read-modify-write operations are not allowed for shared variables. Use core.atomic.atomicOp!"+="(bar, 1) instead or implement a shared opOpAssign for type AnyObject"

3. AnyObject defines opOpAssign shared but not normal opOpAssign => foo += 1 fails with "object is not scalar message" and bar works.

4. AnyObject defines both shared/non-shared opOpAssign => both examples compile

--
June 15, 2018
https://issues.dlang.org/show_bug.cgi?id=18985

--- Comment #5 from RazvanN <razvan.nitu1305@gmail.com> ---
PR: https://github.com/dlang/dmd/pull/8360

--
June 21, 2018
https://issues.dlang.org/show_bug.cgi?id=18985

--- Comment #6 from github-bugzilla@puremagic.com ---
Commits pushed to master at https://github.com/dlang/dmd

https://github.com/dlang/dmd/commit/ff23072e11750a83da1078705a16f7d160e793d9 Fix Issue 18985 - bad error message for += operation on shared Object

https://github.com/dlang/dmd/commit/f6a4a949d24e3174149069c855d67f52fb31c71c Merge pull request #8360 from RazvanN7/Issue_18985

Fix Issue 18985 - bad error message for += operation on shared Object

--
June 21, 2018
https://issues.dlang.org/show_bug.cgi?id=18985

github-bugzilla@puremagic.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--