Skip to content

Remove exception for eponymous template#700

Closed
Geod24 wants to merge 1 commit intodlang:masterfrom
Geod24:update-style-guide
Closed

Remove exception for eponymous template#700
Geod24 wants to merge 1 commit intodlang:masterfrom
Geod24:update-style-guide

Conversation

@Geod24
Copy link
Member

@Geod24 Geod24 commented Nov 12, 2014

I would like to remove this exception from the style guide.

It was originally added some 4 years ago (0749288), but there's no PR for it so I couldn't find any relevant discussion).
It looks like that was a de-facto standard, as e.g. std.typetuple.IndexOf was renamed to indexOf in 2007 (relevant Phobos commit).
I think the reason was to make the intent of the template clear to the reader. However, it overlooked the fact that such templates are often CT implementations of RT function, and thus we got name clashes. This leads to a situation where we have inconsistent name between CT and RT version.
Example:

  • std.algorithm.all / std.typetuple.allSatisfy;
  • std.algorithm.any / std.typetuple.anySatisfy;
  • std.algorithm.map / std.typetuple.staticMap;
  • indexOf / staticIndexOf;

With the effort to move std.typetuple to std.meta.list and draw a clearer separation between tuple and argument list (dlang/phobos#2687), it could also be a good time to consolidate the naming between CT and RT algorithm, and avoid double deprecation time.

@dnadlinger
Copy link
Contributor

Objection.

This distinction is still applied, and is helpful when making sense of generic code. The fact that there is no discussion just means that everybody agrees on it (actually, there used to be quite a bit of discussion from time to time on the finer details – for example, if you are in the mood for some bikeshedding, have a look at my original TemplateAnd/Or pull request).

With regard to name clashes, just use static/renamed/selective imports. My original idea was to have all un-prefixed names in std.meta (e.g. just List or Seq) and encourage use of import meta = std.meta; in code that is not exclusively template metaprogramming.

@Geod24
Copy link
Member Author

Geod24 commented Nov 12, 2014

Not in the mood for bikeshedding, but I had a glance. (For the record: dlang/phobos#690 ).

That confirms what I think: this exception led to a lot of discussion with no obvious advantage. Now we are left with a suffix/prefix that is distinct to each template instead of sharing the same name with a different casing (which makes it much easier to understand / discover). We are trading the benefit of making Phobos easier to use for an hypothetic help when reading generic code (which can only be enforced in Phobos).

With regard to name clashes, just use static/renamed/selective imports. My original idea was to have all un-prefixed names in std.meta (e.g. just List or Seq) and encourage use of import meta = std.meta; in code that is not exclusively template metaprogramming.

This would be the best course of action, no doubt about it. However, there is no way to ensure a specific usage of a module, and relying on a convention would be worse than the actual situation.

@mihails-strasuns
Copy link

Objection++

This looks like another "D module system is not working because symbols clash" debate to me. Such templates use same style as similar functions exactly because they are often just CT implementations for those. The fact that we have anySatisfy instead of just any is totally different problem - one that Phobos devs historically didn't make any good use of scoped/selective/renamed imports and advocating those to users as a way for name resolution.

Actually I'd love to rename those to plain any / all etc. while deprecating old std.typetuple

(of cource DMD name resolution bugs do not help)

@ntrel
Copy link
Contributor

ntrel commented Nov 13, 2014

Actually I'd love to rename those to plain any / all etc. while deprecating old std.typetuple

Interesting, so we could stop trying to avoid clashes and use meta = std.meta where appropriate (and recommend that in the module docs). We could have:

TypeTuple -> List
staticIndexOf, staticMap -> indexOf, map
templateAnd, templateNot, templateOr -> and, not, or

This seems quite elegant.

@Geod24
Copy link
Member Author

Geod24 commented Nov 15, 2014

Objection++

This looks like another "D module system is not working because symbols clash" debate to me. >Such templates use same style as similar functions exactly because they are often just CT implementations for those. The fact that we have anySatisfy instead of just any is totally different problem - one that Phobos devs historically didn't make any good use of scoped/selective/renamed imports and advocating those to users as a way for name resolution.

Actually I'd love to rename those to plain any / all etc. while deprecating old std.typetuple

(of cource DMD name resolution bugs do not help)

It isn't. D module system is working. One can however challenge it's default (i.e. local by default when imported at global scope) in regards of the current issues, but that's another topic.

I think if we are going the way @klickverbot suggest, we should have a way to let the user know which corrective action should be taken, at CT.

@jmdavis
Copy link
Member

jmdavis commented Nov 17, 2014

We should probably give templates their own section rather than put then lumping them in with the classes, interfaces, etc. However, the naming rules will definitely not change. Eponymous templates are supposed to keep the naming rules of whatever they're wrapping. If they wrap a type, then they're PascalCased. If they wrap a value, then they're camelCased. Anything in Phobos which does not follow this is in violation of the style guide and ideally would be changed (though in some cases may not be if we don't deem it worth breaking backwards compatibility to fix it).

Regardless, I think that it's quite clear that the proposed change is unacceptable.

@jmdavis jmdavis closed this Nov 17, 2014
@jmdavis
Copy link
Member

jmdavis commented Nov 17, 2014

How about this instead: #703

@Geod24 Geod24 deleted the update-style-guide branch February 9, 2015 12:05
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.

5 participants