Simplify and improve Erase and EraseAll

[andralex]

This improves memory consumed while building Phobos as follows:

Before:

Building Phobos release: KB=1571056 s=13.13
Building std.meta unittests: KB=326300 s=2.86

After:

Building Phobos release: KB=1518000 s=12.93
Building std.meta unittests: KB=323552 s=2.87

[dlang-bot]

Thanks for your pull request, @andralex!

Bugzilla references

Your 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 locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + phobos#8033"

[maxhaton]

std/meta.d(348:22)[error]: no identifier for declarator
--
  | std/meta.d(348:48)[warn]: Empty declaration

DScanner doesn't like AliasAssign by the looks of things?

[RazvanN7]

@andralex Are these AliasAssign PRs meant for merging or just to test/try the idea out? I was under the impression that AliasAssign is going to be tested with phobos and if it brings any benefits we'll integrate it (possibly under a switch), otherwise we will revert the addition.

If that is the case, what threshold are we considering? It seems that this addition requires 3.3% less memory when building phobos (and 0.84% less memory when building std.meta unittests).

[RazvanN7]

@maxhaton @andralex Dscanner uses libdparse [1] for parsing, which uses dsymbol [2] to do symbol lookup. I assume that the latter needs to be updated. However, we don't know at this point if AliasAssign is going to go into language.

Before we have a clear decision if AliasAssign is going to be part of the language, I suggest we do not integrate it in our standard lib.

[RazvanN7]

cc @Hackerpilot

[andralex]

@andralex Are these AliasAssign PRs meant for merging or just to test/try the idea out? I was under the impression that AliasAssign is going to be tested with phobos and if it brings any benefits we'll integrate it (possibly under a switch), otherwise we will revert the addition.

If that is the case, what threshold are we considering? It seems that this addition requires 3.3% less memory when building phobos (and 0.84% less memory when building std.meta unittests).

Right now we have a catch-22: AliasAssign is not tried so we don't know if it belongs in D, and we don't know if it belongs in D so we don't try it.

These PRs are breaking the cycle. They reveal obvious advantages in simplicity that make the addition worthwhile as long as it doesn't affect negatively some metric. Virtually all PRs are net negative lines of code and the new code is much simpler.

Related: Arguably the language feature removes a limitation ("why can't I rebind an alias if I didn't use it?") rather than being a net addition. Removal of limitations has an easier path to acceptance than brand new features.

[RazvanN7]

The error from the style checker seems to come from libdparse [1].

[1] https://github.com/dlang-community/libdparse/blob/6f0893f6b07ec561d382f26c81e74f7788056828/src/dparse/parser.d#L2376

[RazvanN7]

I have made this PR to libdparse: dlang-community/libdparse#441 . That should fix the style issue once it gets in and a new tag is released for both libdparse and dscanner.

[nordlow]

I have made this PR to libdparse: dlang-community/libdparse#441 . That should fix the style issue once it gets in and a new tag is released for both libdparse and dscanner.

Are there any plans to transition dscanner to a tool that makes use of dmd as a library?