Fix Issue 17864 - POD struct not equivalent to primitive type in comparison

[RazvanN7]

I have to admit that I am terribly confused about shared. So this code:

struct A { int a; }
void g()
{
	shared A a;
	A b;
	a=b; //converts
	assert(a==b); //fail
}

would print : "Error: incompatible types for ((a) is (b)): 'shared(A)' and 'A'", but the comparison succeeds if a's type qualifier is changed to immutable (and a = b is commented, obviously). Now, I am actually surprised that a=b compiles (from what I knew about shared, you were supposed to synchronize accesses to shared variables, although a is not global so it's not subject to concurrency issues). If a=b works there's no reason for a==b to fail compilation. The current fix is a noobish attempt from someone who doesn't really understand how shared works, so if there are any shared experts out there, please destroy!

[dlang-bot]

Thanks for your pull request and interest in making D better, @RazvanN7! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the annotated coverage diff directly on GitHub with CodeCov's browser extension
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	17864	minor	POD struct not equivalent to primitive type in comparison

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + dmd#8358"

[RazvanN7]

why is immutable-mutable ok, immutable-shared ok, but shared-mutable not ok?

[RazvanN7]

commenting the if doesn't seem to influence any code in the test suite.

[Geod24]

Did you try to implement a custom, non synchronized opEquals ?

[schveiguy]

OK, so writing to shared isn't "special" in the compiler. You can do this just fine:

shared int a;
a = 5;

The only thing that the compiler complains about is read/modify/write operations. And this restriction is pretty crude as well:

shared int a;
a++; // error, use atomicInc
a = a + 1; // no complaints

So really the benefit of shared is simply to give you a type-system mechanism to ensure you can do the right thing. It really doesn't enforce the right thing.

IMO, I think shared should be pretty near useless, you should have to cast it away to be able to use it. And that's not what the original restriction does anyway -- if it's not OK to compare mutable to shared, why is it OK to compare shared to shared? I mean, BOTH are subject to races!

I'd say the restriction is misguided at best, and this PR makes sense in the world where shared is "usable". But ultimately, I think shared should simply be locked down completely.

[schveiguy]

I'll approve this, knowing that shared is a mess and will continue to be a mess after this PR is merged :)

[andralex]

So we're overdue a major DIP defining shared properly. Until we do so, we should not expand the code allowed by shared, so I suggest @RazvanN7 to close this PR.