Conversation
|
Thanks for your pull request, @UplinkCoder! Bugzilla referencesYour 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 locallyIf 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#10142" |
UplinkCoder
force-pushed
the
shared_
branch
2 times, most recently
from
e4b602d to
8f85d33
Compare
|
You should rebase this on nicks |
UplinkCoder
force-pushed
the
shared_
branch
8 times, most recently
from
b96ebf8 to
3851551
Compare
|
|
||
| int f1(shared int x) | ||
| { | ||
| x += 7; // Error: Trying to accsess shared state x |
There was a problem hiding this comment.
I think the error should say "Invalid access to shared data x"
'Trying' feels like an intent rather than an error.
| return exp.expressionSemantic(sc); | ||
| } | ||
|
|
||
| void checkAccessSharedCall(CallExp exp) |
src/dmd/expressionsem.d
Outdated
| override void visit(CallExp exp) | ||
| { | ||
| // @@@SHARED@@@ | ||
| // we need to set allow shared for overload resolution |
There was a problem hiding this comment.
why should this affect overload resolution?
There was a problem hiding this comment.
why indeed :)
because for some reason overload-resolution needs to resolve properties.
|
|
||
| auto e1x = resolveProperties(sc, exp.e1); | ||
| if (e1x.op == TOK.error) | ||
| // shared allowed for resolving this |
There was a problem hiding this comment.
I have no idea what this means.
src/dmd/dscope.d
Outdated
| } | ||
|
|
||
|
|
||
| extern (C++) Scope* startRelaxShared() |
There was a problem hiding this comment.
- No description
- I have no idea what "relaxed shared" means
|
|
||
| int sync(shared int *x, int y) | ||
| { | ||
| return x = y; // Error: Trying to accsess shared state x |
There was a problem hiding this comment.
integers cannot be assigned to pointers anyway
There was a problem hiding this comment.
if (e2.type == e1.type.nextof) *e1 = e2
There was a problem hiding this comment.
auto-deref.
You said that the other day, but I don't get it...
int y = 10;
int* x;
x = y;error : cannot implicitly convert expression `y` of type `int` to `int*`
| Feature("rvaluerefparam", "rvalueRefParam", | ||
| "enable rvalue arguments to ref parameters"), | ||
| Feature("restrictiveshared", "restrictiveshared", | ||
| "implement a more restrictive shared", false), |
There was a problem hiding this comment.
"more restrictive shared" has no particular meaning
There was a problem hiding this comment.
disallow reading and writing shared state then?
There was a problem hiding this comment.
Personally, I'd go with 'data' or 'values', in my melon 'state' means something a little more specific; it evokes emotions of state machines or perhaps the 'state' of whether something is shared or not, like "reading or writing the shared-ness of the thing"... I mean, it's obvious that's not what this error means, it's just that I would lean away from using the word 'state' for those reasons. Is there precedent elsewhere?
src/dmd/expressionsem.d
Outdated
| { | ||
| assert (exp.f); | ||
| auto params = exp.f.getParameterList().parameters; | ||
| if (exp.arguments && exp.arguments.dim && params && params.dim) |
There was a problem hiding this comment.
Please write this function as
if (!(exp.arguments && exp.arguments.dim && params && params.dim)
return;
foreach (i, arg;*exp.arguments)
{
if (arg.op != TOK.variable)
continue;
...
}
|
@UplinkCoder please take a look at dlang/druntime#2679 (comment) |
| assert (exp.f); | ||
| auto params = exp.f.getParameterList().parameters; | ||
| if (!exp.arguments || !exp.arguments.dim || !params || !params.dim) | ||
| return ; |
| if (!exp.arguments || !exp.arguments.dim || !params || !params.dim) | ||
| return ; | ||
|
|
||
| { |
There was a problem hiding this comment.
remove and unindent the following
|
@UplinkCoder It seems like Walter has taken the matter into his hands [1]. Maybe you can take a look at his implementation and see which one is better. [1] #10209 |
9 participants
This will block read/write access to shared variables
execpt for the casting shared away operation, and when passing a shared argument to a shared parameter.