Skip to content

Print deprecation messages except if -revert=dip1000 is used #12578

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
atilaneves merged 9 commits into from
Apr 21, 2022
Merged

Print deprecation messages except if -revert=dip1000 is used #12578

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ea490dc
Print deprecation messages except if -revert=dip1000
atilaneves May 24, 2021
5a10999
Fix breakage due to state changing of FuncDeclaration.setUnsafe
atilaneves Apr 15, 2022
ac6182d
Refactoring
atilaneves Apr 15, 2022
91524bc
Fix frontend.h
atilaneves Apr 15, 2022
326e02b
isUnsafe -> setUnsafeDIP1000
atilaneves Apr 15, 2022
8a5d515
checkUnsafe -> isSafeBypassingInference
atilaneves Apr 15, 2022
526eae7
Delete FuncDeclaration.checkUnsafe
atilaneves Apr 15, 2022
dccb5f6
Update src/dmd/frontend.h
atilaneves Apr 15, 2022
8020446
Add changelog entry
atilaneves Apr 15, 2022
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
6 changes: 6 additions & 0 deletions changelog/dip1000_deprecation_warnings.dd
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Print warning messages unless -revert=dip1000 is used
Copy link
Member

@ibuclaw ibuclaw Apr 21, 2022

Choose a reason for hiding this comment

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

I guess the little detail of adding this option was overlooked? :-)

Copy link
Contributor Author

@atilaneves atilaneves Apr 21, 2022

Choose a reason for hiding this comment

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

Yes, unfortunately: #14019


As an incentive to transition to -preview=dip1000 by default, now the
compiler prints what would have been error messages if the flag had
been on as deprecation messages. This new behaviour can be disabled
Copy link
Contributor

@dkorpel dkorpel Apr 15, 2022

Choose a reason for hiding this comment

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

Only in explicit @safe functions, function with inference may still silently be @safe while having scope violations.

with -revert=dip1000.
61 changes: 41 additions & 20 deletions src/dmd/escape.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,24 +316,27 @@ bool checkParamArgumentEscape(Scope* sc, FuncDeclaration fdc, Parameter par, Exp
*/
void unsafeAssign(VarDeclaration v, const char* desc)
{
if (global.params.useDIP1000 == FeatureState.enabled && sc.func.setUnsafe())
if (setUnsafeDIP1000(sc.func))
{
if (!gag)
{
if (assertmsg)
{
error(arg.loc, "%s `%s` assigned to non-scope parameter calling `assert()`",
previewErrorFunc(sc.isDeprecated(), global.params.useDIP1000)
(arg.loc, "%s `%s` assigned to non-scope parameter calling `assert()`",
desc, v.toChars());
}
else
{
error(arg.loc, "%s `%s` assigned to non-scope parameter `%s` calling %s",
previewErrorFunc(sc.isDeprecated(), global.params.useDIP1000)
(arg.loc, "%s `%s` assigned to non-scope parameter `%s` calling %s",
desc, v.toChars(),
par ? par.toChars() : "this",
fdc ? fdc.toPrettyChars() : "indirectly");
}
}
result = true;
if (global.params.useDIP1000 == FeatureState.enabled)
result = true;
}
}

Expand Down Expand Up @@ -774,20 +777,26 @@ bool checkAssignEscape(Scope* sc, Expression e, bool gag, bool byRef)
if (v.isDataseg())
continue;

if (global.params.useDIP1000 == FeatureState.enabled)
if (global.params.useDIP1000 != FeatureState.disabled)
{
if (va && va.isScope() && !v.isReference())
{
if (!(va.storage_class & STC.return_))
{
va.doNotInferReturn = true;
}
else if (fd.setUnsafe())
else if (setUnsafeDIP1000(fd))
{
if (!gag)
error(ae.loc, "address of local variable `%s` assigned to return scope `%s`", v.toChars(), va.toChars());
result = true;
continue;
previewErrorFunc(sc.isDeprecated(), global.params.useDIP1000)
(ae.loc, "address of local variable `%s` assigned to return scope `%s`", v.toChars(), va.toChars());


if (global.params.useDIP1000 == FeatureState.enabled)
{
result = true;
continue;
}
}
}
}
Expand Down Expand Up @@ -976,12 +985,11 @@ bool checkThrowEscape(Scope* sc, Expression e, bool gag)
if (v.isScope() && !v.iscatchvar) // special case: allow catch var to be rethrown
// despite being `scope`
{
if (!gag)
previewErrorFunc(sc.isDeprecated(), global.params.useDIP1000)
(e.loc, "scope variable `%s` may not be thrown", v.toChars());
if (global.params.useDIP1000 == FeatureState.enabled) // https://issues.dlang.org/show_bug.cgi?id=17029
{
if (!gag)
error(e.loc, "scope variable `%s` may not be thrown", v.toChars());
result = true;
}
continue;
}
else
Expand Down Expand Up @@ -1042,13 +1050,16 @@ bool checkNewEscape(Scope* sc, Expression e, bool gag)
*/
!(p.parent == sc.func))
{
if (global.params.useDIP1000 == FeatureState.enabled // https://issues.dlang.org/show_bug.cgi?id=17029
&& sc.func.setUnsafe()) // https://issues.dlang.org/show_bug.cgi?id=20868
if (setUnsafeDIP1000(sc.func)) // https://issues.dlang.org/show_bug.cgi?id=20868
{
// Only look for errors if in module listed on command line
if (!gag)
error(e.loc, "scope variable `%s` may not be copied into allocated memory", v.toChars());
result = true;
previewErrorFunc(sc.isDeprecated(), global.params.useDIP1000)
(e.loc, "scope variable `%s` may not be copied into allocated memory", v.toChars());
if (global.params.useDIP1000 == FeatureState.enabled)
result = true;
}

continue;
}
}
Expand Down Expand Up @@ -1258,11 +1269,13 @@ private bool checkReturnEscapeImpl(Scope* sc, Expression e, bool refs, bool gag)
)
{
// https://issues.dlang.org/show_bug.cgi?id=17029
if (global.params.useDIP1000 == FeatureState.enabled && sc.func.setUnsafe())
if (setUnsafeDIP1000(sc.func))
{
if (!gag)
error(e.loc, "scope variable `%s` may not be returned", v.toChars());
result = true;
previewErrorFunc(sc.isDeprecated(), global.params.useDIP1000)
(e.loc, "scope variable `%s` may not be returned", v.toChars());
if (global.params.useDIP1000 == FeatureState.enabled)
result = true;
}
continue;
}
Expand Down Expand Up @@ -2341,3 +2354,11 @@ private void addMaybe(VarDeclaration va, VarDeclaration v)
va.maybes = new VarDeclarations();
va.maybes.push(v);
}


private bool setUnsafeDIP1000(FuncDeclaration f)
{
return global.params.useDIP1000 == FeatureState.enabled
? f.setUnsafe()
: f.isSafeBypassingInference();
}
30 changes: 15 additions & 15 deletions test/fail_compilation/fail_scope.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@
REQUIRED_ARGS:
TEST_OUTPUT:
---
fail_compilation/fail_scope.d(29): Deprecation: scope variable `da` may not be returned
fail_compilation/fail_scope.d(31): Deprecation: scope variable `o` may not be returned
fail_compilation/fail_scope.d(32): Deprecation: scope variable `dg` may not be returned
fail_compilation/fail_scope.d(34): Deprecation: scope variable `da` may not be returned
fail_compilation/fail_scope.d(36): Deprecation: scope variable `o` may not be returned
fail_compilation/fail_scope.d(37): Deprecation: scope variable `dg` may not be returned
fail_compilation/fail_scope.d(39): Deprecation: scope variable `p` may not be returned
fail_compilation/fail_scope.d(44): Error: returning `cast(char[])string` escapes a reference to local variable `string`
fail_compilation/fail_scope.d(62): Error: returning `s.bar()` escapes a reference to local variable `s`
fail_compilation/fail_scope.d(73): Error: `fail_scope.foo8` called with argument types `(int)` matches both:
Expand All @@ -15,26 +22,19 @@ fail_compilation/fail_scope.d(107): Deprecation: escaping reference to outer loc
fail_compilation/fail_scope.d(126): Error: returning `s.bar()` escapes a reference to local variable `s`
fail_compilation/fail_scope.d(136): Error: returning `foo16226(i)` escapes a reference to local variable `i`
---
//fail_compilation/fail_scope.d(30): Error: scope variable `da` may not be returned
//fail_compilation/fail_scope.d(32): Error: scope variable `o` may not be returned
//fail_compilation/fail_scope.d(33): Error: scope variable `dg` may not be returned
//fail_compilation/fail_scope.d(35): Error: scope variable `da` may not be returned
//fail_compilation/fail_scope.d(37): Error: scope variable `o` may not be returned
//fail_compilation/fail_scope.d(38): Error: scope variable `dg` may not be returned
//fail_compilation/fail_scope.d(40): Error: scope variable `p` may not be returned
*/

alias int delegate() dg_t;

int[] checkEscapeScope1(scope int[] da) { return da; }
int[3] checkEscapeScope2(scope int[3] sa) { return sa; }
Object checkEscapeScope3(scope Object o) { return o; }
dg_t checkEscapeScope4(scope dg_t dg) { return dg; }
int[] checkEscapeScope1(scope int[] da) @safe { return da; }
int[3] checkEscapeScope2(scope int[3] sa) @safe { return sa; }
Object checkEscapeScope3(scope Object o) @safe { return o; }
dg_t checkEscapeScope4(scope dg_t dg) @safe { return dg; }

int[] checkEscapeScope1() { scope int[] da = []; return da; }
int[3] checkEscapeScope2() { scope int[3] sa = [1,2,3]; return sa; }
Object checkEscapeScope3() { scope Object o = new Object; return o; } // same with fail7294.d
dg_t checkEscapeScope4() { scope dg_t dg = () => 1; return dg; }
int[] checkEscapeScope1() @safe { scope int[] da = []; return da; }
int[3] checkEscapeScope2() @safe { scope int[3] sa = [1,2,3]; return sa; }
Object checkEscapeScope3() @safe { scope Object o = new Object; return o; } // same with fail7294.d
dg_t checkEscapeScope4() @safe { scope dg_t dg = () => 1; return dg; }

int* test(scope int* p) @safe { return p; }

Expand Down