std.typetuple: Add All, Any, Not for template predicates.#690
std.typetuple: Add All, Any, Not for template predicates.#690andralex merged 3 commits intodlang:masterfrom
Conversation
There was a problem hiding this comment.
By the way, do we have a policy on when to add one's name to the Authors list? Other open source projects sometimes require adding to the authors list when adding new top level primitives, so I did, but on the other hand, the additions are hardly very original…
There was a problem hiding this comment.
I'm not aware of any official policy. I generally add my name to anything for which I've made any contributions which seem significant enough (though that's obviously vague and subjective). Some people do the same, while others almost always forget to add their names (e.g. I think that I'm the one who added Kenji's name to std.conv in spite of the fact that he's done a ton of work on it). In general, I'd say that you should add your name when you feel like it's a large enough contribution that it makes sense to have your name on it. If anyone disagrees with you putting your name on a module, they can say so. You've added several functions, so I see no problem with you adding your name to the list. And if they're unoriginal enough that someone can argue that they're not really copyrightable, then oh well. It's Boost, so the license is already about as unrestrictive as a license gets anyway, so I see no reason to get super picky about it.
There was a problem hiding this comment.
You add names, you add your name to the authors list.
|
Weren't you arguing that these should work on I don't know how valuable it really is for these sorts of templates to take functions (any type of callable really with how I did it in |
|
Maybe someone else can weigh in the |
|
I guess it depends on your usage. I have always exclusively used template predicates with »type tuple algorithms«, also when operating on values (e.g. the result of Hm, should the term be »predicate template« instead of »template predicate«, which could be interpreted as »predicate acting on templates«? And by the way, adding these to |
That's a good idea. I'll have to look at doing that and adding it to the
Well, with functions, it's a function predicate, not a predicate function, so I'd argue for template predicate.
This has everything to do with In any case, I'd say that we should just put these in |
|
|
|
Every single
That's one of those cases where there isn't any good place to put it, and that's just where it somehow ended up (though it was originally std.contracts, which was a bad name for everything in that module). So, I don't see much point in complaining about that one, as silly as where it is may be - though if you view it as type construction, I suppose that it could go in std.typecons. |
That's like saying writeln has to do with type tuples because it also |
std/typetuple.d
Outdated
There was a problem hiding this comment.
The naming convention for template names is: if it ultimately evaluates to a value -> lowercase. Otherwise -> uppercase. So this should be "not", not "Not".
There was a problem hiding this comment.
We were debating this in another pull request (pull #688 IIRC). These dont' return either values or types. They return templates, so it's not at all clear whether they should be either camelCased or PascalCased. But they're specifically compile-time constructs, and most compile-time constructs are types, so it makes good sense that they'd use the same naming conventions. Also, if you camelCase them, they quickly start conflicting with functions. We already have std.functional.not, so making this not would` conflict with that.
I fully support the naming conventions of camelCasing templates which resolve to values and PascalCasing those that resolve to types, but I think that we should be defaulting to PascalCasing with templates like this which evaluate to other templates or for which it's not at all clear whether they're really evaluating to a value or to a type.
There was a problem hiding this comment.
As soon as one sees "enum Not =" it becomes mightily clear it's a value. Case closed.
There was a problem hiding this comment.
The point is that Not is a template which evaluates to another template. The fact that enum Not appears in the text is just a consequence of the »inner« template having to be eponymous to the outer template.
Both Jonathan and I are aware of the common Phobos conventions concerning the casing of symbol names. Still, we both intuitively chose to put the template predicate primitives in PascalCase. This is simply a case not covered by the current »value/type« conventions. The rules I (and presumably Jonathan) would propose for choosing template names are:
- If a template returns a (type) tuple, its name is PascalCased.
- If a template returns a type, i.e. is(T!Params), its name is PascalCased.
- If a template returns a value, i.e. is(typeof(T!Params)), or returns a fully IFTI-able template function (and thus behaves like a function), its name is camelCased.
- Otherwise, e.g. if the template returns another template, or simply isn't eponymous at all, it is PascalCased.
Now, back to the specific case discussed here. Originally, I just intuitively chose the names, and they still seem to fit well with the rest of my Phobos-convention code. But thinking a bit about it, I think there are two main reasons why the tmeplate predicate primitives should be cased like this:
First, for consistency, because »higher order templates«, i.e. templates acting on other templates, are generally in PascalCase (I don't think there is disagreement about this), and I tend to see Not as a special case of template composition. For example, if Phobos had Compose and PApply templates, `Not could easily be implemented like this:
template not(bool value) { enum not = !value; }
alias PApply!(Compose, not) Not;Thus, having it in upper case just seems a lot more consistent to me. A template which returns another template »feels« completely different from a template which evaluates a value – for instance, you can't even directly get a value out of a Ǹot instance, unless you resort to a kludge like Instantiate.
The second reason is that it offers a nice way to mentally disambiguate between compile-time and run-time »algorithms«. For example, std.typetuple.not would conflict with std.functional.not, as Jonathan also mentioned above. Granted, this can't be a reason to break an otherwise consistent naming convention – after all, D has renamed imports and all that goodness. But I think it would be unwise to ignore this benefit when defining the naming convention in the first place.
To wrap up, I'm all for following the Phobos conventions with regard to naming. I just don't think they are as clear as it might seem here. You are right about pred, though, I'll definitely change that.
There was a problem hiding this comment.
In my opinion, your latter point is precisely why we should go for for upper case. It returns a template, it's upper case. Simple as that. Whether that template ultimately evaluates to a value is more of a detail.
But I'm fine with changing the names if you insist on it. I just think it's bad for usability.
There was a problem hiding this comment.
No, not at all. I'm sorry, you got it backwards. The fact that it returns a template is not even important to the end user because the eponymous trick makes it all transparent. At the end of the day, any instantiation of Not whatsoever will produce a value.
If that's not enough of an argument, if we applied your approach to Phobos in general, a bazillion names would need to be suddenly uppercased. Look no further than https://github.com/D-Programming-Language/phobos/blob/master/std/algorithm.d#L367. Surprise, map "returns" a template! Of course, nobody cares, but by your algorithm we should rename it to Map.
Argument destroyed?
There was a problem hiding this comment.
@andralex: No, no, the fact that Not returns a template is important. It's its sole purpose – the end user instantiates Not to avoid having to write his own template.
Compare this to IFTI (as in your map example), where the user doesn't even need to know that there is a template. Note that because of this, I even explicitly mentioned IFTI in my »algorithm«. Ha! :P
Which names in Phobos would need changing in your opinion (besides staticMap, which is a misnomer)?
There was a problem hiding this comment.
(The point being that we need a »templates which return templates are PascalCased« rule anyway for things like PApply, Compose, etc. Why not just apply it here as well and call it a day?)
There was a problem hiding this comment.
I think we've reached an impasse here.
|
Is std.typetuple the best place for this? |
Where else would they go? They're operating on |
1 similar comment
Where else would they go? They're operating on |
|
Oh, the much more important thing: Are we going to stick with putting them into |
std/typetuple.d
Outdated
There was a problem hiding this comment.
Funny – to my non-native, but mainly BE-trained eyes, they are just the same in this context, but apparently, that is preferred for »restricting« clauses in AE style guides. Would never have guessed that…
|
Also the pull request needs rebasing. |
|
Sorry for neglecting this pull request – I'm currently in the middle of a multi-week exam session, and have been using my learning breaks mainly for participating in futile NG discussions. ;) I'm still not convinced that making The much more important question, though, is in which module to put the templates. For the three discussed here, |
|
Darn, now |
|
Yeah. Introducing it as However, given that |
|
@jmdavis: No, as far as I know, there is no way to write a constraint which disambiguates between them – |
|
(leaving tests in the broken state for further discussion, will come back to the pull request before next Sunday) |
|
I've been thinking about this more and there's a concern from another angle. Let's set aside "not" for a moment. This diff grabs two good names (any, all) for a rare use case, mostly intended for specialists and library writers. Users would routinely expect that "any" and "all" are range-oriented functions that take a Boolean predicate and a range. So I wonder what would be a good prefix or suffix for "not", "any", and "all" that would clarify their specialized use. Please chime in with any (and all) ideas that you might have. Thanks! (I believe just as strongly that "Not", "Any", and "All" should not be among candidates.) |
|
The obvious choice would be |
|
staticXxx sounds good to me (and has a precedent) |
|
ping? |
|
reping |
|
Common! Looks like nobody here looked at Issue 3702, Comment 3. I mean at least |
Names in lowercase as per Andrei's request, see discussion in pull request dlang#690.
|
@andralex, @jmdavis: Sorry for letting you wait – I had an intense exam period at the end of August, and this somehow slipped of my radar. Anyway, I'm not exactly fond of the Besides that, I still think the names should be uppercase. First, I don't know what should be confusing or »overthought« about the rules I proposed: if the template result acts like a value (values, functions, and fully IFTI-able template functions), it's lowercase, else it's uppercase. But the real reason why I think going for upper case would make sense is that template-returning templates are simply a difficult beast to deal with. For example, you can't directly use them without binding them to an alias or using a hack like But I'm willing to accept that I'm in the minority with this opinion here – at least, I don't want to waste even more time on bikeshedding anymore. |
std.typetuple: Add All, Any, Not for template predicates.
|
Merged, thanks for the flexibility! |
|
Spoke too soon, this breaks the build. I reverted now, please reopen when the breakages have been fixed. Thanks! |
See pull request #689 – I also more or less borrowed some of Jonathan's
unit tests.
I'm not sure whether
Any/AllorAnd/Oris the better namingscheme: The former makes it clearer that the templates operate on a
variable number of predicates, whereas the latter avoids any potential
confusion with
anySatisfy/allSatisfy.Also, it can be debated whether the »inner templates« should have a
constraint restricting the argument tuple length to 1 – in virtually
all applications, template predicates will operate on exactly one
argument, but as the templates are theoretically capable of operating
on generic signatures, I didn't add a check.