Skip to content

Adding arity traits #643

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 9, 2012
Merged

Adding arity traits #643

merged 3 commits into from
Jul 9, 2012

Conversation

gchatelet
Copy link
Contributor

First minimal addition to std.traits.
Adding an arity template returning the number of argument of a function.

Maybe it should go to std.functional instead ?

@dnadlinger
Copy link
Member

Imho, the way variadic parameters are handled (i.e. ignored) should be documented, and the description should refer to »parameters« instead of »arguments«.

Also consider leaving off the type of the constant (it should probably be size_t), it will be inferred anyway.

Anyway, I'm not quite sure whether an isolated addition like this makes much sense. I could imagine that such a template would fit well into a set of other callable type handlers/manipulators, but as it is now, I don't see much benefit over ParameterTypeTuple!().length.

@gchatelet
Copy link
Contributor Author

This patch is motivated by the following discussion on the forum. @andralex suggested to add small pull requests and proposed to start with arity.

Yes there is no real benefit over ParameterTypeTuple!().length, it just states your intents clearer.

Also

  • I agree with the uint type, I was supposed to remove it but I forgot.
  • I will also update the documentation accordingly to your comments ( parameters / variadic )

We can debate about whether this should be a coherent proposal or just some piecemeal additions.

@andralex
Copy link
Member

andralex commented Jul 2, 2012

@klickverbot: uint is better than size_t because the latter measures sizes of objects in memory whereas uint is a better fit as a "natural number".

@andralex
Copy link
Member

andralex commented Jul 2, 2012

Probably variadic functions should have uint.max as arity, or not define arity at all. I'd prefer the latter.

@dnadlinger
Copy link
Member

@andralex: typeof(TypeTuple!(1, 2, 3).length) is size_t (well, ulong instead of uint on D_LP64, that is). I thought that this might be a problem for compiling on x86_64, but forgot about all the integer range checking magic for compile-time constants.

@gchatelet
Copy link
Contributor Author

Which one of the following is recommended for the variadic function unittest ?

  • static assert(__traits(compiles,arity!variadicFoo)==false);
  • static assert(is(typeof(arity!variadicFoo))==false);

@jmdavis
Copy link
Member

jmdavis commented Jul 7, 2012

You can go with either:

static assert(__traits(compiles,arity!variadicFoo)==false);
static assert(is(typeof(arity!variadicFoo))==false);

Some people prefer one, some the other. __traits is arguably a bit clearer, but it's longer, and anyone hacking on Phobos needs to understand the is(typeof... synatx as well, since it's quite common, so on some level, the clarity gain is irrelevant. Personally, I used to prefer __traits, but increasingly, I've just been using is(typeof..., because it's shorter.

@jmdavis
Copy link
Member

jmdavis commented Jul 7, 2012

@andralex I actually would have argued size_t over uint, because that's what almost all length stuff is (the primary exception being stuff that needs to always be ulong).

@andralex
Copy link
Member

andralex commented Jul 9, 2012

Alright size_t it is. @gchatelet could you please make the change so I can pull. Thanks!

@gchatelet
Copy link
Contributor Author

@andralex : done !
Thank you all for reviewing my first submission : )

andralex added a commit that referenced this pull request Jul 9, 2012
@andralex andralex merged commit 9efd0ed into dlang:master Jul 9, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants