Rename concepts to standard_case
#93
Comments
|
I would love to but this is not that easy. I was in the LEWG room when we discussed P1745 and I am fully aware of the naming conventions in the C++ Standard Library. I also brought my case there but at this moment it was just a theory. Now it is real and we will have to find the best naming convention for those cases. The problem is that we have |
|
I agree with you in that adding a All the concepts defined in
Some concepts could be moved to a different namespace (
The remaining concepts in
|
|
I did an approach to this subject and found the following issues:
I am not saying no, but we need to address at least the above before we can continue. |
|
The following is my preferred solution ( variations involve maybe a units_concept:: or units::concepts:: namespaces, but this variant avoids ambiguity introduced by using declarations as is currently an issue e.g with using namespace std;.) The solution is
(Edited to shorten some names that are now not ambiguous I think)
|
|
I must admit it has some good points. It is verbose thus easy to understand and unambiguous. However, it is inconsistent with what ISO C++ Committee decided for concepts in C++20 (no prefix or postfix). If I am about to be inconsistent anyway I prefer to use CapitalLeters as it is shorter. We probably need to find a better solution to move to a standard_case. |
I have re-read P1754R1 and the only reference to prefix and suffix is in 1.1.2. It is worth re-reading that since it is an argument about retaining UpperCase rather than ruling out prefixes and suffixes per se. The argument is as I understand it that short prefixes and suffixes are cryptic. That is not comparable with prefixing with full word concept_ : 1.1.2 Retain status quo PascalCase, because (for example) having both std::copy_constructible and std::is_copy_constructible mean different things and give subtly different answers in some cases creates user confusion and pitfalls.We think this concern already exists with std::is_copy_constructible_v<T> and std::CopyConstructible<T>, because new users don’t know that PascalCase means something magical any more than they know that the prefix is_ and suffix _v mean something magical. Novice users will conflate the trait and the concept regardless of the transformation we apply to the words “copy” and “constructible” if both the trait and the concept contain some variation of those words." The above argument seems to be that ambiguity will remain in spite of whether upper case or lower case is used : "One of the authors who initially preferred PascalCase concept naming “found that after a while std::copy_constructible, std::is_copy_constructible, and std::CopyConstructible are equally similar, equally different, and if you see any two of them you have to head for cppreference.com or equivalent to find out the difference.” . |
|
Try an experiment. Take a random list of words (https://randomwordgenerator.com/) start Now ask yourself how to demarcate which of those words is a concept. The only options I see are namespace , prefix, postfix or grammar : EDITED . By chance ratio was in there :) start start start |
I was not referring to the paper but to the discussion that we had in the LEWG room at that time. The committee was clear that any concept-specific prefix or postfix (or even a dedicated namespace) is not the right way to go. Also, the cases we analyzed at that time were different i.e. I will try to consult it with some LEWG members and find the best solution. |
|
Experimenting with c++20 style concepts to design an angle type (#99) , I opted to use a postfix "_concept" naming rule to identify associated concepts. (Bear with my use of concepts since I haven't used them much). Nevertheless the resulting code purely seems to me to work well for legibility and understanding . No more agonising and time wasting about entity naming and distinctiveness each time you wish to deal with a new concept, juts follow the rule so you can get on with other things. It just works... #include <ratio>
#include <type_traits>
namespace std{ namespace experimental {
namespace detail{
template<typename T>
inline constexpr bool is_ratio = false;
template<typename T>
inline constexpr bool is_angle = false;
template<typename T>
inline constexpr bool is_radian = false;
template<typename T>
inline constexpr bool is_real_number = false;
}
template <typename T>
concept ratio_concept = std::experimental::detail::is_ratio<T>;
namespace detail{
template <std::intmax_t num, std::intmax_t den>
inline constexpr bool is_ratio<std::ratio<num,den> > = true;
}
template <typename T>
concept angle_concept = std::experimental::detail::is_angle<T>;
template <typename T>
concept radian_concept = std::experimental::detail::is_radian<T>;
template <typename T>
concept floating_point_concept = std::is_floating_point_v<T>;
template <typename T>
concept arithmetic_concept = std::is_arithmetic_v<T>;
template <typename T>
concept real_number_concept = std::experimental::detail::is_real_number<T>;
namespace detail{
template<std::experimental::floating_point_concept T>
inline constexpr bool is_real_number<T> = true;
}
template <real_number_concept ValueType = double,ratio_concept Exponent = std::ratio<1U>>
class radian;
namespace detail{
template <real_number_concept ValueType,ratio_concept Exponent>
inline constexpr bool is_radian<std::experimental::radian<ValueType,Exponent> > = true;
template <real_number_concept ValueType,ratio_concept Exponent>
inline constexpr bool is_angle<std::experimental::radian<ValueType,Exponent> > = true;
}
template <real_number_concept ValueType,ratio_concept Exponent>
class radian{
public:
using exponent = typename Exponent::type;
using value_type = ValueType;
radian() = default;
radian(const radian & ) = default;
radian(radian &&) = default;
// implicit construction from numeric
constexpr radian( arithmetic_concept const & v) : m_value{v}{}
constexpr radian operator + () { return *this;}
constexpr radian operator - () { return - this->m_value;}
constexpr value_type numeric_value()const { return m_value;}
private:
value_type m_value{};
};
// addition only where exponent is the same
template <radian_concept Lhs, radian_concept Rhs>
inline constexpr auto
operator + (Lhs const & lhs, Rhs const & rhs)
requires std::ratio_equal_v<
std::ratio_subtract<typename Lhs::exponent,typename Rhs::exponent>,
std::ratio<0>
>
{
using value_type = std::common_type_t<typename Lhs::value_type,typename Rhs::value_type> ;
using exponent = typename Lhs::exponent::type;
typedef radian<value_type,exponent> result_type;
return result_type{
lhs.numeric_value() + rhs.numeric_value()
};
}
// subtraction only where exponent is the same
template <radian_concept Lhs, radian_concept Rhs>
inline constexpr auto
operator - (Lhs const & lhs, Rhs const & rhs)
requires std::ratio_equal_v<
std::ratio_subtract<typename Lhs::exponent,typename Rhs::exponent>,
std::ratio<0>
>
{
using value_type = std::common_type_t<typename Lhs::value_type,typename Rhs::value_type> ;
using exponent = typename Lhs::exponent::type;
using result_type = radian<value_type,exponent>;
return result_type{
lhs.numeric_value() - rhs.numeric_value()
};
}
template <radian_concept Lhs, radian_concept Rhs>
inline constexpr auto
operator * (Lhs const & lhs, Rhs const & rhs)
requires std::ratio_equal_v<
std::ratio_add<typename Lhs::exponent,typename Rhs::exponent>,
std::ratio<0>
>
{
return lhs.numeric_value() * rhs.numeric_value();
}
template <radian_concept Lhs, radian_concept Rhs>
inline constexpr auto
operator * (Lhs const & lhs, Rhs const & rhs)
requires ! std::ratio_equal_v<
std::ratio_add<typename Lhs::exponent,typename Rhs::exponent>,
std::ratio<0>
>
{
using value_type = std::common_type_t<typename Lhs::value_type,typename Rhs::value_type> ;
using exponent = std::ratio_add<typename Lhs::exponent,typename Rhs::exponent>;
using result_type = radian<value_type,exponent>;
return result_type {lhs.numeric_value() * rhs.numeric_value()};
}
}} // std::experimental
struct dummy{};
int main()
{
std::experimental::radian<double,std::ratio<2> > constexpr r = 1.2;
std::experimental::angle_concept a = r;
std::experimental::angle_concept b = r + r;
std::experimental::radian<double,std::ratio<-2> > constexpr r1 = 1.2;
std::experimental::angle_concept b1 = r1 + r1;
std::experimental::radian<float,std::ratio<2> > constexpr rf = 1.2f;
// should fail
std::experimental::radian<int,std::ratio<2> > constexpr ri = 1;
}
|
|
As I wrote, there was a long discussion about this. We always want to somehow mark "a new thing" as we are not to use it, we are a bit afraid of it (so we want to make sure it stands out in our code) and we assume that it will probably barely be used. This was of the main arguments against PascalCase for concepts. Defenders of this syntax claimed that PascalCase was not used because it is new or we are afraid of it but because this is how we typed concepts from 80's. Anyway, it was decided that as we do not type This is why I would like us to try to find a solution that matches ISO requirements or leave it as it is right now and leave the bikeshedding discussion to the LEWG room. |
No problem. And I thought of yet another benefit: |
|
http://wg21.link/P1199 would be a solution for snake-case types and equivalent pascal-case concepts, as you'd be able to write |
|
An alternative is to do away with some concepts that can't be directly renamed and instead use concepts like |
|
My latest idea for this, use prefix EDIT ( I am aware that we weren't allowed to identify bikes in the bike shed, by painting them blue, but I just decided to choose to paint them all blue incidentally ;-) ) Rationale : The overall problem is that a name x could represent a type or a concept aka a set of types, so prefix the concept with in to mean in the set of x, therefore by implication x is a set or a concept (Edited to different prefix)
|
|
It seems like this is the full list of concepts that cannot be easily renamed into snake_case due to naming conflicts, or am I missing some?
For The other ones all match certain types who derived certain other types. It seems like the only purpose of the other types is to specify that some user-defined type models that concept. So I'd suggest you keep the name of the concepts as-is - this is the name users want to interact with when writing generic code. Instead, you should rename the type. Maybe something like BTW, why is it |
|
Thanks @foonathan! Yes, it seems is a full list of problematic framework types. However, there are some other potential conflicts there. For example: I like your suggestions about adding Regarding your question about |
|
maybe type |
How? It seems the type just inject a couple of members into the class, so they wouldn't even be strictly necessary. |
OK, maybe I was wrong and one step ahead. At least for now, with the dowcasting facility working as it is now, the end-user is nearly never facing those base classes. However, if we decide (or will be forced to disable) a downcasting facility than all of the results of calculations will result in those types and it will be up to the user to explicitly cast those results into the child classes that have more user-friendly names and additional type information (i.e. how to print a unit symbol on the screen). With this, I assume a user might be confused to see a type like |
This actually will always be visible to the user. For a function with the following definition: Velocity auto avg_speed(Length auto l, Time auto t);the user will see in the debugger: avg_speed(basic_quantity<si::dim_length, si::metre, double> l, basic_quantity<si::dim_time, si::second, double> t)As there will be no other types that will make |
|
I am just suggesting rename : quantity<D,U,V>to basic_quantity<D,U,V>therefore : avg_speed(quantity<si::dim_length, si::metre, double> l, quantity<si::dim_time, si::second, double> t)to avg_speed(basic_quantity<si::dim_length, si::metre, double> l, basic_quantity<si::dim_time, si::second, double> t) |
'shorthands for specifying template parameters' seems to me an excellent and perfectly reasonable use for concepts just as a class template is merely a shorthand for specifying many types. Concepts is the next level of concision, which we have been waiting for for a long time, so let us use them! template<std::intmax_t N, typename D, typename U, typename Rep>
requires(N == 0)
inline Rep pow(const basic_quantity<D,U,Rep>&) noexcept
{
return 1;
}to : template<std::intmax_t N, quantity Q>
requires(N == 0)
inline Q::rep pow(const Q&) noexcept
{
return 1;
} |
I'd disagree. Concepts should be "open": user types should also be able to model it, otherwise you're just hard-coding types. For return types and variables you can use auto or CTAD: auto foo() /* -> quantity */
{
…
}
auto q1 = foo();
quantity q2 = foo();There is just no nice syntax for function parameters. While I agree that it would be nice to have something shorter, I'd rather have a designated language feature for it. Maybe CTAD can be extended to function parameters? Or some form of |
In what I said as far as I know I never precluded that concepts be open...
Yet concepts work just fine. Why add yet another feature? here is an example why it is superior to do things this way See also #111 where I lay out pretty much the same arguments, but in more detail |
If they worked fine, you would have a name for them...
Yes, it's less typing. |
Such base class could be a CRTP base class. They are input and output the derived type instead of the base class, like this: https://github.com/johelegp/jge/blob/llvm-D79773/include/jge/detail/quantity_wrappers.hpp#L39-L64. And are used like this: https://github.com/johelegp/jge/blob/llvm-D79773/include/jge/cartesian.hpp#L14-L22 (the deduction guide due to the pending CTAD for inherited constructors). Not that I like that. I think a single template, like Or maybe the explosion of templates is not that bad. They, after all, use the same base template. And themselves add very little otherwise. |
|
Following cplusplus/papers#929 (comment), I believe it reasonable to rename |
|
I think you understood the note in the wrong way.
The above means a type-erasure-based (runtime polymorphism) type that represents in runtime any type matched by a concept. I was attending this meeting and again I mentioned that |
|
My understanding was that LEWG's standing on With |
|
Yeah, I had the same hope here and raised this at the LEWG meeting. But it seems that the Committee still did not decide if we should use |
|
With #213 in mind, that wouldn't work.
|
Right 😞 |
|
|
|
At this point, I think PRs of alternatives should help guide decisions. I hope to get around doing that. Alternatively, somewhere in the future, metaclasses could help. I recall being able to do something like |
|
Another alternative is to append template<class T> concept quantity_type = specialization_of<T, quantity>;This seems to go against point 2.2 of https://wg21.link/P1851, but that's a general guideline. We justify the use of the |
|
Another suggestion:
|
|
The list of concepts to be renamed can be found here: https://mpusz.github.io/mp-units/2.1/users_guide/framework_basics/basic_concepts. None of the mentioned above are there, and I am not sure if we want to add more to the list. |
|
Actually, it's the subset whose
If those are solved, the others can simply be spelled like |
|
Agree. Actually |
|
|
|
This an interesting naming convention and might be a solution for our problems :-) However, @atomgalaxy used it also for cvref-qualified things while we typically do not do that: mp-units/src/core/include/mp-units/bits/quantity_cast.h Lines 49 to 54 in 4690a84 |
|
Bronek uses "some_*" in his lib and that's a bit less dependent on people knowing English a/an distinctions and slightly more greppable |
|
Also thanks for quoting my fist pass on that talk ;) I hope to give it better next time. |
|
I thought about |
I don't see a problem in this particular example. |
|
The main problem with having Just because you can write it, doesn't mean you should. |
|
@atomgalaxy, do you recommend all similar concepts to match any cvref-qualified specialization of a class template? |
|
Yeah, so far I haven't found a use for concepts that aren't cvref-agnostic (and if I need those, I usually want to add "and is an lvalue reference" but... you can usually spell that as a binding constraint anyway). Conversely, I found myself removing cvref pretty much every time, because forwarding cvref-qualifiers is the majority usecase. |
|
BTW, this is the fastest-to-compile way I could find to implement such concepts: https://www.youtube.com/watch?v=Jhggz8rtHbk&t=2562s |
|
That seems to be 10 min 30 s too late. |
|
For the record, Bronek has been using the |
Following the changes introduced by P1754R1 to the concepts library in C++20, consider renaming all concepts to
standard_case, for consistency.The text was updated successfully, but these errors were encountered: