www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.announce - Draft Review DIP: Enum and Function Parameter Attributes

reply Mike Parker <aldacron gmail.com> writes:
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
Mar 20 2018
next sibling parent reply Meta <jared771 gmail.com> writes:
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.
Mar 20 2018
parent reply Mike Parker <aldacron gmail.com> writes:
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!
Mar 20 2018
parent Meta <jared771 gmail.com> writes:
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.
Mar 21 2018
prev sibling parent Martin Tschierschke <mt smartdolphin.de> writes:
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
Mar 21 2018