fix Issue 16589 - Taking address of stack variables in @safe code is …#6172
fix Issue 16589 - Taking address of stack variables in @safe code is …#6172WalterBright merged 1 commit intodlang:masterfrom
Conversation
|
Should it be part of |
099677b to
d91a93e
Compare
|
Unfortunately, yes, as it is tripping in Phobos. |
…allowed in some cases
|
Isn't it unnecessarily limiting though to implement this separately from DIP1000? It is perfectly valid to take address of the field and pass it to some function (expecting pointer) for processing, I would be very surprised if there isn't plenty of such code in the wild. Thus if it gets merged alone, it would break some legit use cases without a tool to make it work again (== |
|
It's been an error for years to take the address of a local in It does break at least one Phobos function (the autotester quits at the first error), which is why I amended it to only check if If the user needs to BTW, this rule nicely resolves the ambiguity of what |
andralex
left a comment
There was a problem hiding this comment.
Few nits to improve the lot of readers and reduce LOC.
|
|
||
| bool checkAddressVar(VarDeclaration v) | ||
| { | ||
| if (v) |
There was a problem hiding this comment.
if (!v)
return true; // nuttin' to do
... rest of the code ...| } | ||
| if (sc.func && !sc.intypeof && !v.isDataseg()) | ||
| { | ||
| if (sc.func.setUnsafe()) |
There was a problem hiding this comment.
if + if = &&
if (sc.func && !sc.intypeof && !v.isDataseg() && sc.func.setUnsafe())
{
...
}| VarDeclaration v = ve.var.isVarDeclaration(); | ||
| if (v) | ||
| { | ||
| if (!checkAddressVar(v)) |
There was a problem hiding this comment.
if (v && !checkAddressVar(v))
return new ErrorExp();| VarDeclaration v = ve.var.isVarDeclaration(); | ||
| if (v && v.storage_class & STCref) | ||
| { | ||
| if (!checkAddressVar(v)) |
There was a problem hiding this comment.
if (v && v.storage_class & STCref) && !checkAddressVar(v))
return new ErrorExp();| VarDeclaration v = ve.var.isVarDeclaration(); | ||
| if (v) | ||
| { | ||
| if (!checkAddressVar(v)) |
There was a problem hiding this comment.
if (v && !checkAddressVar(v))
return new ErrorExp();|
Auto-merge toggled on |
…allowed in some cases