search

msoucy / dproto

D Protocol Buffer mixins to create structures at compile time
Boost Software License 1.0
37 stars 16 forks source link

UDA-based interface #55

Closed msoucy closed 8 years ago

msoucy commented 9 years ago

This is a rather large change, so I'm requesting a second (and third...) set of eyes on this.

It replaces the majority of the buffer-based code with a UDA-based approach. Since it's such a large change, I'm a bit worried that I accidentally broke something that the unit tests don't yet cover.

@denizzzka , @Downchuck mind taking a look?

msoucy commented 9 years ago

Currently it fails under gdc, ldc, and dmd-2.066.1. This is because I'm explicitly disabling the default constructor, but I don't know the best way to handle a default object

denizzzka commented 9 years ago

I'm a bit worried that I accidentally broke something that the unit tests don't yet cover

Don't worry: someone will face it sooner or later, and unittests will be improved

Downchuck commented 9 years ago

@msoucy I'll also take a look on that constructor issue.

Downchuck commented 9 years ago

Not sure here, but I think you can use "alias" instead of ProtoField on those two offending lines. Not sure what's going on with string.msgType property in that ProtoField struct.

msoucy commented 9 years ago

So, it CAN be used, but there's one problem: GDC and LDC2 don't support FieldNameTuple either...

Downchuck commented 9 years ago

Looks like that's in 2.067, and the current GDC/LDC2 are not yet caught up. I don't want to get too caught up in old version issues, I don't know what the right decision is.

The FieldNameTuple template looks small and simple, not sure if it can just be patched in with a version check on compilation: https://github.com/D-Programming-Language/phobos/commit/c9f2202f768d5922c472455808496ddbc2752c3f

msoucy commented 8 years ago

Any final comments? If not, I'll be merging it in on Saturday or Sunday.