Skip to content

Commit

Permalink
Merge pull request #10172 from aG0aep6G/static-nested-in-pure
Browse files Browse the repository at this point in the history 
fix handling of static impure functions nested in pure functions (issues 20047 and 20050)
  • Loading branch information
@thewilsonator
thewilsonator authored Jul 15, 2019
2 parents 983cd0a + 084e40f commit 22113af
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 107 deletions.
125 changes: 21 additions & 104 deletions src/dmd/expression.d
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1089,77 +1089,13 @@ extern (C++) abstract class Expression : ASTNode
if (sc.flags & (SCOPE.ctfe | SCOPE.debug_))
return false;

/* Given:
* void f() {
* pure void g() {
* /+pure+/ void h() {
* /+pure+/ void i() { }
* }
* }
* }
* g() can call h() but not f()
* i() can call h() and g() but not f()
*/

// Find the closest pure parent of the calling function
FuncDeclaration outerfunc = sc.func;
FuncDeclaration calledparent = f;

if (outerfunc.isInstantiated())
{
// The attributes of outerfunc should be inferred from the call of f.
}
else if (f.isInstantiated())
{
// The attributes of f are inferred from its body.
}
else if (f.isFuncLiteralDeclaration())
{
// The attributes of f are always inferred in its declared place.
}
else
{
/* Today, static local functions are impure by default, but they cannot
* violate purity of enclosing functions.
*
* auto foo() pure { // non instantiated function
* static auto bar() { // static, without pure attribute
* impureFunc(); // impure call
* // Although impureFunc is called inside bar, f(= impureFunc)
* // is not callable inside pure outerfunc(= foo <- bar).
* }
*
* bar();
* // Although bar is called inside foo, f(= bar) is callable
* // bacause calledparent(= foo) is same with outerfunc(= foo).
* }
*/

while (outerfunc.toParent2() && outerfunc.isPureBypassingInference() == PURE.impure && outerfunc.toParent2().isFuncDeclaration())
{
outerfunc = outerfunc.toParent2().isFuncDeclaration();
if (outerfunc.type.ty == Terror)
return true;
}
while (calledparent.toParent2() && calledparent.isPureBypassingInference() == PURE.impure && calledparent.toParent2().isFuncDeclaration())
{
calledparent = calledparent.toParent2().isFuncDeclaration();
if (calledparent.type.ty == Terror)
return true;
}
}

// If the caller has a pure parent, then either the called func must be pure,
// OR, they must have the same pure parent.
if (!f.isPure() && calledparent != outerfunc)
// If the call has a pure parent, then the called func must be pure.
if (!f.isPure() && checkImpure(sc))
{
FuncDeclaration ff = outerfunc;
if (sc.flags & SCOPE.compile ? ff.isPureBypassingInference() >= PURE.weak : ff.setImpure())
{
error("`pure` %s `%s` cannot call impure %s `%s`",
ff.kind(), ff.toPrettyChars(), f.kind(), f.toPrettyChars());
return true;
}
error("`pure` %s `%s` cannot call impure %s `%s`",
sc.func.kind(), sc.func.toPrettyChars(), f.kind(),
f.toPrettyChars());
return true;
}
return false;
}
Expand Down Expand Up @@ -1205,41 +1141,11 @@ extern (C++) abstract class Expression : ASTNode
if (v.ident == Id.gate)
return false;

/* Accessing global mutable state.
* Therefore, this function and all its immediately enclosing
* functions must be pure.
*/
/* Today, static local functions are impure by default, but they cannot
* violate purity of enclosing functions.
*
* auto foo() pure { // non instantiated function
* static auto bar() { // static, without pure attribute
* globalData++; // impure access
* // Although globalData is accessed inside bar,
* // it is not accessible inside pure foo.
* }
* }
*/
for (Dsymbol s = sc.func; s; s = s.toParent2())
if (checkImpure(sc))
{
FuncDeclaration ff = s.isFuncDeclaration();
if (!ff)
break;
if (sc.flags & SCOPE.compile ? ff.isPureBypassingInference() >= PURE.weak : ff.setImpure())
{
error("`pure` %s `%s` cannot access mutable static data `%s`",
ff.kind(), ff.toPrettyChars(), v.toChars());
err = true;
break;
}

/* If the enclosing is an instantiated function or a lambda, its
* attribute inference result is preferred.
*/
if (ff.isInstantiated())
break;
if (ff.isFuncLiteralDeclaration())
break;
error("`pure` %s `%s` cannot access mutable static data `%s`",
sc.func.kind(), sc.func.toPrettyChars(), v.toChars());
err = true;
}
}
else
Expand Down Expand Up @@ -1308,6 +1214,17 @@ extern (C++) abstract class Expression : ASTNode
return err;
}

/*
Check if sc.func is impure or can be made impure.
Returns true on error, i.e. if sc.func is pure and cannot be made impure.
*/
private static bool checkImpure(Scope* sc)
{
return sc.func && (sc.flags & SCOPE.compile
? sc.func.isPureBypassingInference() >= PURE.weak
: sc.func.setImpure());
}

/*********************************************
* Calling function f.
* Check the safety, i.e. if we're in a @safe function
Expand Down
19 changes: 16 additions & 3 deletions test/fail_compilation/testInference.d
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ void test12422() pure
/*
TEST_OUTPUT:
---
fail_compilation/testInference.d(196): Error: `pure` function `testInference.test13729a` cannot access mutable static data `g13729`
fail_compilation/testInference.d(198): Error: `pure` function `testInference.test13729a` cannot call impure function `testInference.test13729a.foo`
fail_compilation/testInference.d(206): Error: `pure` function `testInference.test13729b` cannot call impure function `testInference.test13729b.foo!().foo`
---
*/
Expand All @@ -193,9 +193,9 @@ void test13729a() pure
{
static void foo() // typed as impure
{
g13729++; // disallowed
g13729++;
}
foo();
foo(); // cannot call impure function
}
void test13729b() pure
{
Expand Down Expand Up @@ -224,3 +224,16 @@ void test17086_call ()
bool f;
test17086(f);
}

/*
TEST_OUTPUT:
---
fail_compilation/testInference.d(238): Error: `pure` function `testInference.test20047_pure_function` cannot call impure function `testInference.test20047_pure_function.bug`
---
*/
void test20047_impure_function() {}
void test20047_pure_function() pure
{
static void bug() { return test20047_impure_function(); }
bug();
}
23 changes: 23 additions & 0 deletions test/runnable/mars1.d
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1937,6 +1937,28 @@ void testNegConst()

////////////////////////////////////////////////////////////////////////

// https://issues.dlang.org/show_bug.cgi?id=20050

int test20050_g = 0;
void test20050_impure_function_1() { ++test20050_g; }
void function() test20050_get_impure_function() pure
{
static void impure_function_2()
{
++test20050_g;
test20050_impure_function_1();
}
return &impure_function_2;
}
void test20050()
{
auto f = test20050_get_impure_function();
f();
assert(test20050_g == 2);
}

////////////////////////////////////////////////////////////////////////

int main()
{
testgoto();
Expand Down Expand Up @@ -2006,6 +2028,7 @@ int main()
test18794();
testfastpar();
testNegConst();
test20050();

printf("Success\n");
return 0;
Expand Down

0 comments on commit 22113af

Please sign in to comment.