Skip to content

Commit

Permalink
Add third error message parameter to setUnsafe() (#14198)
Browse files Browse the repository at this point in the history
  • Loading branch information
dkorpel committed Jun 15, 2022
1 parent 24d5ee1 commit 3e2f56d
Show file tree
Hide file tree
Showing 9 changed files with 48 additions and 35 deletions.
19 changes: 11 additions & 8 deletions src/dmd/escape.d
Original file line number Diff line number Diff line change
Expand Up @@ -328,12 +328,12 @@ bool checkParamArgumentEscape(Scope* sc, FuncDeclaration fdc, Parameter par, STC
else if (par)
{
result |= sc.setUnsafeDIP1000(gag, arg.loc,
desc ~ " `%s` assigned to non-scope parameter `%s`", v, par);
desc ~ " `%s` assigned to non-scope parameter `%s` calling `%s`", v, par, fdc);
}
else
{
result |= sc.setUnsafeDIP1000(gag, arg.loc,
desc ~ " `%s` assigned to non-scope parameter `this`", v);
desc ~ " `%s` assigned to non-scope parameter `this` calling `%s`", v, fdc);
}
}

Expand Down Expand Up @@ -2492,40 +2492,43 @@ private void addMaybe(VarDeclaration va, VarDeclaration v)
* fmt = printf-style format string
* arg0 = (optional) argument for first %s format specifier
* arg1 = (optional) argument for second %s format specifier
* arg2 = (optional) argument for third %s format specifier
* Returns: whether an actual safe error (not deprecation) occured
*/
private bool setUnsafePreview(Scope* sc, FeatureState fs, bool gag, Loc loc, const(char)* msg, RootObject arg0 = null, RootObject arg1 = null)
private bool setUnsafePreview(Scope* sc, FeatureState fs, bool gag, Loc loc, const(char)* msg,
RootObject arg0 = null, RootObject arg1 = null, RootObject arg2 = null)
{
if (fs == FeatureState.disabled)
{
return false;
}
else if (fs == FeatureState.enabled)
{
return sc.setUnsafe(gag, loc, msg, arg0, arg1);
return sc.setUnsafe(gag, loc, msg, arg0, arg1, arg2);
}
else
{
if (sc.func.isSafeBypassingInference())
{
if (!gag)
previewErrorFunc(sc.isDeprecated(), fs)(
loc, msg, arg0 ? arg0.toChars() : "", arg1 ? arg1.toChars() : ""
loc, msg, arg0 ? arg0.toChars() : "", arg1 ? arg1.toChars() : "", arg2 ? arg2.toChars() : ""
);
}
else if (!sc.func.safetyViolation)
{
import dmd.func : AttributeViolation;
sc.func.safetyViolation = new AttributeViolation(loc, msg, arg0, arg1);
sc.func.safetyViolation = new AttributeViolation(loc, msg, arg0, arg1, arg2);
}
return false;
}
}

// `setUnsafePreview` partially evaluated for dip1000
private bool setUnsafeDIP1000(Scope* sc, bool gag, Loc loc, const(char)* msg, RootObject arg0 = null, RootObject arg1 = null)
private bool setUnsafeDIP1000(Scope* sc, bool gag, Loc loc, const(char)* msg,
RootObject arg0 = null, RootObject arg1 = null, RootObject arg2 = null)
{
return setUnsafePreview(sc, global.params.useDIP1000, gag, loc, msg, arg0, arg1);
return setUnsafePreview(sc, global.params.useDIP1000, gag, loc, msg, arg0, arg1, arg2);
}

/***************************************
Expand Down
2 changes: 1 addition & 1 deletion src/dmd/expression.d
Original file line number Diff line number Diff line change
Expand Up @@ -1438,7 +1438,7 @@ extern (C++) abstract class Expression : ASTNode
else if (!sc.func.safetyViolation)
{
import dmd.func : AttributeViolation;
sc.func.safetyViolation = new AttributeViolation(this.loc, null, f, null);
sc.func.safetyViolation = new AttributeViolation(this.loc, null, f, null, null);
}
}
return false;
Expand Down
9 changes: 6 additions & 3 deletions src/dmd/frontend.h
Original file line number Diff line number Diff line change
Expand Up @@ -2740,18 +2740,21 @@ struct AttributeViolation final
const char* fmtStr;
RootObject* arg0;
RootObject* arg1;
RootObject* arg2;
AttributeViolation() :
loc(Loc(nullptr, 0u, 0u)),
fmtStr(nullptr),
arg0(nullptr),
arg1(nullptr)
arg1(nullptr),
arg2(nullptr)
{
}
AttributeViolation(Loc loc, const char* fmtStr = nullptr, RootObject* arg0 = nullptr, RootObject* arg1 = nullptr) :
AttributeViolation(Loc loc, const char* fmtStr = nullptr, RootObject* arg0 = nullptr, RootObject* arg1 = nullptr, RootObject* arg2 = nullptr) :
loc(loc),
fmtStr(fmtStr),
arg0(arg0),
arg1(arg1)
arg1(arg1),
arg2(arg2)
{}
};

Expand Down
21 changes: 14 additions & 7 deletions src/dmd/func.d
Original file line number Diff line number Diff line change
Expand Up @@ -1476,25 +1476,27 @@ extern (C++) class FuncDeclaration : Declaration
* fmt = printf-style format string
* arg0 = (optional) argument for first %s format specifier
* arg1 = (optional) argument for second %s format specifier
* arg2 = (optional) argument for third %s format specifier
* Returns: whether there's a safe error
*/
extern (D) final bool setUnsafe(
bool gag = false, Loc loc = Loc.init, const(char)* fmt = null, RootObject arg0 = null, RootObject arg1 = null)
bool gag = false, Loc loc = Loc.init, const(char)* fmt = null,
RootObject arg0 = null, RootObject arg1 = null, RootObject arg2 = null)
{
if (flags & FUNCFLAG.safetyInprocess)
{
flags &= ~FUNCFLAG.safetyInprocess;
type.toTypeFunction().trust = TRUST.system;
if (fmt || arg0)
safetyViolation = new AttributeViolation(loc, fmt, arg0, arg1);
safetyViolation = new AttributeViolation(loc, fmt, arg0, arg1, arg2);

if (fes)
fes.func.setUnsafe();
}
else if (isSafe())
{
if (!gag && fmt)
.error(loc, fmt, arg0 ? arg0.toChars() : "", arg1 ? arg1.toChars() : "");
.error(loc, fmt, arg0 ? arg0.toChars() : "", arg1 ? arg1.toChars() : "", arg2 ? arg2.toChars() : "");

return true;
}
Expand Down Expand Up @@ -4370,10 +4372,12 @@ extern (C++) final class NewDeclaration : FuncDeclaration
* fmt = printf-style format string
* arg0 = (optional) argument for first %s format specifier
* arg1 = (optional) argument for second %s format specifier
* arg2 = (optional) argument for third %s format specifier
* Returns: whether there's a safe error
*/
bool setUnsafe(Scope* sc,
bool gag = false, Loc loc = Loc.init, const(char)* fmt = null, RootObject arg0 = null, RootObject arg1 = null)
bool gag = false, Loc loc = Loc.init, const(char)* fmt = null,
RootObject arg0 = null, RootObject arg1 = null, RootObject arg2 = null)
{
// TODO:
// For @system variables, unsafe initializers at global scope should mark
Expand All @@ -4394,13 +4398,13 @@ bool setUnsafe(Scope* sc,
{
// Message wil be gagged, but still call error() to update global.errors and for
// -verrors=spec
.error(loc, fmt, arg0 ? arg0.toChars() : "", arg1 ? arg1.toChars() : "");
.error(loc, fmt, arg0 ? arg0.toChars() : "", arg1 ? arg1.toChars() : "", arg2 ? arg2.toChars() : "");
return true;
}
return false;
}

return sc.func.setUnsafe(gag, loc, fmt, arg0, arg1);
return sc.func.setUnsafe(gag, loc, fmt, arg0, arg1, arg2);
}

/// Stores a reason why a function failed to infer a function attribute like `@safe` or `pure`
Expand All @@ -4421,6 +4425,8 @@ struct AttributeViolation
RootObject arg0 = null;
/// ditto
RootObject arg1 = null;
/// ditto
RootObject arg2 = null;
}

/// Print the reason why `fd` was inferred `@system` as a supplemental error
Expand All @@ -4438,7 +4444,8 @@ void errorSupplementalInferredSafety(FuncDeclaration fd, int maxDepth, bool depr
errorFunc(s.loc, deprecation ?
"which would be `@system` because of:" :
"which was inferred `@system` because of:");
errorFunc(s.loc, s.fmtStr, s.arg0 ? s.arg0.toChars() : "", s.arg1 ? s.arg1.toChars() : "");
errorFunc(s.loc, s.fmtStr,
s.arg0 ? s.arg0.toChars() : "", s.arg1 ? s.arg1.toChars() : "", s.arg2 ? s.arg2.toChars() : "");
}
else if (FuncDeclaration fd2 = cast(FuncDeclaration) s.arg0)
{
Expand Down
6 changes: 3 additions & 3 deletions test/fail_compilation/retscope.d
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ struct HTTP
/*
TEST_OUTPUT:
---
fail_compilation/retscope.d(96): Error: reference to local variable `sa` assigned to non-scope parameter `a`
fail_compilation/retscope.d(96): Error: reference to local variable `sa` assigned to non-scope parameter `a` calling `bar8`
---
*/
// https://issues.dlang.org/show_bug.cgi?id=8838
Expand Down Expand Up @@ -403,7 +403,7 @@ class Foo13
/*
TEST_OUTPUT:
---
fail_compilation/retscope.d(1205): Error: scope variable `f14` assigned to non-scope parameter `this`
fail_compilation/retscope.d(1205): Error: scope variable `f14` assigned to non-scope parameter `this` calling `foo`
---
*/

Expand Down Expand Up @@ -454,7 +454,7 @@ fail_compilation/retscope.d(1311): Error: scope variable `u2` assigned to `ek` w
/*
TEST_OUTPUT:
---
fail_compilation/retscope.d(1405): Error: reference to local variable `buf` assigned to non-scope parameter `__anonymous_param`
fail_compilation/retscope.d(1405): Error: reference to local variable `buf` assigned to non-scope parameter `__anonymous_param` calling `myprintf`
---
*/

Expand Down
4 changes: 2 additions & 2 deletions test/fail_compilation/retscope2.d
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ fail_compilation/retscope2.d(504): Error: scope variable `c` may not be returned
/*
TEST_OUTPUT:
---
fail_compilation/retscope2.d(604): Error: scope variable `_param_0` assigned to non-scope parameter `__anonymous_param`
fail_compilation/retscope2.d(604): Error: scope variable `_param_1` assigned to non-scope parameter `__anonymous_param`
fail_compilation/retscope2.d(604): Error: scope variable `_param_0` assigned to non-scope parameter `__anonymous_param` calling `foo600`
fail_compilation/retscope2.d(604): Error: scope variable `_param_1` assigned to non-scope parameter `__anonymous_param` calling `foo600`
fail_compilation/retscope2.d(614): Error: template instance `retscope2.test600!(int*, int*)` error instantiating
---
*/
Expand Down
12 changes: 6 additions & 6 deletions test/fail_compilation/retscope6.d
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,9 @@ void foo() @safe
/* TEST_OUTPUT:
---
fail_compilation/retscope6.d(8016): Error: address of variable `i` assigned to `p` with longer lifetime
fail_compilation/retscope6.d(8031): Error: reference to local variable `i` assigned to non-scope parameter `p`
fail_compilation/retscope6.d(8031): Error: reference to local variable `j` assigned to non-scope parameter `q`
fail_compilation/retscope6.d(8048): Error: reference to local variable `j` assigned to non-scope parameter `q`
fail_compilation/retscope6.d(8031): Error: reference to local variable `i` assigned to non-scope parameter `p` calling `betty`
fail_compilation/retscope6.d(8031): Error: reference to local variable `j` assigned to non-scope parameter `q` calling `betty`
fail_compilation/retscope6.d(8048): Error: reference to local variable `j` assigned to non-scope parameter `q` calling `archie`
---
*/

Expand Down Expand Up @@ -172,7 +172,7 @@ T9 testfred()

/* TEST_OUTPUT:
---
fail_compilation/retscope6.d(10003): Error: scope variable `values` assigned to non-scope parameter `values`
fail_compilation/retscope6.d(10003): Error: scope variable `values` assigned to non-scope parameter `values` calling `escape`
---
*/

Expand Down Expand Up @@ -234,7 +234,7 @@ const(int)* f_c_20150() @safe nothrow

/* TEST_OUTPUT:
---
fail_compilation/retscope6.d(13010): Error: reference to local variable `str` assigned to non-scope parameter `x`
fail_compilation/retscope6.d(13010): Error: reference to local variable `str` assigned to non-scope parameter `x` calling `f_throw`
---
*/

Expand All @@ -254,7 +254,7 @@ void escape_throw_20150() @safe

/* TEST_OUTPUT:
---
fail_compilation/retscope6.d(14019): Error: scope variable `scopePtr` assigned to non-scope parameter `x`
fail_compilation/retscope6.d(14019): Error: scope variable `scopePtr` assigned to non-scope parameter `x` calling `noInfer23021`
fail_compilation/retscope6.d(14022): Error: scope variable `scopePtr` may not be returned
---
*/
Expand Down
2 changes: 1 addition & 1 deletion test/fail_compilation/test17423.d
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* REQUIRED_ARGS: -preview=dip1000
TEST_OUTPUT:
---
fail_compilation/test17423.d(26): Error: reference to local `this` assigned to non-scope parameter `dlg`
fail_compilation/test17423.d(26): Error: reference to local `this` assigned to non-scope parameter `dlg` calling `opApply`
---
*/

Expand Down
8 changes: 4 additions & 4 deletions test/fail_compilation/test20245.d
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@
REQUIRED_ARGS: -preview=dip1000
TEST_OUTPUT:
---
fail_compilation/test20245.d(20): Error: reference to local variable `x` assigned to non-scope parameter `ptr`
fail_compilation/test20245.d(20): Error: reference to local variable `x` assigned to non-scope parameter `ptr` calling `escape`
fail_compilation/test20245.d(21): Error: copying `&x` into allocated memory escapes a reference to parameter `x`
fail_compilation/test20245.d(22): Error: scope variable `a` may not be returned
fail_compilation/test20245.d(26): Error: cannot take address of `scope` variable `x` since `scope` applies to first indirection only
fail_compilation/test20245.d(32): Error: reference to local variable `x` assigned to non-scope parameter `ptr`
fail_compilation/test20245.d(32): Error: reference to local variable `x` assigned to non-scope parameter `ptr` calling `escape`
fail_compilation/test20245.d(33): Error: copying `&x` into allocated memory escapes a reference to parameter `x`
fail_compilation/test20245.d(49): Error: reference to local variable `price` assigned to non-scope `this.minPrice`
fail_compilation/test20245.d(68): Error: reference to local variable `this` assigned to non-scope parameter `msg`
fail_compilation/test20245.d(88): Error: reference to local variable `this` assigned to non-scope parameter `content`
fail_compilation/test20245.d(68): Error: reference to local variable `this` assigned to non-scope parameter `msg` calling `this`
fail_compilation/test20245.d(88): Error: reference to local variable `this` assigned to non-scope parameter `content` calling `listUp`
---
*/

Expand Down

0 comments on commit 3e2f56d

Please sign in to comment.