Skip to content

fix Issue 17784 - [scope][DIP1000] Confusing error message for escapi… #8054

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
WalterBright merged 1 commit into from
Mar 21, 2018
Merged

fix Issue 17784 - [scope][DIP1000] Confusing error message for escapi… #8054

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
156 changes: 144 additions & 12 deletions src/dmd/escape.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -585,9 +585,143 @@ bool checkThrowEscape(Scope* sc, Expression e, bool gag)
bool checkNewEscape(Scope* sc, Expression e, bool gag)
{
//printf("[%s] checkNewEscape, e = %s\n", e.loc.toChars(), e.toChars());
return checkReturnEscapeImpl(sc, e, false, gag, false);
enum log = false;
if (log) printf("[%s] checkNewEscape, e: `%s`\n", e.loc.toChars(), e.toChars());
EscapeByResults er;

escapeByValue(e, &er);

if (!er.byref.dim && !er.byvalue.dim && !er.byexp.dim)
return false;

bool result = false;
foreach (VarDeclaration v; er.byvalue)
{
if (log) printf("byvalue `%s`\n", v.toChars());
if (v.isDataseg())
continue;

Dsymbol p = v.toParent2();

if (v.isScope())
{
if (sc._module && sc._module.isRoot() &&
/* This case comes up when the ReturnStatement of a __foreachbody is
* checked for escapes by the caller of __foreachbody. Skip it.
*
* struct S { static int opApply(int delegate(S*) dg); }
* S* foo() {
* foreach (S* s; S) // create __foreachbody for body of foreach
* return s; // s is inferred as 'scope' but incorrectly tested in foo()
* return null; }
*/
!(p.parent == sc.func))
{
// Only look for errors if in module listed on command line
if (global.params.vsafe) // https://issues.dlang.org/show_bug.cgi?id=17029
{
if (!gag)
error(e.loc, "scope variable `%s` may not be copied into allocated memory", v.toChars());
Copy link
Contributor

@JinShil JinShil Mar 19, 2018

Choose a reason for hiding this comment

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

I suggest using the term "GC-allocated memory" to be more precise. The problem isn't about memory (per se) or its allocation but about the lifetimes of objects under the jurisdiction of the GC. I suggest something like:

scope variable %s may not be copied into GC-allocated memory because doing so would cause it to escape

Copy link
Member Author

@WalterBright WalterBright Mar 19, 2018

Choose a reason for hiding this comment

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

I didn't use that term because the message could apply to any allocated memory.

result = true;
}
continue;
}
}
else if (v.storage_class & STC.variadic && p == sc.func)
{
Type tb = v.type.toBasetype();
if (tb.ty == Tarray || tb.ty == Tsarray)
{
if (!gag)
error(e.loc, "copying `%s` into allocated memory escapes a reference to variadic parameter `%s`", e.toChars(), v.toChars());
Copy link
Contributor

@JinShil JinShil Mar 19, 2018

Choose a reason for hiding this comment

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

My suggestion:

copying %s into GC-allocated memory escapes a reference to variadic parameter %s

result = false;
}
}
else
{
//printf("no infer for %s in %s, %d\n", v.toChars(), sc.func.ident.toChars(), __LINE__);
v.doNotInferScope = true;
}
}

foreach (VarDeclaration v; er.byref)
{
if (log) printf("byref `%s`\n", v.toChars());

void escapingRef(VarDeclaration v)
{
if (!gag)
{
const(char)* kind = (v.storage_class & STC.parameter) ? "parameter" : "local";
error(e.loc, "copying `%s` into allocated memory escapes a reference to %s variable `%s`",
Copy link
Contributor

@JinShil JinShil Mar 19, 2018

Choose a reason for hiding this comment

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

I suggest:

copying %s into GC-allocated memory escapes a reference to %s variable %s

e.toChars(), kind, v.toChars());
}
result = true;
}

if (v.isDataseg())
continue;

Dsymbol p = v.toParent2();

if ((v.storage_class & (STC.ref_ | STC.out_)) == 0)
{
if (p == sc.func)
{
escapingRef(v);
continue;
}
}

/* Check for returning a ref variable by 'ref', but should be 'return ref'
* Infer the addition of 'return', or set result to be the offending expression.
*/
if (v.storage_class & (STC.ref_ | STC.out_))
{
if (global.params.useDIP25 &&
sc._module && sc._module.isRoot())
{
// Only look for errors if in module listed on command line

if (p == sc.func)
{
//printf("escaping reference to local ref variable %s\n", v.toChars());
//printf("storage class = x%llx\n", v.storage_class);
escapingRef(v);
continue;
}
// Don't need to be concerned if v's parent does not return a ref
FuncDeclaration fd = p.isFuncDeclaration();
if (fd && fd.type && fd.type.ty == Tfunction)
{
TypeFunction tf = cast(TypeFunction)fd.type;
if (tf.isref)
{
if (!gag)
error(e.loc, "storing reference to outer local variable `%s` into allocated memory causes it to escape",
v.toChars());
result = true;
continue;
}
}

}
}
}

foreach (Expression ee; er.byexp)
{
if (log) printf("byexp %s\n", ee.toChars());
if (!gag)
error(ee.loc, "storing reference to stack allocated value returned by `%s` into allocated memory causes it to escape",
ee.toChars());
result = true;
}

return result;
}


/************************************
* Detect cases where pointers to the stack can 'escape' the
* lifetime of the stack frame by returning 'e' by value.
Expand All @@ -602,7 +736,7 @@ bool checkNewEscape(Scope* sc, Expression e, bool gag)
bool checkReturnEscape(Scope* sc, Expression e, bool gag)
{
//printf("[%s] checkReturnEscape, e = %s\n", e.loc.toChars(), e.toChars());
return checkReturnEscapeImpl(sc, e, false, gag, true);
return checkReturnEscapeImpl(sc, e, false, gag);
}

/************************************
Expand All @@ -625,20 +759,19 @@ bool checkReturnEscapeRef(Scope* sc, Expression e, bool gag)
printf("parent2 function %s\n", sc.func.toParent2().toChars());
}

return checkReturnEscapeImpl(sc, e, true, gag, true);
return checkReturnEscapeImpl(sc, e, true, gag);
}

/***************************************
* Implementation of checking for escapes in `return` and `new`.
* Implementation of checking for escapes in `return`.
* Params:
* sc = used to determine current function and module
* e = expression to check
* gag = do not print error messages
* isReturn = it's a `return`, otherwise `new`
* Returns:
* true if references to the stack can escape
*/
private bool checkReturnEscapeImpl(Scope* sc, Expression e, bool refs, bool gag, bool isReturn)
private bool checkReturnEscapeImpl(Scope* sc, Expression e, bool refs, bool gag)
{
enum log = false;
if (log) printf("[%s] checkReturnEscapeImpl, refs: %d e: `%s`\n", e.loc.toChars(), refs, e.toChars());
Expand All @@ -665,16 +798,15 @@ private bool checkReturnEscapeImpl(Scope* sc, Expression e, bool refs, bool gag,
!(v.storage_class & STC.return_) &&
v.isParameter() &&
sc.func.flags & FUNCFLAG.returnInprocess &&
p == sc.func &&
isReturn)
p == sc.func)
{
inferReturn(sc.func, v); // infer addition of 'return'
continue;
}

if (v.isScope())
{
if (v.storage_class & STC.return_ && isReturn)
if (v.storage_class & STC.return_)
continue;

if (sc._module && sc._module.isRoot() &&
Expand Down Expand Up @@ -747,7 +879,7 @@ private bool checkReturnEscapeImpl(Scope* sc, Expression e, bool refs, bool gag,
continue;
}
FuncDeclaration fd = p.isFuncDeclaration();
if (fd && sc.func.flags & FUNCFLAG.returnInprocess && isReturn)
if (fd && sc.func.flags & FUNCFLAG.returnInprocess)
{
/* Code like:
* int x;
Expand All @@ -766,9 +898,9 @@ private bool checkReturnEscapeImpl(Scope* sc, Expression e, bool refs, bool gag,
* Infer the addition of 'return', or set result to be the offending expression.
*/
if ( (v.storage_class & (STC.ref_ | STC.out_)) &&
(!isReturn || !(v.storage_class & (STC.return_ | STC.foreach_))))
!(v.storage_class & (STC.return_ | STC.foreach_)))
{
if (sc.func.flags & FUNCFLAG.returnInprocess && p == sc.func && isReturn)
if (sc.func.flags & FUNCFLAG.returnInprocess && p == sc.func)
{
inferReturn(sc.func, v); // infer addition of 'return'
}
Expand Down
2 changes: 1 addition & 1 deletion test/fail_compilation/fail17842.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* TEST_OUTPUT:
---
fail_compilation/fail17842.d(14): Error: scope variable `p` assigned to non-scope `*q`
fail_compilation/fail17842.d(23): Error: scope variable `obj` may not be returned
fail_compilation/fail17842.d(23): Error: scope variable `obj` may not be copied into allocated memory
---
*/

Expand Down
2 changes: 1 addition & 1 deletion test/fail_compilation/retscope2.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ struct T17388
/*
TEST_OUTPUT:
---
fail_compilation/retscope2.d(1306): Error: returning `& i` escapes a reference to local variable `i`
fail_compilation/retscope2.d(1306): Error: copying `& i` into allocated memory escapes a reference to local variable `i`
---
*/

Expand Down
43 changes: 41 additions & 2 deletions test/fail_compilation/retscope3.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ PERMUTE_ARGS:
/*
TEST_OUTPUT:
---
fail_compilation/retscope3.d(2008): Error: returning `& i` escapes a reference to local variable `i`
fail_compilation/retscope3.d(2017): Error: returning `S2000(& i)` escapes a reference to local variable `i`
fail_compilation/retscope3.d(2008): Error: copying `& i` into allocated memory escapes a reference to local variable `i`
fail_compilation/retscope3.d(2017): Error: copying `S2000(& i)` into allocated memory escapes a reference to local variable `i`
---
*/

Expand Down Expand Up @@ -90,3 +90,42 @@ void test3000() @safe
}
}

/**********************************************/

/*
TEST_OUTPUT:
---
fail_compilation/retscope3.d(4003): Error: copying `u[]` into allocated memory escapes a reference to variadic parameter `u`
fail_compilation/retscope3.d(4016): Error: storing reference to outer local variable `i` into allocated memory causes it to escape
fail_compilation/retscope3.d(4025): Error: storing reference to stack allocated value returned by `makeSA()` into allocated memory causes it to escape
---
*/

#line 4000

void bar4000(int[1] u...) @safe
{
int[][] n = [u[]];
}

void bar4001() @safe
{
static int i;
int*[] n = [&i];
}

ref int bar4002(return ref int i) @safe
{
void nested()
{
int*[] n = [&i];
}
return i;
}

int[3] makeSA() @safe;

void bar4003() @safe
{
int[][] a = [makeSA()[]];
}
2 changes: 1 addition & 1 deletion test/fail_compilation/retscope6.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ PERMUTE_ARGS:
/*
TEST_OUTPUT:
---
fail_compilation/retscope6.d(6007): Error: returning `& i` escapes a reference to local variable `i`
fail_compilation/retscope6.d(6007): Error: copying `& i` into allocated memory escapes a reference to local variable `i`
---
*/

Expand Down
18 changes: 9 additions & 9 deletions test/fail_compilation/test18282.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
TEST_OUTPUT:
---
fail_compilation/test18282.d(25): Error: scope variable `aa` may not be returned
fail_compilation/test18282.d(34): Error: returning `& i` escapes a reference to local variable `i`
fail_compilation/test18282.d(35): Error: returning `& i` escapes a reference to local variable `i`
fail_compilation/test18282.d(34): Error: copying `& i` into allocated memory escapes a reference to local variable `i`
fail_compilation/test18282.d(35): Error: copying `& i` into allocated memory escapes a reference to local variable `i`
fail_compilation/test18282.d(36): Error: scope variable `staa` may not be returned
fail_compilation/test18282.d(44): Error: returning `S2000(& i)` escapes a reference to local variable `i`
fail_compilation/test18282.d(53): Error: returning `& i` escapes a reference to local variable `i`
fail_compilation/test18282.d(53): Error: returning `& c` escapes a reference to local variable `c`
fail_compilation/test18282.d(44): Error: copying `S2000(& i)` into allocated memory escapes a reference to local variable `i`
fail_compilation/test18282.d(53): Error: copying `& i` into allocated memory escapes a reference to local variable `i`
fail_compilation/test18282.d(53): Error: copying `& c` into allocated memory escapes a reference to local variable `c`
---
*/

Expand Down Expand Up @@ -57,10 +57,10 @@ void bar2()
/******************************
TEST_OUTPUT:
---
fail_compilation/test18282.d(1007): Error: returning `& foo` escapes a reference to local variable `foo`
fail_compilation/test18282.d(1008): Error: returning `& foo` escapes a reference to local variable `foo`
fail_compilation/test18282.d(1009): Error: returning `& foo` escapes a reference to local variable `foo`
fail_compilation/test18282.d(1016): Error: returning `&this` escapes a reference to parameter `this`, perhaps annotate with `return`
fail_compilation/test18282.d(1007): Error: copying `& foo` into allocated memory escapes a reference to local variable `foo`
fail_compilation/test18282.d(1008): Error: copying `& foo` into allocated memory escapes a reference to local variable `foo`
fail_compilation/test18282.d(1009): Error: copying `& foo` into allocated memory escapes a reference to local variable `foo`
fail_compilation/test18282.d(1016): Error: copying `&this` into allocated memory escapes a reference to parameter variable `this`
---
*/

Expand Down