search

brunocodutra / metal

Love template metaprogramming
https://brunocodutra.github.io/metal/
MIT License
325 stars 23 forks source link

rename typename to class #102

Closed ecrypa closed 5 years ago

ecrypa commented 5 years ago

see #96

brunocodutra commented 5 years ago

Anything else you want to do before this gets merged?

ecrypa commented 5 years ago

In addition, I considered to benchmark the two versions. Do you think that there could be compilers that have different performance? class and typename both have several meanings, and compilers have to decide which one is relevant. I believe that the difference is negligible, but let us just make sure that you agree(?).

ecrypa commented 5 years ago

Finally, I did not touch the word "typename" in the docs; see https://github.com/brunocodutra/metal/issues/96#issuecomment-539212504. I believe that is what you want, but I feel like I should point that out.

In summary, there are only these three minor points (tail inside/outside; performance?; docs) that I am not sure about. If you think it is alright then go ahead.

brunocodutra commented 5 years ago

Regarding the questions about formatting, the whole point of setting up clang-format was to free us from worrying about this kind of detail that usually costs way more than it adds value. IMO there should be zero sections of the code in which clang-format is disabled and I believe the only reason there are some is because clang-format was generating utterly unreadable code at some point, but I'd be ok to remove those too. Unless you feel very strongly about this, I'd just let clang-format do whatever it must and move on.

I considered to benchmark the two versions

I wouldn't invest time in this, this stylistic change should not affect semantics on any ~decent~ compiler that parses code into an AST.

I did not touch the word "typename" in the docs.

It's not that I prefer one or another on the docs, my concern is whether definitions generated by Doxygen will use typename or class, have you checked this? The documentation should consistently use the same notation that Doxygen generates.

ecrypa commented 5 years ago

It's not that I prefer one or another on the docs, my concern is whether definitions generated by Doxygen will use typename or class, have you checked this? The documentation should consistently use the same notation that Doxygen generates.

I had not checked it before, but now I did. Doxygen now shows class consistently in the code.

I was referring to another topic: "typename" also appears in the docs as a word (natural English language):

https://github.com/brunocodutra/metal/blob/ebda810ea65a4cce4349a61d266a8e61822bd5db/include/metal/value/eval.hpp#L10

I did not touch these when refactoring.

brunocodutra commented 5 years ago

"typename" also appears in the docs as a word (natural English language):

Oh, actually it had not occurred to me until now, but typename is the only option in this case, since it refers to the disambiguator.