add -preview=nosharedaccess flag and implementation

[WalterBright]

This is a simple implementation of the idea that shared data should not be directly accessed, but should only be accessed via core.atomic primitives.

It's intended to replace #10142 which I don't think is the right approach, and is incomplete.

[dlang-bot]

Thanks for your pull request, @WalterBright!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

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#10209"

[WalterBright]

This is important because @TurkeyMan needs it to clinch a design win for D. It's structured to no interfere with the rest of the compiler. He's also set up to use it immediately and see if it is suitable or not.

[TurkeyMan]

Should the test prove that writes are denied too?
I feel like it might also be good to prove that taking the address is okay, and passing by pointer/ref.

[WalterBright]

line 25

[TurkeyMan]

Sorry, I missed that one hiding in there.

[TurkeyMan]

I suppose those other tests are implied by the function arguments.

[WalterBright]

done

[stefan-koch-sociomantic]

please assert that passing a shared argument to non shared parameter fails like it is supposed to.

[TurkeyMan]

Thank you so much!

[WalterBright]

@TurkeyMan I expect you'll give it a good thrashing, and expect to have to tweak it here and there.

[TurkeyMan]

I most certainly will. I have been implementing our machinery in D, which is tricky, because I have to maintain compatibility with the C++, but it's not really a port rather than a re-image of the API with shared stuff woven through.
I'll try and share some of the utility types that fall out of this as I'm happy with them.

[thewilsonator]

speaking of incomplete, this needs to be disallowed:

@safe void foo(shared(int)* p)
{
    auto a = cast()p;
}

otherwise e.g. Mutexes guarding shared objects, are not protected by the type system.

[ghost]

This is nicely implemented but honestly this looks like a warning with -we forced.

[WalterBright]

@thewilsonator the cast() only affects the "head" of the type, so the cast()p will still produce shared(int)*, so the cast is safe.

[WalterBright]

this looks like a warning

The idea is to make it a non-optional language feature.

[thewilsonator]

touché. Ignore that then.

[WalterBright]

I'll update my proposed DIP on this to match.

dlang/DIPs#159

[UplinkCoder]

The following should fail, and yet it does not

int f1(shared int x)
{
    read(x); // should fail trying to pass shared argument to non-shared parameter!!!
}
int read(int x) { return x; }

Do you see now why I implemented it the way I did?

[Geod24]

The following should fail, and yet it does not

int is a value type, shared does not matter. Replace shared with immutable and your example still compiles, as it should.

[Geod24]

This is not covered by the test

[WalterBright]

fixed.

[UplinkCoder]

@Geod24 I am not sure if we want to get into the habit of special casing storage classes on basic types?

[atilaneves]

It's true that copying an int is reading it, and that reading something that might be changed by another thread is bad. I'm thinking how bad it is to pass a potentially garbled value to another function.

[TurkeyMan]

So we're talking about this right?

void fun()
{
  shared int x = 10;
  int y = x;
}

It's not clear to me what the shared should actually mean in this case... since we're talking int, it's obviously perfectly safe, so the assignment is fine. (you could replace with immutable and it's equally fine)
If x were something more interesting than int, it's still thread-local, but that makes me start to wonder why the local might have been marked shared?

I suppose this is valid:

void fun()
{
  shared int x = 10;
  shareWithOtherThreads(&x);
  int y = x;
}

Now x is not necessarily thread-local... although it IS on the local stack, so any foreign thread is gonna be hitting dangling stack memory in a few nanoseconds. Safe to expect that function must join internally.

This may be the trouble case:

void fun()
{
  shared int x = 10;
  shareWithOtherThreads(&x);
  int y = x;
  joinOtherThreadsThatMayStillHaveX();
}

This is highly contrived, but it's a theoretical problem case with shared local values assigning back to non-shared.

[WalterBright]

The following should fail, and yet it does not

It fails now.

[TurkeyMan]

I'm not sure failing is right, but it's the conservative option, and we should do that for now until we can prove any situation where that assignment should be expected to work, and understand the context.

[WalterBright]

This should fail:

int i;
shared int j;
i = j;

because it is reading j. Passing a shared value as a parameter also requires reading it, and so should fail. Shared data should be passed by pointer or ref or read atomically.

[Geod24]

It's true that copying an int is reading it, and that reading something that might be changed by another thread is bad. I'm thinking how bad it is to pass a potentially garbled value to another function.

That's true, but the failure should be reported at the point the variable is read.
That is:

shared int val;
int read(int x) { return x; }
void main () {
    read(val); // This fails
    shared int* gelVal = ...;
    read(*getVal); // Fails
}

But the originally provided snipped should not fail, because it does not read a shared variable.

[WalterBright]

I simply don't understand your comment, in that it isn't clear what you tested this PR doing and what you expected it to and what was the piece of code you tested.

[Geod24]

Test:

void foo() { shared int i; }

Expected result: Compiles
Actual result:

test.d(1): Error: direct access to shared i is not allowed, see core.atomic

[WalterBright]

@Geod24 That's because the code generated is:

shared int i = 0;

Note the initialization. That's a write to i. To not initialize it, use = void. What to do about initialization of shared variables is a bit tricky (as it also involves constructors which can execute arbitrary code), so I decided to be conservative in the initial version.

Manu will be using this initially, so I'm going to rely on his experience for some guidance here.

[thewilsonator]

Not that is should be cause to delay testing but that error message should definitely be improved since is not at all immediately obvious that is contains an initialisation that is a write.

[WalterBright]

Polishing error messages is good but is something we should do after the design has been shown to be successful.

Essentially, @TurkeyMan needs this PR now, and it doesn't hurt anything to get it incorporated, since the behavior is enabled with a switch.

[andralex]

This will be interesting.

[andralex]

This line and the previous one should be doubly indented lest there is confusion with the body of the if statement.

[WalterBright]

added { ... } for that.

[Geod24]

What to do about initialization of shared variables is a bit tricky (as it also involves constructors which can execute arbitrary code), so I decided to be conservative in the initial version.

Default initialization can never execute arbitrary code.
When it comes to constructors, it sounds to me like strongly pure ctors could give us the guarantees we need, but I don't mind leaving that discussion for another PR.
However, having default initialization working at global scope but not local scope is just terrible.

[TurkeyMan]

I am concerned about the initialisation logic in this patch too, but I doubt I'll have issues with it personally, since in all my use cases, data is not constructed shared, it's promoted.
I think the construction probably needs to be determined before this compiler switch is documented though.
I think there are a few cases that should work and make sense:

1. default construction should always work, it doesn't run code (I think this catches most cases users would encounter)
2. shared constructors should work. It might be annoying to cast shared away from this to actually do construction, but that's a matter for the future.
3. pure constructors (with only scope arguments) may produce an object that could initialise a shared instance.

I expect 2 should work right now, and 1 should be made to work. 3 and maybe options to make 2 smoother could be considered in the future.

[thewilsonator]

This fails to compile

private extern(C) void _d_setSameMutex(shared Object ownee, shared Object owner) nothrow;

void setSameMutex(shared Object ownee, shared Object owner)
{
    _d_setSameMutex(ownee, owner);
}

It seems classes are unhandled.