Bug 20674 – [DIP1000] inference of `scope` is easily confused

Status
NEW
Severity
enhancement
Priority
P4
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2020-03-16T02:45:59Z
Last change time
2024-12-13T19:07:43Z
Keywords
pull, rejects-valid, safe
Assigned to
No Owner
Creator
ag0aep6g
See also
https://issues.dlang.org/show_bug.cgi?id=21209
Moved to GitHub: dmd#19679 →

Comments

Comment #0 by ag0aep6g — 2020-03-16T02:45:59Z
This compiles as expected: ---- int* f()(int* p) { static int g; g = 0; return new int; } int* g() @safe { int x; return f(&x); } ---- This doesn't, but should: ---- int* f()(int* p) { static int g; g = 0; auto p2 = p; /* the only difference */ return new int; } int* g() @safe { int x; return f(&x); /* Error: reference to local variable `x` assigned to non-scope parameter `p` calling test.f!().f */ } ----
Comment #1 by bugzilla — 2020-03-16T04:00:22Z
To do what this example asks for, data flow analysis would be required. This can be done, but is a large increase in implementation complexity and time to compile. It is not a bug that DFA is not currently done. Marked as an enhancement.
Comment #2 by snarwin+bugzilla — 2021-05-09T20:49:15Z
Note that this can be worked around by explicitly declaring the local variable in the second example `scope`: --- int* f()(int* p) { static int g; g = 0; scope p2 = p; /* the only difference */ return new int; } int* g() @safe { int x; return f(&x); /* ok */ } ---
Comment #3 by dkorpel — 2021-08-10T10:36:57Z
(In reply to Walter Bright from comment #1) > To do what this example asks for, data flow analysis would be required. This > can be done, but is a large increase in implementation complexity and time > to compile. This puzzles me. The examples don't contain a single branch or loop. Surely you can infer scope here without creating a control-flow graph, right? Also, while I don't know exactly how the current inference works (it's not documented anywhere), it looks like it has a pretty complex loop already: https://github.com/dlang/dmd/blob/8884ad0d232d300b424c025cf65ecd2623ec8df5/src/dmd/escape.d#L2176
Comment #4 by snarwin+bugzilla — 2021-08-10T13:55:13Z
(In reply to Dennis from comment #3) > (In reply to Walter Bright from comment #1) > > To do what this example asks for, data flow analysis would be required. This > > can be done, but is a large increase in implementation complexity and time > > to compile. > > This puzzles me. The examples don't contain a single branch or loop. Surely > you can infer scope here without creating a control-flow graph, right? Currently, in order to infer scope for `p`, you only need to check each usage of `p` for compliance with scope. To make the example (and others like it) work, you would need to recursively check each usage of any variable that `p` might be assigned to (like `p2`), and each usage of any variable that any of *those* variables might be assigned to, and so on, until you cannot find any new variables to check. What I've just described is a graph search problem. Whether the compiler actually builds an explicit control-flow graph or not, it amounts to the same thing.
Comment #5 by bugzilla — 2022-09-01T02:07:08Z
(In reply to Dennis from comment #3) > Surely you can infer scope here without creating a control-flow graph, right? The trouble is, once you start doing a lame DFA, then people want a real DFA. Doing DFA in the front end will significantly slow it down.
Comment #6 by timon.gehr — 2022-11-03T17:43:54Z
AFAIU DIP1000 just copies the lifetime attribute from the initializer to the variable, so I don't think you need anything more fancy than computing connected components.
Comment #7 by dlang-bot — 2023-01-27T15:15:46Z
@dkorpel updated dlang/dmd pull request #14492 "Fix 20674, 23208, 23300 - improve `scope` inference" fixing this issue: - Fix 20674, 23208, 23300, 23294 - improve `scope` inference https://github.com/dlang/dmd/pull/14492
Comment #8 by robert.schadek — 2024-12-13T19:07:43Z
THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/dmd/issues/19679 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB