Bug 23491 – Nonsensical deprecation message when using delegate

Status
RESOLVED
Resolution
FIXED
Severity
normal
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2022-11-17T09:38:15Z
Last change time
2022-11-28T07:49:21Z
Keywords
pull
Assigned to
No Owner
Creator
Mathias LANG

Comments

Comment #0 by pro.mathias.lang — 2022-11-17T09:38:15Z
Code: ``` import core.internal.string : unsignedToTempString; void main () @safe { toString(null); } void toString (void delegate (char[]) @safe sink) @safe { char[20] buffer = void; sink(unsignedToTempString(42, buffer)); } ``` Message: foo.d(9): Deprecation: reference to local variable `buffer` assigned to non-scope parameter `__anonymous_param` calling `` The actual problem is that the return value of `unsignedToTempString`, which is a slice of `buffer`, is passed to the first argument of `sink` that is not scope. However, the message: 1) Doesn't name `sink` at all (The '``' is obviously a bug); 2) References `buffer` but not the return value of `unsignedToTempString`; 3) Doesn't handle anonymous params gracefully;
Comment #1 by razvan.nitu1305 — 2022-11-18T13:38:56Z
I would argue that problem number 2 is not valid. The compiler just looks at the signature and it sees that unsignedToTempString has a scope return parameter and assumes that that is the one you are returning so it mentions that one. It makes more sense to mention the variable that the user can actually change, rather than the library function parameter which cannot. However, points 1 and 3 definitely need fixing.
Comment #2 by pro.mathias.lang — 2022-11-18T13:53:38Z
(In reply to RazvanN from comment #1) > I would argue that problem number 2 is not valid. The compiler just [...] The problem I see is that the message is misleading without extra information. In this case, to understand it, one needs to look at `unsignedToTempString` signature. The fix, in that case, is to change `sink` signature. Note that I am not saying that the message is *incorrect* (it most definitely is correct) or that it shouldn't mention `buffer` - Just that it can & should be improved, perhaps with an `errorSupplemental` (`deprecationSupplemental` here) so that people don't get stuck looking at the call that returns the reference, instead of how the returned reference is used.
Comment #3 by dkorpel — 2022-11-18T14:19:14Z
(In reply to Mathias LANG from comment #2) > Note that I am not saying that the message is *incorrect* (it most > definitely is correct) or that it shouldn't mention `buffer` - Just that it > can & should be improved, perhaps with an `errorSupplemental` > (`deprecationSupplemental` here) so that people don't get stuck looking at > the call that returns the reference, instead of how the returned reference > is used. I'm working on a fix for the `` bug and `__anonymous_param`, I'll leave the mentioning of `unsignedToTempString` as a future enhancement since currently the information where `buffer` got escaped from is discarded, so it requires a lot of refactoring of `escapeByValue` and `escapeByRef`.
Comment #4 by dlang-bot — 2022-11-21T13:30:34Z
@dkorpel created dlang/dmd pull request #14649 "Fix 23491 - Nonsensical deprecation message when using delegate" fixing this issue: - Fix 23491 - Nonsensical deprecation message when using delegate https://github.com/dlang/dmd/pull/14649
Comment #5 by dlang-bot — 2022-11-28T07:49:21Z
dlang/dmd pull request #14649 "Fix 23491 - Nonsensical deprecation message when using delegate" was merged into master: - d637dfdf254fd7e63b485f6570723849e28e36eb by Dennis Korpel: Fix 23491 - Nonsensical deprecation message when using delegate https://github.com/dlang/dmd/pull/14649