-
-
Notifications
You must be signed in to change notification settings - Fork 609
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix Issue 9466 - Compiler crash with code-coverage generation with large files #2785
Conversation
| fail_compilation/ice8795.d(13): Error: found 'EOF' when expecting ')' | ||
| fail_compilation/ice8795.d(13): Error: found 'EOF' instead of statement | ||
| fail_compilation/ice8795.d(14): Error: anonymous classes not allowed | ||
| fail_compilation/ice8795.d-mixin-13(13): Error: found 'EOF' when expecting '(' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh.. what kind of strange file name in the diagnostic is this? And why is this related to the COV fixes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The -cov error was caused by a string mixin. The string had more lines in it than the source file, and so there was memory corruption.
The problem is that string literals can contain an arbitrary number of lines, and those lines may have no correspondence to any source file. So what to do, what to do, to give them source/line info? My solution was to create a pseudo-filename that consisted of the source location where the mixin statement was.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will definitely break tools however. Tools which actively support D might be fixable so they take this change into account, but most text-based tools which e.g. allow go-to-file+line by double-clicking a diagnostic message will not work for string mixins anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But that never worked correctly, right?
Note that this pull is related to #426 which fixes debugging for mixins.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, broken but at least we could jump to a file. I do like #426 though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message should say, in a mixin and then point to the mixin location. Likewise cov and debug lines should all be pointing to the single line with the mixin.
fix Issue 9466 - Compiler crash with code-coverage generation with large files
|
With this change, I now get a crash in optlink when linking ddmd. |
|
When compiling with |
|
Reduced: The generate loop is just to reduce the size of the test case, the expanded version with 2048 lines of mixin works just fine. Looks hitting an internal module count limit inside optlink. This will need to be reverted if optlink can't be fixed soon. |
valgrind once again found the problem.
https://d.puremagic.com/issues/show_bug.cgi?id=9466