Skip to content
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

Rename concepts to standard_case #93

Open
marioarbras opened this issue Apr 17, 2020 · 63 comments
Open

Rename concepts to standard_case #93

marioarbras opened this issue Apr 17, 2020 · 63 comments
Labels
design Design-related discussion help wanted Extra attention is needed iso The ISO C++ Committee related work

Comments

@marioarbras
Copy link

Following the changes introduced by P1754R1 to the concepts library in C++20, consider renaming all concepts to standard_case, for consistency.

@mpusz
Copy link
Owner

mpusz commented Apr 17, 2020

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 ratio type and Ratio concept, quantity type and Quantity concept. How would you rename any of those to be consistent with standard_case and still be easy to understand and use? And no, I do not think that _c postfix for concepts is a good idea ;-)

@marioarbras
Copy link
Author

I agree with you in that adding a _c postfix is not the solution 😄

All the concepts defined in dimensions.h, bitrate.h, and information.h seem to be no problem. If you make them standard_case there are no name clashes with the types in the units::cgs, units::natural and units::si namespaces.

From To
units::DimensionOf units::dimension_of
units::QuantityOf units::quantity_of_dimension
units::Length units::length
units::Mass units::mass
units::Time units::time
units::Current units::current
units::Temperature units::temperature
units::Substance units::substance
units::LuminousIntensity units::luminous_intensity
units::Frequency units::frequency
units::Area units::area
units::Volume units::volume
units::Velocity units::velocity
units::Acceleration units::acceleration
units::Force units::force
units::Momentum units::momentum
units::Energy units::energy
units::Density units::density
units::Power units::power
units::Voltage units::voltage
units::ElectricCharge units::electric_charge
units::Capacitance units::capacitance
units::SurfaceTension units::surface_tension
units::Pressure units::pressure
units::MagneticInduction units::magnetic_induction
units::MagneticFlux units::magnetic_flux
units::Inductance units::inductance
units::Conductance units::conductance
units::CatalyticActivity units::catalytic_activity
units::AbsorbedDose units::absorbed_dose
units::CurrentDensity units::current_density
units::Concentration units::concentration
units::Luminance units::luminance
units::DynamicViscosity units::dynamic_viscosity
units::HeatCapacity units::heat_capacity
units::SpecificHeatCapacity units::specific_heat_capacity
units::MolarHeatCapacity units::molar_heat_capacity
units::ThermalConductivity units::thermal_conductivity
units::ElectricFieldStrength units::electric_field_strength
units::ChargeDensity units::charge_density
units::SurfaceChargeDensity units::surface_charge_density
units::Permittivity units::permittivity
units::Permeability units::permeability
units::MolarEnergy units::molar_energy
units::Bitrate units::bitrate
units::Information units::information

Some concepts could be moved to a different namespace (utils maybe?), as they are not necessarily units-related.

From To
units::Downcastable utils::downcastable (although units::downcastable would still work)
units::TypeList utils::type_list (although units::type_list would still work)
units::Scalar utils::Scalar (although units::scalar would still work)
units::Ratio utils::ratio

The remaining concepts in concepts.h seem to be the problematic ones. What about something along these lines?

From To
units::PrefixFamily units::unit_prefix_family
units::Prefix units::unit_prefix
units::UnitRatio units::unit_ratio
units::Unit units::physical_unit or units::measurement_unit
units::BaseDimension units::base_quantity_dimension
units::DerivedDimension units::derived_quantity_dimension
units::Dimension units::quantity_dimension
units::Exponent units::exponent or units::dimension_exponent
units::UnitOf units::unit_of_dimension
units::Quantity units::physical_quantity or units::quantifiable
units::WrappedQuantity units::wrapped_physical_quantity or units::wrapped_quantifiable

@mpusz
Copy link
Owner

mpusz commented May 7, 2020

I did an approach to this subject and found the following issues:

  • bitrate and information actually is a problem as we have such aliases already
  • I do not like the idea to have ratio and utils::ratio
  • when we have prefix_family and unit_prefix_family and similar how to know which one is a type and which one is a concept (same applies to prefix, base_dimension, derived_dimension, quantity)

I am not saying no, but we need to address at least the above before we can continue.

@kwikius
Copy link
Contributor

kwikius commented May 8, 2020

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

  • Consistent.
  • Descriptive.
  • Follows existing convention.
  • Unlikely to collide with current symbols.
  • Easy to remember.
  • Easy to extend to new symbols.

(Edited to shorten some names that are now not ambiguous I think)

From To
units::DimensionOf units::concept_dimension
units::QuantityOf units::concept_quantity
units::Length units::concept_length
units::Mass units::concept_mass
units::Time units::concept_time
units::Current units::concept_current
units::Temperature units::concept_temperature
units::Substance units::concept_substance
units::LuminousIntensity units::concept_luminous_intensity
units::Frequency units::concept_frequency
units::Area units::concept_area
units::Volume units::concept_volume
units::Velocity units::concept_velocity
units::Acceleration units::concept_acceleration
units::Force units::concept_force
units::Momentum units::concept_momentum
units::Energy units::concept_energy
units::Density units::concept_density
units::Power units::concept_power
units::Voltage units::concept_voltage
units::ElectricCharge units::concept_electric_charge
units::Capacitance units::concept_capacitance
units::SurfaceTension units::concept_surface_tension
units::Pressure units::concept_pressure
units::MagneticInduction units::concept_magnetic_induction
units::MagneticFlux units::concept_magnetic_flux
units::Inductance units::concept_inductance
units::Conductance units::concept_conductance
units::CatalyticActivity units::concept_catalytic_activity
units::AbsorbedDose units::concept_absorbed_dose
units::CurrentDensity units::concept_current_density
units::Concentration units::concept_concentration
units::Luminance units::concept_luminance
units::DynamicViscosity units::concept_dynamic_viscosity
units::HeatCapacity units::concept_heat_capacity
units::SpecificHeatCapacity units::concept_specific_heat_capacity
units::MolarHeatCapacity units::concept_molar_heat_capacity
units::ThermalConductivity units::concept_thermal_conductivity
units::ElectricFieldStrength units::concept_electric_field_strength
units::ChargeDensity units::concept_charge_density
units::SurfaceChargeDensity units::concept_surface_charge_density
units::Permittivity units::concept_permittivity
units::Permeability units::concept_permeability
units::MolarEnergy units::concept_molar_energy
units::Bitrate units::concept_bitrate
units::Information units::concept_information

@mpusz
Copy link
Owner

mpusz commented May 8, 2020

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.

@kwikius
Copy link
Contributor

kwikius commented May 9, 2020

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.” .

@kwikius
Copy link
Contributor

kwikius commented May 9, 2020

Try an experiment.

Take a random list of words (https://randomwordgenerator.com/)

start
memorandum
clerk
energy
bacon
neglect
isolation
chauvinist
ratio
person
story
domination
safari
respect
sodium
concentration
trail

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
memorandum
concept_clerk
energy
concept_bacon
neglect
isolation
chauvinist
concept_ratio
person
story
domination
safari
respect
sodium
concept_concentration
trail

start
memorandum
clerk_concept
energy
bacon_concept
neglect
isolation
chauvinist
ratio_concept
person
story
domination
safari
respect
sodium
concentration_concept
trail

start
memorandum
[)clerk(]
energy
[)bacon(]
neglect
isolation
chauvinist
[)ratio(]
person
story
domination
safari
respect
sodium
[)concentration(]
trail

@mpusz
Copy link
Owner

mpusz commented May 9, 2020

I have re-read P1754R1 and the only reference to prefix and suffix is in 1.1.2.

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. std::ranges::sentinel means a concept but there is no type named sentinel in the ranges library. If it was, we would have big naming issues there. Now when we have quantity and Quantity, unit and Unit, ratio and Ratio, and more, we have to find out a good solution. We can add the prefix or suffix but I hope we can do something more consistent with the C++ Library or we will have the same discussion soon when we will try to standardize it.

I will try to consult it with some LEWG members and find the best solution.

@kwikius
Copy link
Contributor

kwikius commented May 21, 2020

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 .
In the brave new world of in code concept names to be added to all the other types of entity names there needs to be some way to distinguish concepts. It is just unrealistic to expect to find terse meaningful names ad-hoc, therefore using a consistent scheme seems to me to be the right way to go.

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;

}

@mpusz
Copy link
Owner

mpusz commented May 24, 2020

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 _c postfix for classes we should not mark concepts in a special way (FYI, I was against it and voted for PascalCase). We can add _concept postfix to
every concept in the library but when we will go with it to the Committee we will have to rename all of them again.

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.

@kwikius
Copy link
Contributor

kwikius commented May 25, 2020

We can add _concept postfix to
every concept in the library but when we will go with it to the Committee we will have to rename all of them again.

No problem. And I thought of yet another benefit:
Using _concept postfix all concepts will be easy to grep, when you want to rename them ;-)

@mpusz mpusz added the help wanted Extra attention is needed label May 28, 2020
@JohelEGP
Copy link
Collaborator

http://wg21.link/P1199 would be a solution for snake-case types and equivalent pascal-case concepts, as you'd be able to write units::quantity auto delta{2q_m};. I don't remember its status, or if anyone commented about it at all on the Internet. It predates https://github.com/cplusplus/papers/.

@JohelEGP
Copy link
Collaborator

An alternative is to do away with some concepts that can't be directly renamed and instead use concepts like specialization_of, std::same_as, std::derived_from or whatever's best for a particular case. Not as concise, but works.

@kwikius
Copy link
Contributor

kwikius commented Jun 7, 2020

My latest idea for this, use prefix in_ .

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)

From To
units::DimensionOf units::in_dimension_of
units::QuantityOf units::in_quantity_of
units::Length units::in_length
units::Mass units::in_mass
units::Time units::in_time
units::Current units::in_current
units::Temperature units::in_temperature
units::Substance units::in_substance
units::LuminousIntensity units::in_luminous_intensity
units::Frequency units::in_frequency
units::Area units::in_area
units::Volume units::in_volume
units::Velocity units::in_velocity
units::Acceleration units::in_acceleration
units::Force units::in_force
units::Momentum units::in_momentum
units::Energy units::in_energy
units::Density units::in_density
units::Power units::in_power
units::Voltage units::in_voltage
units::ElectricCharge units::in_electric_charge
units::Capacitance units::in_capacitance
units::SurfaceTension units::in_surface_tension
units::Pressure units::in_pressure
units::MagneticInduction units::in_magnetic_induction
units::MagneticFlux units::in_magnetic_flux
units::Inductance units::in_inductance
units::Conductance units::in_conductance
units::CatalyticActivity units::in_catalytic_activity
units::AbsorbedDose units::in_absorbed_dose
units::CurrentDensity units::in_current_density
units::Concentration units::in_concentration
units::Luminance units::in_luminance
units::DynamicViscosity units::in_dynamic_viscosity
units::HeatCapacity units::in_heat_capacity
units::SpecificHeatCapacity units::in_specific_heat_capacity
units::MolarHeatCapacity units::in_molar_heat_capacity
units::ThermalConductivity units::in_thermal_conductivity
units::ElectricFieldStrength units::in_electric_field_strength
units::ChargeDensity units::in_charge_density
units::SurfaceChargeDensity units::in_surface_charge_density
units::Permittivity units::in_permittivity
units::Permeability units::in_permeability
units::MolarEnergy units::in_molar_energy
units::Bitrate units::in_bitrate
units::Information units::in_information

@foonathan
Copy link

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?

  • Ratio: all instantiations of ratio template
  • Prefix: all instantiations of prefix template
  • Quantity: all instantiations of quantity template
  • PrefixFamily: all derived from units::prefix_family
  • Unit: all derived from units::scaled_unit
  • BaseDimension: derived from units::base_dimension
  • DerivedDimension: "derived" from units::derived_dimension

For Ratio, Prefix, and Quantity, my personal preference is dropping those concepts, but you disagree. So what about renaming them to XXX_instantiation or XXX_inst or something along those lines? They are not generic concepts, they are merely shorthands for specifying template parameters. Their name should reflect those.

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 XXX_base or enable_XXX (because base_dimension_base might be confusing)?

BTW, why is it Unit and not ScaledUnit?

@mpusz
Copy link
Owner

mpusz commented Jun 17, 2020

Thanks @foonathan! Yes, it seems is a full list of problematic framework types. However, there are some other potential conflicts there. For example:

https://github.com/mpusz/units/blob/dcbadfe285b9fbe6357604537ae7d44d430f84c8/src/include/units/data/information.h#L48-L52

I like your suggestions about adding _inst to some concepts. However, changing the types may lower compile-time errors and debugging experience as the user faces there types rather than concepts.

Regarding your question about Unit a scaled_unit is only a common base which is an implementation detail and user should never play with this type (probably it will even not be exposed in the public module interface). For many units the ratio is equal 1 so talking about a scaled unit, in this case, may be misleading. You can find a whole units hierarchy here: https://mpusz.github.io/units/framework/units.html#class-hierarchy.

@kwikius
Copy link
Contributor

kwikius commented Jun 22, 2020

maybe type quantity could be renamed to basic_quantity following precedent of std::basic_string and free up quantity for the concept?

@foonathan
Copy link

However, changing the types may lower compile-time errors and debugging experience as the user faces there types rather than concepts.

How? It seems the type just inject a couple of members into the class, so they wouldn't even be strictly necessary.

@mpusz
Copy link
Owner

mpusz commented Jun 22, 2020

How?

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 enable_dervied_dimension in the debugger in a quantity type.

@mpusz
Copy link
Owner

mpusz commented Jun 22, 2020

maybe type quantity could be renamed to basic_quantity following precedent of std::basic_string and free up quantity for the concept?

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 basic_quantity a more defined entity I would not like to follow with this... The downcasting facility (as of now) does not downcast quantity(si::dim_length, ...) to si::length(...) as without strong typedefs in the language it is really hard to define a child class with exactly the same semantics as the base one (base class instantiation) in a few lines (i.e. importing all the constructors, operators, etc)...

@kwikius
Copy link
Contributor

kwikius commented Jun 22, 2020

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)

@kwikius
Copy link
Contributor

kwikius commented Jun 22, 2020

For Ratio, Prefix, and Quantity, [...] They are not generic concepts, they are merely shorthands for specifying template parameters. [...]

'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!
(The only time you really need to specify the class template parameters explicitly is when defining the class template)

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;
}

@foonathan
Copy link

'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.

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 quanitty<auto> feature? I think that would be a better solution to the verbosity than using concepts.

@kwikius
Copy link
Contributor

kwikius commented Jun 23, 2020

'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.

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:

In what I said as far as I know I never precluded that concepts be open...

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 quanitty<auto> feature? I think that would be a better solution to the verbosity than using concepts.

Yet concepts work just fine. Why add yet another feature?

here is an example why it is superior to do things this way

#129

See also #111 where I lay out pretty much the same arguments, but in more detail

@foonathan
Copy link

Yet concepts work just fine.

If they worked fine, you would have a name for them...

here is an example why it is superior to do things this way

Yes, it's less typing.

@mpusz
Copy link
Owner

mpusz commented Jan 27, 2021

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 any_ for type erased types. If not I will be the first to suggest using it for a concept matching any instance of a class template ;-)

@JohelEGP
Copy link
Collaborator

With #213 in mind, that wouldn't work.

  • any_quantity would be an specialization of quantity for which the first template argument is a Dimension.
  • any_quantity_kind would be an specialization of quantity for which the first template argument is a Kind.
    There'd be no quantity_kind template, so any_quantity_kind wouldn't fit that prescription of any_.

@mpusz
Copy link
Owner

mpusz commented Jan 28, 2021

There'd be no quantity_kind template, so any_quantity_kind wouldn't fit that prescription of any_.

Right 😞

@JohelEGP
Copy link
Collaborator

Quantity could be renamed to relative_quantity and QuantityPoint to absolute_quantity.

@JohelEGP
Copy link
Collaborator

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 metre.is(unit), which would replace the need for some concepts. Assuming that no sanity checks are needed, like Kind does. Those guard against users that, for example, redeclare ratio as something else in the derived type.

@mpusz mpusz pinned this issue May 12, 2021
@JohelEGP
Copy link
Collaborator

JohelEGP commented Jan 7, 2022

Another alternative is to append _type to concepts that defer to specialization_of only:

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 _type suffix by requiring the concept to have the form of the example above.

@mpusz mpusz added the design Design-related discussion label Jun 24, 2023
@mpusz mpusz added the iso The ISO C++ Committee related work label Aug 23, 2023
@JohelEGP
Copy link
Collaborator

JohelEGP commented Oct 12, 2023

Another suggestion:

  • scalar_rep for a scalar number type.
  • vector_rep for a vector number type.
  • tensor_rep for a tensor number type.
  • scalar_qty for Quantity with a scalar_rep.
  • vector_qty for Quantity with a vector_rep.
  • tensor_qty for Quantity with a tensor_rep.
  • scalar_point_qty for QuantityPoint with a scalar_rep.
  • vector_point_qty for QuantityPoint with a vector_rep.
  • tensor_point_qty for QuantityPoint with a tensor_rep.

@mpusz
Copy link
Owner

mpusz commented Oct 12, 2023

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.

@JohelEGP
Copy link
Collaborator

JohelEGP commented Oct 12, 2023

Actually, it's the subset whose snake_case spelling clashes with an existing entity:

  • QuantitySpec
  • Unit
  • Reference
  • Quantity
  • PointOrigin
  • QuantityPoint

If those are solved, the others can simply be spelled like snake_case.

@mpusz
Copy link
Owner

mpusz commented Oct 12, 2023

Agree. Actually Unit and PointOrigin do not clash with anything, but I think it would be hard to reason about when we are talking about types and when about concepts if those were provided with such names.

@JohelEGP
Copy link
Collaborator

JohelEGP commented Jan 7, 2024

a_quantity: https://youtu.be/Jhggz8rtHbk?t=1816.

@mpusz
Copy link
Owner

mpusz commented Jan 8, 2024

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:

template<QuantitySpec auto ToQS, typename Q>
requires Quantity<std::remove_cvref_t<Q>> && (castable(Q::quantity_spec, ToQS))
[[nodiscard]] constexpr Quantity auto quantity_cast(Q&& q)
{
return quantity{std::forward<Q>(q).numerical_value_is_an_implementation_detail_, make_reference(ToQS, Q::unit)};
}

@atomgalaxy
Copy link

Bronek uses "some_*" in his lib and that's a bit less dependent on people knowing English a/an distinctions and slightly more greppable

@atomgalaxy
Copy link

Also thanks for quoting my fist pass on that talk ;) I hope to give it better next time.

@mpusz
Copy link
Owner

mpusz commented Jan 8, 2024

I thought about any_* but that could be confused with type-erased wrappers although we agreed in LEWG that any_ prefix should not be reserved for such abstractions.

@JohelEGP
Copy link
Collaborator

JohelEGP commented Jan 8, 2024

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:

template<QuantitySpec auto ToQS, typename Q>
requires Quantity<std::remove_cvref_t<Q>> && (castable(Q::quantity_spec, ToQS))
[[nodiscard]] constexpr Quantity auto quantity_cast(Q&& q)
{
return quantity{std::forward<Q>(q).numerical_value_is_an_implementation_detail_, make_reference(ToQS, Q::unit)};
}

I don't see a problem in this particular example.
Q is a forwarding reference, so we accept all cv-ref combinations.
For ToQS, the NTTP, its placeholder type isn't ref-qualified,
so it's just like an auto function parameter in that the cv-ref qualification of the argument doesn't matter.
All uses of NTTPs are like that, so there shouldn't be a problem with the naming convention.

@atomgalaxy
Copy link

The main problem with having std::remove_cvref_t everywhere is that it's really verbose and users don't care. It's also not exactly quick to compile before clang17/gcc-trunk (14?).

Just because you can write it, doesn't mean you should.

@mpusz
Copy link
Owner

mpusz commented Jan 8, 2024

@atomgalaxy, do you recommend all similar concepts to match any cvref-qualified specialization of a class template?

@atomgalaxy
Copy link

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.

@atomgalaxy
Copy link

BTW, this is the fastest-to-compile way I could find to implement such concepts: https://www.youtube.com/watch?v=Jhggz8rtHbk&t=2562s

@JohelEGP
Copy link
Collaborator

That seems to be 10 min 30 s too late.
Here's 10 min 30 s earlier:
1705764317

@atomgalaxy
Copy link

For the record, Bronek has been using the some_{typename} naming scheme, to avoid having to know which article (a/an) is appropriate in English.

@JohelEGP
Copy link
Collaborator

With reflection, we can define a

template<class T, std::meta::info TypeOrTemplate>
concept some = /* ... */;

used like some<^quantity> auto f();.

@atomgalaxy
Copy link

atomgalaxy commented Oct 17, 2024 via email

@JohelEGP
Copy link
Collaborator

I am of the opinion that we should snake_case all concepts we can right now.
WG21 won't accept them in their current form.
This will make the few stand out.
The refactor might also inspire you to fix the remaining few.

@mpusz
Copy link
Owner

mpusz commented Nov 14, 2024

Yes and no. The current spelling makes the interface easier for everyone to understand, which makes it easier to discuss the design and interfaces. The last thing we need now is ambiguity when we talk about types and concepts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Design-related discussion help wanted Extra attention is needed iso The ISO C++ Committee related work
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants