Fix for issue 19656 #9462
Fix for issue 19656 #9462
Conversation
When DMD encounters an associative array with a class type (say Foo) specified as key, the compiler immediately wants to analyze the Foo class. And in some cases it might so happen that DMD was already in the process of analyzing Foo. In such a scenario, the visit(ClassDeclaration ) method malfunctions and marks Foo with PASS.semanticdone without actually completing analysis of the class. Since this method marks a ongoing call with setting _scope to null, this situation can be avoided by testing _scope for Foo before calling the dsymbolSemantic method. This makes sure that if semantic analysis for Foo is already running, it would not be run again.
|
Thanks for your pull request and interest in making D better, @puneet! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please see CONTRIBUTING.md for more information. If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment. Bugzilla references
Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub fetch digger
dub run digger -- build "master + dmd#9462" |
|
You need to make the commit message say "Fix issue 19656" for the bot to properly recognise it. |
|
Thanks for being patient with me. This is my first PR for dmd. Now as I run DMD with the fixes, I am getting more such issues with circular dependency. In all I have filed 5 issues on Bugzilla related to circular references: https://issues.dlang.org/show_bug.cgi?id=19655 This PR resolves the first three issue. Issue 19746 which I discovered later needs a similar fix and I can create a separate PR for that. But there can be a generic fix which can avoid all these issues and perhaps many more which have not been discovered. It involves making sure that if a ClassDeclaration is being analyzed, then any recursive call (due to circular refs) for semantic analysis should be rejected irrespective of where it originates from. I am going to create a PR with this generic fix. If you find that in order, then we do not need separate PRs for these issues. |
No problem. Thanks for your contributions.
In which case please change the commit message to "fixes issues 19655, 19656 & 19657"
That would be preferable. |
What is the value of semanticRun at this point? PASS.init or PASS.semantic? |
PASS.semantic |
Alright, I will add the more generic PR that fixes all the 5 issues. Since all these issues are regressions wrt v2.074 and v2.070, should I create the PR on stable branch or on master branch? |
|
Preferably stable. |
Thanks. |
|
I see that #9471 has been merged. Is this still needed? |
When DMD encounters an associative array with a class type (say Foo) specified as key, the compiler immediately wants to analyze the Foo class. And in some cases (circular module references), it may so happen that DMD may already be in the process of analyzing Foo. In such a scenario, the second call to visit(ClassDeclaration ) method malfunctions and marks Foo with PASS.semanticdone without actually completing analysis of the class.
Since this method marks a ongoing call with setting _scope to null, this situation can be avoided by testing cldec._scope for Foo before calling the dsymbolSemantic method. This makes sure that if semantic analysis for Foo is already running, it would not be run again.