Skip to content

Fix bug 18079 - rdmd does not discover all dependencies#291

Closed
marler8997 wants to merge 1 commit intodlang:masterfrom
marler8997:bug18079
Closed

Fix bug 18079 - rdmd does not discover all dependencies#291
marler8997 wants to merge 1 commit intodlang:masterfrom
marler8997:bug18079

Conversation

@marler8997
Copy link
Contributor

Fix for issue https://issues.dlang.org/show_bug.cgi?id=18079

This will result in a known performance decrease but this is unavoidable unless we can modify rdmd to invoke dmd 1 time instead of 2.

@dlang-bot
Copy link
Contributor

dlang-bot commented Jan 20, 2018

Thanks for your pull request, @marler8997! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.

Some tips to help speed things up:

  • smaller, focused PRs are easier to review than big ones

  • try not to mix up refactoring or style changes with bug fixes or feature enhancements

  • provide helpful commit messages explaining the rationale behind each change

Bear in mind that large or tricky changes may require multiple rounds of review and revision.

Please see CONTRIBUTING.md for more information.

Bugzilla references

Auto-close Bugzilla Description
18079 rdmd does not discover all dependencies

@dlang-bot dlang-bot added the Type.Bug Things don't work as they were intended to, or the way they were intended to work doesn't make sense label Jan 20, 2018
@marler8997 marler8997 changed the title Fix bug 8858 Fix bug 18079 Jan 20, 2018
@timotheecour
Copy link
Contributor

indeed, on a simple test using time command, simply adding -deps kills performance. This defeats purpose of this PR and justifies the need for #271

time rdmd --force --extra-file=fun2.d --extra-file=fun2.d --extra-file=fun3.d -deps=/tmp/z01.txt main.d

0.74s user 0.13s system 98% cpu 0.883 total

time rdmd --force --extra-file=fun2.d --extra-file=fun2.d --extra-file=fun3.d main.d
 0.12s user 0.04s system 94% cpu 0.172 total

@timotheecour
Copy link
Contributor

timotheecour commented Jan 20, 2018

NOTE: just filed this bug, it prevents this PR from working at all currently: https://issues.dlang.org/show_bug.cgi?id=18271

@JinShil JinShil changed the title Fix bug 18079 Fix bug 18079 - rdmd does not discover all dependencies Jan 20, 2018
@marler8997
Copy link
Contributor Author

marler8997 commented Jan 20, 2018

I think this may be by design. It only performs the full semantic analysis on modules imported by the first module given on the command line. I remember seeing the code do this and I thought it was odd but I assumed there was a reason for this.

@marler8997
Copy link
Contributor Author

https://github.com/dlang/dmd/blob/f947c0881432988dcd8cd80d5abe71e4c0463867/src/dmd/mars.d#L822

This is where the extra semantic analysis is done when the -deps flag is used. As you can see, only module[0].aimports is being scanned. This is either a bug or by design. If it's a bug, then the fix is likely to loop over Modules.modules instead of modules[0].aimports.

@timotheecour
Copy link
Contributor

thanks, replied directly on bug report (https://issues.dlang.org/show_bug.cgi?id=18271#c2) to keep that side discussion separate

@wilzbach wilzbach added the PR.Blocked A PR blocker by another issue / PR, external to the PR (as opposed to WIP) label Feb 11, 2018
@marler8997 marler8997 closed this Mar 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR.Blocked A PR blocker by another issue / PR, external to the PR (as opposed to WIP) Type.Bug Things don't work as they were intended to, or the way they were intended to work doesn't make sense

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants