dmd
dmd copied to clipboard
mtype: Add TypeAggregate and refactor repetitive code for structs and classes
Signed-off-by: Luís Ferreira [email protected]
Previously reverted https://github.com/dlang/dmd/pull/13734 by https://github.com/dlang/dmd/pull/13742 . CC @ibuclaw .
Thanks for your pull request, @ljmf00!
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 + dmd#13800"
@ljmf00 @ibuclaw what's the status of this PR?
@ljmf00 @ibuclaw what's the status of this PR?
The needless whitespace changes make it hard to vet that the functional changes to the C++ headers are OK.
@ibuclaw Fixed. Can you re-review?
Also, I'm going to add a clang-format
config proposal and try to fix the format inconsistency. It is utterly unproductive to copy and fix indentation by hand, especially the side of the pointer. There are also some types that don't correspond to the D type. It is not related to this change, so I'm going to do those in separate PRs.
Should be tested against either gdc or ldc (@kinke) to gauge how much of a nuance that asym field will be without an accessor.
This needs rebasing on top of #13862.
This needs rebasing on top of #13862.
Done. Do you want me to make sym
an accessor? Probably an inline implementation on mtype.h
is preferable.
This needs rebasing on top of #13862.
Done. Do you want me to make
sym
an accessor? Probably an inline implementation onmtype.h
is preferable.
Could we instead turn this on its head and keep the sym fields in TypeClass and TypeStruct, instead having the accessor in TypeAggregate?
Also considering that getting the .sym
from the former two nodes is in the hot path, and we want to avoid a systematic slowdown by a thousand cuts.
ping @ljmf00
Could we instead turn this on its head and keep the sym fields in TypeClass and TypeStruct, instead having the accessor in TypeAggregate?
Also considering that getting the
.sym
from the former two nodes is in the hot path, and we want to avoid a systematic slowdown by a thousand cuts.
I think moving the asym
to TypeClass
and TypeStruct
will make this change less useful. The accessors are there just to facilitate the casting, ultimately, they should be inlined. The choice of not having the accessor would require casting the asym
to the respective type when using the specialization of either TypeClass
or TypeStruct
, but when dealing with TypeAggregate
I would still have the abstraction.
If it's not what you mean, can you clarify?