Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/dmd/cli.d
Original file line number Diff line number Diff line change
Expand Up @@ -721,6 +721,8 @@ dmd -cov -unittest myprog.d
"destruct fields of partially constructed objects"),
Feature("rvaluerefparam", "rvalueRefParam",
"enable rvalue arguments to ref parameters"),
Feature("restrictiveshared", "restrictiveshared",
"implement a more restrictive shared", false),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"more restrictive shared" has no particular meaning

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

disallow reading and writing shared state then?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

];
}

Expand Down
14 changes: 14 additions & 0 deletions src/dmd/dscope.d
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ enum SCOPE
ignoresymbolvisibility = 0x0200, /// ignore symbol visibility
/// https://issues.dlang.org/show_bug.cgi?id=15907
onlysafeaccess = 0x0400, /// unsafe access is not allowed for @safe code
allowsharedaccess = 0x0800, /// allow shared access for the sub-expressions
free = 0x8000, /// is on free list

fullinst = 0x10000, /// fully instantiate templates
Expand Down Expand Up @@ -273,6 +274,19 @@ struct Scope
}


extern (C++) Scope* startAllowSharedAccess()
{
Scope* sc = this.push();
sc.flags = this.flags | SCOPE.allowsharedaccess;
return sc;
}

extern (C++) Scope* endAllowSharedAccess()
{
assert(flags & SCOPE.allowsharedaccess);
return pop();
}

/*******************************
* Merge results of `ctorflow` into `this`.
* Params:
Expand Down
96 changes: 84 additions & 12 deletions src/dmd/expressionsem.d
Original file line number Diff line number Diff line change
Expand Up @@ -1288,7 +1288,7 @@ private Expression resolvePropertiesX(Scope* sc, Expression e1, Expression e2 =
{
VarExp ve = cast(VarExp)e1;
VarDeclaration v = ve.var.isVarDeclaration();
if (v && ve.checkPurity(sc, v))
if (v && (ve.checkPurity(sc, v) || checkAccessShared(sc, ve.loc, v)))
return new ErrorExp();
}
if (e2)
Expand Down Expand Up @@ -1605,6 +1605,27 @@ private bool preFunctionParameters(Scope* sc, Expressions* exps)
return err;
}

/********************************************
* Issue an error if shared state is Accessed.
* Returns:
* true an error was issued
*/
private bool checkAccessShared(Scope* sc, Loc loc, VarDeclaration vd)
{
if (!global.params.restrictiveshared)
{
return false;
}

if (vd.type && vd.type.isShared() && !(sc.flags & SCOPE.allowsharedaccess))
{
loc.error("Invalid access to shared data `%s`", vd.toChars());
return true;
}

return false;
}

/********************************************
* Issue an error if default construction is disabled for type t.
* Default construction is required for arrays and 'out' parameters.
Expand Down Expand Up @@ -4011,8 +4032,50 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
return exp.expressionSemantic(sc);
}

void checkAccessSharedCall(CallExp exp)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No description.

{
assert (exp.f);
auto params = exp.f.getParameterList().parameters;
if (!exp.arguments || !exp.arguments.dim || !params || !params.dim)
return ;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space


{
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove and unindent the following

foreach (i, arg;*exp.arguments)
{
if (arg.op == TOK.variable)
{
auto ve = cast (VarExp) arg;
auto vd = ve.var.isVarDeclaration();
if (vd &&
vd.type &&
vd.type.isShared() &&
i < params.dim &&
!(*params)[i].type.isShared)
{
checkAccessShared(sc, arg.loc, vd);
}
}
}
}
}

override void visit(CallExp exp)
{

// @@@SHARED@@@
// overload resolution calls `resolvePropertiesX` which
// checks for shared access.
// We need to set allow shared for overload resolution
sc = sc.startAllowSharedAccess();
// after resolution is done
scope (exit)
{
sc = sc.endAllowSharedAccess();
if (exp.f && !exp.f.semantic3Errors
&& exp.f.type.isTypeFunction() !is null)
checkAccessSharedCall(exp);
}

static if (LOGSEMANTIC)
{
printf("CallExp::semantic() %s\n", exp.toChars());
Expand Down Expand Up @@ -6798,19 +6861,28 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
return;
}

// for static alias this: https://issues.dlang.org/show_bug.cgi?id=17684
if (exp.e1.op == TOK.type)
exp.e1 = resolveAliasThis(sc, exp.e1);

auto e1x = resolveProperties(sc, exp.e1);
if (e1x.op == TOK.error)
// shared allowed for resolving this
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea what this means.

// @@@SHARED@@@
{
result = e1x;
return;
sc = sc.startAllowSharedAccess();

// for static alias this: https://issues.dlang.org/show_bug.cgi?id=17684
if (exp.e1.op == TOK.type)
exp.e1 = resolveAliasThis(sc, exp.e1);

auto e1x = resolveProperties(sc, exp.e1);

if (e1x.op == TOK.error)
{
result = e1x;
return;
}
if (e1x.checkType())
return setError();
exp.e1 = e1x;

sc = sc.endAllowSharedAccess();
}
if (e1x.checkType())
return setError();
exp.e1 = e1x;

if (!exp.e1.type)
{
Expand Down
1 change: 1 addition & 0 deletions src/dmd/globals.d
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ struct Param
// https://issues.dlang.org/show_bug.cgi?id=14246
bool fieldwise; // do struct equality testing field-wise rather than by memcmp()
bool rvalueRefParam; // allow rvalues to be arguments to ref parameters
bool restrictiveshared; // restrict the use of shared to casting & shared functions

CppStdRevision cplusplus = CppStdRevision.cpp98; // version of C++ standard to support

Expand Down
1 change: 1 addition & 0 deletions src/dmd/globals.h
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ struct Param
// https://issues.dlang.org/show_bug.cgi?id=14246
bool fieldwise; // do struct equality testing field-wise rather than by memcmp()
bool rvalueRefParam; // allow rvalues to be arguments to ref parameters
bool restrictiveshared; // restrict the use of shared to casting & shared functions
CppStdRevision cplusplus; // version of C++ name mangling to support
bool markdown; // enable Markdown replacements in Ddoc
bool vmarkdown; // list instances of Markdown replacements in Ddoc
Expand Down
1 change: 1 addition & 0 deletions src/dmd/scope.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ class CPPNamespaceDeclaration;
#define SCOPEctor 0x0001 // constructor type
#define SCOPEcondition 0x0004 // inside static if/assert condition
#define SCOPEdebug 0x0008 // inside debug conditional
#define SCOPEnosharedcheck 0x0800 // allow shared access

// Flags that would be inherited beyond scope nesting
#define SCOPEnoaccesscheck 0x0002 // don't do access checks
Expand Down
22 changes: 22 additions & 0 deletions test/compilable/test_shared.d
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/*
REQUIRED_ARGS:-preview=restrictiveshared
TEST_OUTPUT:
---
---
*/


int f1(shared int x)
{
auto r = sync(&x, 22);
return r;

}

int sync(shared int *x, int y)
{
auto unshared_x = cast()x;

return *unshared_x = y;
}

24 changes: 24 additions & 0 deletions test/fail_compilation/fail_shared.d
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/*
REQUIRED_ARGS:-preview=restrictiveshared
TEST_OUTPUT:
---
fail_compilation/fail_shared.d(15): Error: Trying to Access shared state `x`
fail_compilation/fail_shared.d(16): Error: Trying to Access shared state `x`
fail_compilation/fail_shared.d(17): Error: Trying to Access shared state `x`
fail_compilation/fail_shared.d(22): Error: Trying to Access shared state `x`
---
*/


int f1(shared int x)
{
x += 7; // Error: Trying to accsess shared state x
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the error should say "Invalid access to shared data x"
'Trying' feels like an intent rather than an error.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spelling errors

x = 22; // Error: Trying to accsess shared state x
return x; // Error: Trying to accsess shared state x
}

int sync(shared int *x, int y)
{
return x = y; // Error: Trying to accsess shared state x
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

integers cannot be assigned to pointers anyway

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

auto-deref.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (e2.type == e1.type.nextof) *e1 = e2

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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*`

}