fix Issue 17423 - pointer assignment to in member function is not ac… #7999
Conversation
|
Thanks for your pull request, @WalterBright! Bugzilla references
|
WalterBright
force-pushed
the
fix17423
branch
2 times, most recently
from
09bdcbd to
2cdcaa3
Compare
| foreach (VarDeclaration v; ad.fields) | ||
| { | ||
| if (v.hasPointers()) | ||
| return stc; |
There was a problem hiding this comment.
don't know what you mean
There was a problem hiding this comment.
What about it? Please don't make me guess.
There was a problem hiding this comment.
Sorry - my point was only that these two lines are uncovered, thus return stc is never called and thus all code that you added has never an effect for the testcase.
There was a problem hiding this comment.
Should be covered now.
| foreach(_; o) { i = 0; } | ||
| i = x; | ||
| } | ||
| } |
There was a problem hiding this comment.
The test file from the bug report still triggers in a segfault (with this PR).
There was a problem hiding this comment.
It fails to compile with -dip1000
There was a problem hiding this comment.
Oh my point was that 17423 isn't fixed by this PR because the segfault still exists and with -dip1000 it currently doesn't segfault: https://run.dlang.io/is/W1DGlg
There was a problem hiding this comment.
My point is -dip1000 is required to fix it. The whole point of dip1000 is to fix these sorts of issues.
|
Why are the coverage test results not showing up in the code view anymore? |
They are for me:
Though I needed to hit refresh after I went to the diff view, maybe that's a new bug from CodeCov ... |
|
Yes, it worked for me too after I hit refresh. Thanks for the tip! Anyhow, it's all green now and good to go. |
|
FYI: I opened an issue (codecov/browser-extension#53), but the extension seems to be unmaintained (codecov/browser-extension#52). |
|
Done. Please consider returning the favor by reviewing some of the pull requests that require someone with your unique skills and position. |
…counted for