Thread overview
Draft Review DIP: Enum and Function Parameter Attributes
Mar 21, 2018
Mike Parker
Mar 21, 2018
Meta
Mar 21, 2018
Mike Parker
Mar 21, 2018
Meta
March 21, 2018
As it stands, this DIP [1] is currently the candidate to become DIP 1013. Any an all feedback for Draft Review is welcome. Please read the intent [2] behind the Draft Review before participating. Thanks!

[1] https://github.com/dlang/DIPs/pull/105
[2] https://github.com/dlang/DIPs/blob/master/PROCEDURE.md#draft-review
March 21, 2018
On Wednesday, 21 March 2018 at 00:12:43 UTC, Mike Parker wrote:
> As it stands, this DIP [1] is currently the candidate to become DIP 1013. Any an all feedback for Draft Review is welcome. Please read the intent [2] behind the Draft Review before participating. Thanks!
>
> [1] https://github.com/dlang/DIPs/pull/105
> [2] https://github.com/dlang/DIPs/blob/master/PROCEDURE.md#draft-review

Low hanging fruit:

IMO, the abstract is not right. This may differ between institutions, but the way I was taught to write abstracts is that they should be a condensed summary of what your document is about. Think of it as an overview so someone can quickly read it to get an idea of what you'll be talking about through the rest of the document.


> It is currently not possible to attach attributes to both enums and function parameters

Again, probably a matter of taste, but I think this reads better as "...not possible to attach attributes to enums _or_ function parameters..."


> Attributes and user-defined attributes (UDA) serve as a means to provide extra meta data

I believe it's usually written "metadata", as one word. See this from Google Books' Ngram viewer:
https://books.google.com/ngrams/graph?content=meta+data%2Cmetadata&case_insensitive=on&year_start=1965&year_end=2000&corpus=15&smoothing=3&share=&direct_url=t4%3B%2Cmeta%20data%3B%2Cc0%3B%2Cs0%3B%3Bmeta%20data%3B%2Cc0%3B%3BMeta%20Data%3B%2Cc0%3B%3BMeta%20data%3B%2Cc0%3B%3BMETA%20DATA%3B%2Cc0%3B.t4%3B%2Cmetadata%3B%2Cc0%3B%2Cs0%3B%3Bmetadata%3B%2Cc0%3B%3BMetadata%3B%2Cc0%3B%3BMETADATA%3B%2Cc0


> What can be said for why attributes were included as a feature in D can also be said for why they should be extended to enums and function parameters.

This sentence reads very awkwardly.


> It is benefitial to provide extra meta data about a symbol that can be used at compile-time.

You assert this but don't provide any logical argument or examples of why it's useful (yes, I know you have examples below).


> Most of the framework for attributes already exist and it would just be...

Should be "most of the framework for attributes already _exists_". Also, it's more a matter of personal taste, but I would suggest an Oxford comma between "exist" and "and" in that sentence.


> Allow additional meta information (attributes) to be attached with enums and functions parameters

Should be "...to be attached _to_ enums and _function_ parameters..."


> ...and it would just be a matter of extending that into the respective symbols...

I think that this should be "to" as opposed to "into".


> A current solution for applying an UDA...

Should be "_a_ UDA".


> A solution for applying the deprecation attribute to an enum member can be done by
> reimplementing an enum as a structure with static enums. This allows the attribute to be
> placed with the desired enum member. While still allowing for any existing code that
> simply used an enum before hand to still work accordingly, as if the struct was an enum.

> enum SomeEnumImpl
> {
>     none         = -1,
>     actualValue2 = 2,
>     actualValue3 = 3,
> }

> struct SomeEnum
> {
>     SomeEnumImpl x;
>     alias this x;
> 
>     deprecated("reason for deprecation")
>     static enum deprecatedValue0 = none;

>     deprecated("reason for deprecation")
>     static enum deprecatedValue1 = none;
> }

AFAIK `static` on an enum does nothing. Also, `enum deprecatedValue0 = none` should qualify `none`, i.e., `SomeEnumImpl.none`.


> Allowing to deprecate enums which should not be used anymore as seen here:

This language is sloppy. It could be better written as "Deperecating individual enum values" or something similar. Also, this is just a copy&paste of the same example from the "Existing Solutions" section; I don't think the whole bit with creating a struct with enum members, etc. really needs to be repeated.


All of these examples except the first seem to rely on function parameter attributes being accessible outside that function. If I recall correctly, this caused some controversy in past discussion which must be addressed in this DIP.
March 21, 2018
On Wednesday, 21 March 2018 at 02:26:02 UTC, Meta wrote:

>
> Low hanging fruit:
>

Thanks for the feedback, but Draft Review comments should go in the PR thread. I'll add a link over there to the post, but when you get a chance, it would help to paste your remarks into a comment there.

Thanks!
March 21, 2018
On Wednesday, 21 March 2018 at 05:57:47 UTC, Mike Parker wrote:
> On Wednesday, 21 March 2018 at 02:26:02 UTC, Meta wrote:
>
>>
>> Low hanging fruit:
>>
>
> Thanks for the feedback, but Draft Review comments should go in the PR thread. I'll add a link over there to the post, but when you get a chance, it would help to paste your remarks into a comment there.
>
> Thanks!

Whoops, sorry.
March 21, 2018
On Wednesday, 21 March 2018 at 00:12:43 UTC, Mike Parker wrote:
> As it stands, this DIP [1] is currently the candidate to become DIP 1013. Any an all feedback for Draft Review is welcome. Please read the intent [2] behind the Draft Review before participating. Thanks!
>
> [1] https://github.com/dlang/DIPs/pull/105
> [2] https://github.com/dlang/DIPs/blob/master/PROCEDURE.md#draft-review

Shouldn't this be [0] to start reading, and than comment at [1] ?
[0] https://github.com/skl131313/DIPs/blob/9c44cfad7175ae9c68b244794d91981d73cc0e1d/DIPs/dip10ss.md