Skip to content

Fix bug 8858#268

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

Fix bug 8858#268
marler8997 wants to merge 1 commit intodlang:masterfrom
marler8997:bug8858

Conversation

@marler8997
Copy link
Contributor

@dlang-bot
Copy link
Contributor

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
8858 DMD's -v option doesn't output dependencies with imports inside functions

@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 Dec 5, 2017
@marler8997
Copy link
Contributor Author

Looks like the jenkins build failed with this:

/var/lib/jenkins/dlang_projects@3/distribution/bin/../imports/core/runtime.d(653): Error: module config is in file 'rt/config.d' which cannot be read

@andralex
Copy link
Member

andralex commented Dec 5, 2017

@marler8997 appreciate you looking this also cc @wilzbach @CyberShadow @RazvanN7 who at various points looked into this.

@marler8997
Copy link
Contributor Author

If dlang/dmd#7400 gets accepted into DMD , then rdmd could use the new -deps- option instead of -deps. This causes the compiler to still analyze the dependencies but doesn't print them which make more sense for rdmd. I would suggest waiting to see if that PR gets accepted before analyzing this one.

@CyberShadow
Copy link
Member

This slows down compilation significantly...

@marler8997
Copy link
Contributor Author

Not surprising. It is of course required for rdmd to work with local imports. Performance and robustness always seem to be at odds:) This bug would also become much worse if lazy imports were implemented.

Of course, if rdmd starts using the "compile imports" feature then this would solve the performance problems. I also confirmed that the "compile imports" feature works with local imports, so maybe we forego this change and wait to see if that gets accepted?

@CyberShadow
Copy link
Member

I don't think there is an immediate urgency to fix 8858/7016 in rdmd, however, the compilation speed drop will affect all rdmd users. So, perhaps we should delay this until the situation with DMD doing recursive compilation becomes more clear.

Thank you for working on this regardless.

@marler8997
Copy link
Contributor Author

Note that this Pull Request becomes obsolete if both (dlang/dmd#7099) and (#271) are accepted.

@dlang-bot dlang-bot added the PR.NeedsAttention A PR that is stalled/not mergeable anymore/abandoned and needs to be taken over label Jan 16, 2018
@marler8997 marler8997 closed this Jan 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR.NeedsAttention A PR that is stalled/not mergeable anymore/abandoned and needs to be taken over 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