-
-
Notifications
You must be signed in to change notification settings - Fork 714
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
Use alias assignment in Filter #8040
Conversation
Thanks for your pull request, @andralex! Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub run digger -- build "master + phobos#8040" |
generated/dscanner-9364d6f15f4a610fda49a693dbc18608bfc701bb/dsc --config .dscanner.ini --styleCheck etc std -I.
---
| std/meta.d(885:20)[error]: no identifier for declarator
| std/meta.d(885:44)[warn]: Empty declaration
| posix.mak:577: recipe for target 'dscanner' failed The style checker seems not to like this idiom. |
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.
Wait a minute. Isn't alias assigment currently an experimental feature? If it is rejected or changed later on, we will have problems.
Oh I see that you explained using experimental features in #8033. I assume you have discussed with Walter and/or Atila about the issue? |
Yes, @WalterBright and @atilaneves are in the loop. Essentially we can't get an estimate on how well the feature could fare outside the stdlib because the stdlib would be its primary beneficiary (which would then improve the life of client code). The PRs are intentionally bite-sized so we can easily dustmite and pinpoint performance problems back to them. |
Worst case you could always revert the specific commits that use this if the feature were removed too. Probably best to keep a list of them just in case. |
It took me a sec to realize what the recursive AliasSeq assignment was with the eponymous alias but I get it now and indeed this is really quite decent. Cool. |
Someone would need to upgrade/improve/fix dscanner before this can be used. |
That has been fixed. :) |
Code is greatly simplified, memory consumed is reduced.